Incorrect inline assembly

[Quuxplusone]

Bugzilla Link	PR46028
Status	NEW
Importance	P normal
Reported by	Leonard Chan (leonardchan@google.com)
Reported on	2020-05-21 16:08:04 -0700
Last modified on	2020-05-23 06:53:27 -0700
Version	trunk
Hardware	PC All
CC	jpaquette@apple.com, konstantin.schwarz@hightec-rt.com, llvm-bugs@lists.llvm.org, Matthew.Arsenault@amd.com, ndesaulniers@google.com, phosek@chromium.org, quentin.colombet@gmail.com, roland@hack.frob.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

Cobalt on Fuchsia is crashing with the following backtrace:

[559.327] 01780.02271>    #1    0x00000044fb98788c in google::protobuf::internal::NoBarrier_CompareAndSwap(long volatile*, long, long) ./../../third_party/protobuf/src/google/protobuf/stubs/atomicops_internals_arm64_gcc.h:194 <<VMO#1225101=cobalt.cmx>>+0x21588c sp 0xe87e708be0
[559.327] 01780.02271>    #2    0x00000044fb9a6df8 in google::protobuf::internal::Acquire_CompareAndSwap(long volatile*, long, long) ./../../third_party/protobuf/src/google/protobuf/stubs/atomicops_internals_arm64_gcc.h:266 <<VMO#1225101=cobalt.cmx>>+0x234df8 sp 0xe87e708c10
[559.328] 01780.02271>    #3    0x00000044fb9a6d64 in google::protobuf::GoogleOnceInitImpl(long*, google::protobuf::Closure*) ./../../third_party/protobuf/src/google/protobuf/stubs/once.cc:78 <<VMO#1225101=cobalt.cmx>>+0x234d64 sp 0xe87e708c40
[559.328] 01780.02271>    #4    0x00000044fb9812b0 in google::protobuf::GoogleOnceInit(long*, void (*)()) ./../../third_party/protobuf/src/google/protobuf/stubs/once.h:128 <<VMO#1225101=cobalt.cmx>>+0x20f2b0 sp 0xe87e708c70
[559.328] 01780.02271>    #5    0x00000044fb98137c in protobuf_clearcut_2eproto::InitDefaultsLogEvent() ./gen/third_party/cobalt/src/lib/clearcut/clearcut.pb.cc:85 <<VMO#1225101=cobalt.cmx>>+0x20f37c sp 0xe87e708cc0
[559.328] 01780.02271>    #6    0x00000044fb982504 in cobalt::lib::clearcut::LogEvent::default_instance() ./gen/third_party/cobalt/src/lib/clearcut/clearcut.pb.cc:383 <<VMO#1225101=cobalt.cmx>>+0x210504 sp 0xe87e708cd0
[559.328] 01780.02271>    #7    0x00000044fb9b3af8 in void google::protobuf::internal::MessageTypeTraits<cobalt::clearcut_extensions::LogEventExtension>::Register<cobalt::lib::clearcut::LogEvent>(int, unsigned char, bool) ./../../third_party/protobuf/src/google/protobuf/extension_set.h:1065 <<VMO#1225101=cobalt.cmx>>+0x241af8 sp 0xe87e708ce0
[559.328] 01780.02271>    #8    0x00000044fb9b3ac0 in google::protobuf::internal::ExtensionIdentifier<cobalt::lib::clearcut::LogEvent, google::protobuf::internal::MessageTypeTraits<cobalt::clearcut_extensions::LogEventExtension>, (unsigned char)11, false>::Register(int) ./../../third_party/protobuf/src/google/protobuf/extension_set.h:1180 <<VMO#1225101=cobalt.cmx>>+0x241ac0 sp 0xe87e708d20
[559.328] 01780.02271>    #9    0x00000044fb9b3a50 in google::protobuf::internal::ExtensionIdentifier<cobalt::lib::clearcut::LogEvent, google::protobuf::internal::MessageTypeTraits<cobalt::clearcut_extensions::LogEventExtension>, (unsigned char)11, false>::ExtensionIdentifier(int, cobalt::clearcut_extensions::LogEventExtension const&) ./../../third_party/protobuf/src/google/protobuf/extension_set.h:1172 <<VMO#1225101=cobalt.cmx>>+0x241a50 sp 0xe87e708d40
[559.328] 01780.02271>    #10   0x00000044fb9b3a04 in __cxx_global_var_init ./gen/third_party/cobalt/src/pb/clearcut_extensions.pb.cc:317 <<VMO#1225101=cobalt.cmx>>+0x241a04 sp 0xe87e708d70
[559.328] 01780.02271>    #11   0x00000044fb9b3b64 in _GLOBAL__sub_I_clearcut_extensions.pb.cc ./gen/third_party/cobalt/src/pb/clearcut_extensions.pb.cc:0 <<VMO#1225101=cobalt.cmx>>+0x241b64 sp 0xe87e708d90
[559.328] 01780.02271>    #12   0x0000804a7e01ea68 in do_init_fini ./../../zircon/third_party/ulib/musl/ldso/dynlink.c:1549 <libc.so>+0x53a68 sp 0xe87e708da0
[559.329] 01780.02271>    #13   0x0000804a7e027d08 in start_main ./../../zircon/third_party/ulib/musl/src/env/__libc_start_main.c:109 <libc.so>+0x5cd08 sp 0xe87e708f20
[559.329] 01780.02271>    #14   0x0000804a7e027f2c in __libc_start_main ./../../zircon/third_party/ulib/musl/src/env/__libc_start_main.c:164 <libc.so>+0x5cf2c sp 0xe87e709000
[559.329] 01780.02271>    #15   0x00000044fb817014 in _start ./../../zircon/system/ulib/c/Scrt1.cc:7 <<VMO#1225101=cobalt.cmx>>+0xa5014 sp 0x81a9d6cdbff0

The issue seems to point to this inline assembly in protobuf:

inline Atomic64 NoBarrier_CompareAndSwap(volatile Atomic64* ptr,
                                         Atomic64 old_value,
                                         Atomic64 new_value) {
  Atomic64 prev;
  int32_t temp;
  __asm__ __volatile__ (  // NOLINT
    "0:                                    \n\t"
    "ldxr %[prev], %[ptr]                  \n\t"
    "cmp %[prev], %[old_value]             \n\t"
    "bne 1f                                \n\t"
    "stxr %w[temp], %[new_value], %[ptr]   \n\t"
    "cbnz %w[temp], 0b                     \n\t"
    "1:                                    \n\t"
    : [prev]"=&r" (prev),
      [temp]"=&r" (temp),
      [ptr]"+Q" (*ptr)
    : [old_value]"IJr" (old_value),
      [new_value]"r" (new_value)
    : "cc", "memory"
  );  // NOLINT
  return prev;
}

A bisect seems to show commits https://reviews.llvm.org/rG91063cf85a4038537731f582a27936187fb0759c and https://reviews.llvm.org/rG5425cdc3adf9998aeaf587d93417bd2f4f1373c9 both contribute to this crash. Comparing the assembly in a working case vs a broken case:

Working (ToT those commits reverted)

      2c: 0c 7d 5f c8                   ldxr    x12, [x8]
      30: 9f 01 0a eb                   cmp     x12, x10
      34: 61 00 00 54                   b.ne    0x40 <_ZN6google8protobuf8internal24NoBarrier_CompareAndSwapEPVlll+0x40>
      38: 0b 7d 09 c8                   stxr    w9, x11, [x8]
      3c: 89 ff ff 35                   cbnz    w9, 0x2c <_ZN6google8protobuf8internal24NoBarrier_CompareAndSwapEPVlll+0x2c>
      40: ec 0b 00 f9                   str     x12, [sp, #16]
      44: e9 0f 00 b9                   str     w9, [sp, #12]

Broken (ToT)

      2c: 08 7d 5f c8                   ldxr    x8, [x8]
      30: 1f 01 0a eb                   cmp     x8, x10
      34: 61 00 00 54                   b.ne    0x40 <_ZN6google8protobuf8internal24NoBarrier_CompareAndSwapEPVlll+0x40>
      38: 0b 7d 09 c8                   stxr    w9, x11, [x8]
      3c: 89 ff ff 35                   cbnz    w9, 0x2c <_ZN6google8protobuf8internal24NoBarrier_CompareAndSwapEPVlll+0x2c>
      40: e8 0b 00 f9                   str     x8, [sp, #16]
      44: e9 0f 00 b9                   str     w9, [sp, #12]

It seems that the broken case is initially writing over x8 in ldxr x8,[x8] and then reading from a corrupted value in stxr w9, x11, [x8]. The instruction that we also seem to be crashing on is the ldxr x8, [x8], probably because we're loading from a bad value when jumping back to it.

This can be reproduced with clang++ --target=aarch64-fuchsia -O0 /tmp/test.cc. Note that can only reproduce this on AArch64 at -O0 since this should be the only configuration that uses GlobalISel.

[Quuxplusone]

Sorry, typo. Meant to say: "It seems that the broken case is initially writing over x8 in ldxr x8,[x8] and then reading from a corrupted value in that same instruction if we jump back to it from the cbnz."

[Quuxplusone]

The generated code is clear violation of the "earlyclobber" constraint, i.e. the "&" in [prev]"=&r". Logic related to earlyclobber might be involved in what's broken.

[Quuxplusone]

I've posted https://reviews.llvm.org/D80467 for review.
Please let me know if it solves your problem.