[PATCH] D72367: Summary: update macro for OFF_T so that sanitizer works on AARCH64.

[Shu-Chun Weng via Phabricator via llvm-commits]

scw added inline comments.


================
Comment at: compiler-rt/lib/sanitizer_common/symbolizer/sanitizer_wrappers.cpp:22
 
 // Need to match ../sanitizer_common/sanitizer_internal_defs.h
+#if defined(__powerpc64__) || defined(__aarch64__)
----------------
eugenis wrote:
> vitalybuka wrote:
> > Do we need to change sanitizer_internal_defs.h
> If anything, we need to change this file.
> But I don't think it can break aarch64, because unsigned long is same width as unsigned long long there.
> 
> This change only affects 32-bit PowerPC.
> Should it say !defined(__x86_64__) instead? That would be closer to how OFF_T is defined in sanitizer_internal_defs.h.
> 
`!defined(x86_64)` is reasonable. Though the exact condition in sanitizer_internal_defs.h is complicated: https://github.com/llvm/llvm-project/blob/5e2f4dc37b1bf72bd27e929a68fec18ae1f5cfa8/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h#L175 plus https://github.com/llvm/llvm-project/blob/5e2f4dc37b1bf72bd27e929a68fec18ae1f5cfa8/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h#L134 for `uptr`.

This change actually should only affect powerpc64 -- ARCH_PPC is an google internally defined macro so this is always false in the wild.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72367