[PATCH] D120362: [sanitizer][sancov] Refactor GetNextInstructionPc/GetPreviousInstructionPc

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D120362#3341256 <https://reviews.llvm.org/D120362#3341256>, @uweigand wrote:

> This seems to have caused build bot failures on s390x: https://lab.llvm.org/buildbot/#/builders/94/builds/7935.
>
> The patch changes the logic for s390x -- it now uses 4 when it used to use 1.  4 clearly is wrong - we have instructions of sizes 2, 4, or 6 bytes.  In general it is not possible to determine the "previous" instruction just from a PC value, similar to Intel, so using - 1 seems the best option - at least this results in a value guaranteed to be *within* the previous instruction.
>
> Also, I noticed this bit in `test/tsan/test.h`:
>
>   //The const kPCInc must be in sync with StackTrace::GetPreviousInstructionPc
>
> This now no longer seems to be the case.

Sent D120432 <https://reviews.llvm.org/D120432> to use -1 for s390x.



================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.h:101
   return pc - 2;
-#else
+#elif defined(__i386__) || defined(__x86_64__) || defined(_M_IX86) || defined(_M_X64)
   return pc - 1;
----------------
vitalybuka wrote:
> SANITIZER_I386 || SANITIZER_X64
I'll use `SANITIZER_I386 || SANITIZER_X32 || SANITIZER_X64` to cover x86-32.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120362