This was seen on Android O, debug builds.

Link to bot?

clang only, so perhaps lower priority from that.  Also root cause has been reverted, so dropping priority.

Do we think this is a compiler issue, or an issue with the QUIC code?

We don't know. Does the stack look like a possible code bug.

I am unable to reproduce so far.

This is my args.gn:

target_os = "android"
is_clang = true
symbol_level = 1
use_goma = true
target_cpu = "arm64"

time ninja -C out/gnand8 chrome_public_apk -j1000
build/android/adb_install_apk.py out/gnand8/apks/ChromePublic.apk

I still can't repro. Trying with the args.gn someone uses who saw this crash.


thakis@thakis:~/src/chrome/src$ build/android/adb_install_apk.py out/gnand6/apks/ChromePublic.apk
thakis@thakis:~/src/chrome/src$ cat out/gnand6/args.gn 
target_os = "android"
is_clang = true
use_goma = true
proprietary_codecs=true                                                                                       
ffmpeg_branding="Chrome"  

thakis@thakis:~/src/chrome/src$ ninja -C out/gnand6 chrome_public_apk -j1000
ninja: Entering directory `out/gnand6'
ninja: no work to do.

thakis@thakis: ~/src/chrome/src$ build/android/adb_install_apk.py out/gnand6/apks/ChromePublic.apk

Chrome installs fine and runs fine on a Nexus 9 running Android 7.1.1. I'm synced to #474341.

I can reproduce this locally with my "normal" (overcomplicated) config. I'll try to reduce it to see if it still crashes.

Yeah, this crashes for me (not literally at startup but the first time I attempt to load a page) on Nexus 6 running 7.x, with just:

use_goma = true
target_os = "android"
is_clang = true

I'll try in a clank build tomorrow. Which target are you building, which apk do you copy over?

chrome_public_apk, installing ChromePublic.apk.

I am technically using a clankium tree here but I'm building a public target so there shouldn't actually be much different..

torne's binary also doesn't crash on the Nexus 9 I used for testing. I can reproduce the crash on Torne's Nexus 6.

$ build/android/adb_gdb_chrome_public --output-directory=out/gnand6 --adb=third_party/android_tools/sdk/platform-tools/adb 
# (takes several minutes)
(gdb) handle SIG33 nostop noprint pass
(gdb) c

Thread 31 "Thread-3" received signal SIGBUS, Bus error.
[Switching to Thread 9499.9576]
net::QuicCryptoClientStream::DoReceiveREJ (this=0x868bc800, in=0x868bc940, cached=0x93684220) at ../../net/quic/core/quic_crypto_client_stream.cc:402
402	      if (reject_reasons[i] == HANDSHAKE_OK || reject_reasons[i] >= 32) {
(gdb) bt
#0  net::QuicCryptoClientStream::DoReceiveREJ (this=0x868bc800, in=0x868bc940, cached=0x93684220) at ../../net/quic/core/quic_crypto_client_stream.cc:402
#1  0x8f4c179c in net::QuicCryptoClientStream::DoHandshakeLoop (this=0x868bc800, in=0x868bc940) at ../../net/quic/core/quic_crypto_client_stream.cc:212
#2  0x8f4a7912 in net::CryptoFramer::Process (this=0x868bc924, input=..., perspective=<optimized out>) at ../../net/quic/core/crypto/crypto_framer.cc:270
#3  0x8f4a76bc in net::CryptoFramer::ProcessInput (this=0x868bc924, input=..., perspective=net::Perspective::IS_CLIENT) at ../../net/quic/core/crypto/crypto_framer.cc:73
#4  0x8f4c3f64 in net::QuicCryptoStream::OnDataAvailable (this=0x868bc800) at ../../net/quic/core/quic_crypto_stream.cc:68
#5  0x8f4d7fca in net::QuicStreamSequencer::OnStreamFrame (this=0x868bc818, frame=...) at ../../net/quic/core/quic_stream_sequencer.cc:80
#6  0x8f4d70e2 in net::QuicStream::OnStreamFrame (this=0x868bc800, frame=...) at ../../net/quic/core/quic_stream.cc:124
#7  0x8f48faa4 in net::QuicChromiumClientSession::OnStreamFrame (this=0x85c50f00, frame=...) at ../../net/quic/chromium/quic_chromium_client_session.cc:694
#8  0x8f4bcac8 in net::QuicConnection::OnStreamFrame (this=0x846b5000, frame=...) at ../../net/quic/core/quic_connection.cc:683
#9  0x8f4c7d2e in net::QuicFramer::ProcessFrameData (this=0x846b5010, reader=0x85207588, header=...) at ../../net/quic/core/quic_framer.cc:972
#10 0x8f4c77ea in net::QuicFramer::ProcessDataPacket (this=0x846b5010, encrypted_reader=<optimized out>, public_header=..., packet=..., 
    decrypted_buffer=0x852076c0 "\244\001.\005\222\001AA>0Y\027\303\065\210\341\020\347C\213\302\f\244\314\027\003L>\332H\331J\036\232\322\v\262\063\221,\325w\367tt7\002\347\033m\244\234\nS\fu\246!\262\016P\356E\252\312\067?\354\r^{\357\232\324F\375\335St*\037%9\205֝G\253ΐs$\326\366\257\"\244~", buffer_length=<optimized out>) at ../../net/quic/core/quic_framer.cc:601
#11 0x8f4c71fc in net::QuicFramer::ProcessPacket (this=0x846b5010, packet=...) at ../../net/quic/core/quic_framer.cc:529
#12 0x8f4be382 in net::QuicConnection::ProcessUdpPacket (this=0x846b5000, self_address=..., peer_address=..., packet=...) at ../../net/quic/core/quic_connection.cc:1212
#13 0x8f491ea4 in net::QuicChromiumClientSession::OnPacket (this=0x85c50f00, packet=..., local_address=..., peer_address=...) at ../../net/quic/chromium/quic_chromium_client_session.cc:1593
#14 0x8f494394 in net::QuicChromiumPacketReader::OnReadComplete (this=0x86888d60, result=<optimized out>) at ../../net/quic/chromium/quic_chromium_packet_reader.cc:89
#15 0x8f4942aa in net::QuicChromiumPacketReader::StartReading (this=0x86888d60) at ../../net/quic/chromium/quic_chromium_packet_reader.cc:65
#16 0x8f4943b8 in net::QuicChromiumPacketReader::OnReadComplete (this=0x86888d60, result=<optimized out>) at ../../net/quic/chromium/quic_chromium_packet_reader.cc:94
#17 0x8f37baa8 in base::Callback<void (net::CertDatabase::Observer*), (base::internal::CopyMode)1, (base::internal::RepeatMode)1>::Run(net::CertDatabase::Observer*) const & (this=0x85208074, args=0x546) at ../../base/callback.h:80
#18 0x8f4fc7dc in net::UDPSocketPosix::DoReadCallback (this=<optimized out>, rv=1) at ../../net/socket/udp_socket_posix.cc:592
#19 0x8f4fc6ec in net::UDPSocketPosix::DidCompleteRead (this=0x911e4e88) at ../../net/socket/udp_socket_posix.cc:614
#20 0x8f4fc5fe in net::UDPSocketPosix::ReadWatcher::OnFileCanReadWithoutBlocking (this=0x911e4f08) at ../../net/socket/udp_socket_posix.cc:577
#21 0x935c59e0 in base::MessagePumpLibevent::FileDescriptorWatcher::OnFileCanReadWithoutBlocking (fd=138, this=<optimized out>, pump=<optimized out>) at ../../base/message_l

That code goes something like:

  const uint32_t* reject_reasons;
  size_t num_reject_reasons;
  static_assert(sizeof(QuicTag) == sizeof(uint32_t), "header out of sync");
  if (in->GetTaglist(kRREJ, &reject_reasons, &num_reject_reasons) ==
      QUIC_NO_ERROR) {
    uint32_t packed_error = 0;
    for (size_t i = 0; i < num_reject_reasons; ++i) {
      // HANDSHAKE_OK is 0 and don't report that as error.
      if (reject_reasons[i] == HANDSHAKE_OK || reject_reasons[i] >= 32) {


and GetTaglist() is here: https://cs.chromium.org/chromium/src/net/quic/core/crypto/crypto_handshake_message.cc?type=cs&l=82 and does

  QuicTagValueMap::const_iterator it = tag_value_map_.find(tag);
  // ...
  *out_tags = reinterpret_cast<const QuicTag*>(it->second.data());
  *out_len = it->second.size() / sizeof(QuicTag);
  return ret;

And QuicTagValueMap is a `typedef std::map<QuicTag, std::string> QuicTagValueMap`. (And a QuicTag is an uint32_t.

So we're casting a string data pointer to a uint32_t and hoping that that works out. Here we get a SIGBUS (i.e. an alignment error) so I'm guessing fate is catching up with us and calling our bluff. This is likely a code bug.

This is almost certainly why it doesn't fail on Nexus 9 - nvidia's denver recompiling microarchitecture probably just doesn't have (or isn't configured to have) ARM's alignment restrictions.

Quic folks: Things work "fine" (as in "no crash, things seem to work") for me with this change:

$ git diff
diff --git a/net/quic/core/crypto/crypto_handshake_message.cc b/net/quic/core/crypto/crypto_handshake_message.cc
index 220c46adfc0d..b08ff56d3cca 100644
--- a/net/quic/core/crypto/crypto_handshake_message.cc
+++ b/net/quic/core/crypto/crypto_handshake_message.cc
@@ -89,6 +89,8 @@ QuicErrorCode CryptoHandshakeMessage::GetTaglist(QuicTag tag,
     ret = QUIC_CRYPTO_MESSAGE_PARAMETER_NOT_FOUND;
   } else if (it->second.size() % sizeof(QuicTag) != 0) {
     ret = QUIC_INVALID_CRYPTO_MESSAGE_PARAMETER;
+  } else if (reinterpret_cast<uintptr_t>(it->second.data()) % sizeof(QuicTag) != 0) {
+    ret = QUIC_INVALID_CRYPTO_MESSAGE_PARAMETER;
   }
 
   if (ret != QUIC_NO_ERROR) {


However, the Real Bug (tm) is probably that we end up with an unaligned QuicTag list in the value of a QuicTagValueMap in the first place. So the Right Fix (tm) is probably to change the code that creates that list and makes sure that alignment is up to snuff, instead of making the reading code treat this as an error.

Can one of you take this on? Or point me in the right direction?

This blocks -Oz, which in turn is the only blocker I know of for switching chrome/android to clang at this point.

I'm happy to look at this. Question though. What does -Oz do? Is it causing data to be unaligned, or is it causing the read of unaligned data to fail? The way this code works is that a QUIC handshake message on the wire is a map from a 4 byte QuicTag to some sequence of bytes. The format/interpretation of the sequence of bytes depends on the tag (and is really up to the caller). So when populating the map we don't know the type of data, a priori, which I guess is going to complicate whatever we do here...

+agl who original wrote CryptoHandshakeMessage.

BTW: Here's the code which populates this map from the wire:

https://cs.chromium.org/chromium/src/net/quic/core/crypto/crypto_framer.cc?sq=package:chromium&dr=CSs&l=268

-Oz just changes optimization settings. The current code has the bug, it's just lucky that all android compilers happen to load things into temporaries that are aligned right. (Arm chips care about alignment, Intel chips are generally happy with unaligned reads. But they're UB everywhere.)

Totally agree with you that the code is currently exhibiting undefined behavior. Just trying to understand why it's not currently crashing :) Do I understand correctly that -Oz no longer loads into temporaries that happened to be aligned right?

Anyway, I'm working on coming up with a fix...

I didn't look at what exactly changed to cause the problem, sorry.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ebba10fb085b8b01e2faa64523bed3e5373f8b5e

commit ebba10fb085b8b01e2faa64523bed3e5373f8b5e
Author: rch <rch@chromium.org>
Date: Sun May 28 04:24:08 2017

Change CryptoHandshakeMessage::GetTaglist to tag a QuicTagVector*
instead of a QuicTag** to avoid unaligned read problems on arm.

Merge internal change: 157281990

BUG= 726137 

Review-Url: https://codereview.chromium.org/2907743003
Cr-Commit-Position: refs/heads/master@{#475255}

[modify] https://crrev.com/ebba10fb085b8b01e2faa64523bed3e5373f8b5e/net/quic/core/crypto/crypto_handshake_message.cc
[modify] https://crrev.com/ebba10fb085b8b01e2faa64523bed3e5373f8b5e/net/quic/core/crypto/crypto_handshake_message.h
[modify] https://crrev.com/ebba10fb085b8b01e2faa64523bed3e5373f8b5e/net/quic/core/crypto/crypto_server_test.cc
[modify] https://crrev.com/ebba10fb085b8b01e2faa64523bed3e5373f8b5e/net/quic/core/crypto/crypto_utils.cc
[modify] https://crrev.com/ebba10fb085b8b01e2faa64523bed3e5373f8b5e/net/quic/core/crypto/quic_crypto_client_config.cc
[modify] https://crrev.com/ebba10fb085b8b01e2faa64523bed3e5373f8b5e/net/quic/core/crypto/quic_crypto_server_config.cc
[modify] https://crrev.com/ebba10fb085b8b01e2faa64523bed3e5373f8b5e/net/quic/core/crypto/quic_crypto_server_config_test.cc
[modify] https://crrev.com/ebba10fb085b8b01e2faa64523bed3e5373f8b5e/net/quic/core/quic_config.cc
[modify] https://crrev.com/ebba10fb085b8b01e2faa64523bed3e5373f8b5e/net/quic/core/quic_crypto_client_stream.cc
[modify] https://crrev.com/ebba10fb085b8b01e2faa64523bed3e5373f8b5e/net/quic/core/quic_crypto_server_stream.cc
[modify] https://crrev.com/ebba10fb085b8b01e2faa64523bed3e5373f8b5e/net/tools/quic/stateless_rejector_test.cc