[PATCH] D152407: [AArch64] Merge LDRSWpre-LD[U]RSW pair into LDPSWpre.

Shu-Chun Weng via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 25 15:56:18 PDT 2023


scw added a comment.

We see a miscompilation after this patch in protobuf under MSan. This <https://github.com/protocolbuffers/protobuf/blob/caf55184b2d0e8cbb99e5b487b453dc8721af4fe/src/google/protobuf/repeated_ptr_field.h#L449> is the line where the miscompilation happens, and I extracted it to this godbolt <https://godbolt.org/z/hx47zbe64> with compilation option `-fsanitize=memory -O`.

Before this patch, the AArch64 assembly is (registers are renamed a bit, because this is disassembled from a binary I built internally)

  1bfbbc64: b8808d49      ldrsw   x9, [x10, #8]!
  1bfbbc68: ca0d014e      eor     x14, x10, x13
  1bfbbc6c: b8404d4c      ldr     w12, [x10, #4]!
  1bfbbc70: ca0d014d      eor     x13, x10, x13

After, it became

  1bfbb51c: 29c13149      ldp     w9, w12, [x10, #8]!
  1bfbb520: 93407d4a      sxtw    x10, w10
  1bfbb524: ca0d014e      eor     x14, x10, x13
  1bfbb528: ca0d014d      eor     x13, x10, x13

There are two problems:

1. The after sequence touches `x10`, before calculating `x13` and `x14`, which are supposed to be the addresses of the shadow memory. This caused the crash because our `x10` was pointing to a stack pointer where the 32-th bit is 1. I believe that line should have been `sxtw x9, w9` instead, to perform the extension done in `ldrsw` in the before sequence.
2. This also miscalculate `x13`, where in the before sequence, after execution, it holds `old(x13) xor (x10 + 12)`, yet after the "after" sequence, it holds `old(x13) xor (x10 + 8)`, the same as `x14`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152407/new/

https://reviews.llvm.org/D152407



More information about the llvm-commits mailing list