[compiler-rt] fix and simplify GetMaxVirtualAddress for ppc64*

[willschm]

Summary

Use __builtin_frame_address(0) to calculate the memory layout for power. This works for both PPC64 endian-ness modes, so also eliminates the need for the BIG_ENDIAN/LITTLE_ENDIAN checks.

By trial and error, it also looks like the kPPC64_ShadowOffset64 value is (1ULL << 41) for both BE and LE, so that #if/#elif/#endif block has also been simplified.

[kcc]

I wonder if you can add a powerpc-specific test for this.

lib/sanitizer_common/sanitizer_posix.cc

91

I'd prefer if you don't use builtins in this file as it it is compiled by compilers that don't understand it.
Arguably, such compilers will never have powerpc64 defined, but for consistency still let's still not use builtin here.

There is GET_CURRENT_FRAME in sanitizer_common/sanitizer_internal_defs.h and
MostSignificantSetBitIndex/LeastSignificantSetBitIndex in sanitizer_common/sanitizer_common.h
You probably need MostSignificantSetBitIndex

[willschm]

Reworked to use GET_CURRENT_FRAME and MostSignificantSetBitIndex helper functions per previous comments.

[willschm]

In D6044#5, @kcc wrote:

I wonder if you can add a powerpc-specific test for this.

Maybe. :-)
Is there an existing test that checks for something similar for other platforms? I'm not very familiar with the existing set of tests, so not sure what the options are, or what is practical.

[kcc]

I would add a test similar to test/asan/TestCases/debug_mapping.cc that will use ASAN_OPTIONS=verbosity=1
to check that the mapping is what's expected on the given system.

The test will need the REQUIRES: clause similar to the one below, but for powerpc
// REQUIRES: x86_64-supported-target
( I am not sure what exactly that should be. Alexey?)

lib/sanitizer_common/sanitizer_posix.cc

89

space is missing between '-' and '1'.

[foad]

I can confirm that this works for me on big-endian PPC64 Fedora 19.

But the comments here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55975#c27 suggest that with "ulimit -s unlimited", the stack might not be right up near the 1<<44 or 1<<46 byte limit, so maybe it would be a bit safer to do something like:

if (GET_CURRENT_FRAME() < 1ULL << 44)
  return (1ULL << 44) - 1;
else
  return (1ULL << 46) - 1;

In practice, on my machine, this doesn't seem to be a problem. Even with "ulimit -s unlimited" the stack is still very close to 1<<46.

[willschm]

In D6044#11, @foad wrote:

I can confirm that this works for me on big-endian PPC64 Fedora 19.

But the comments here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55975#c27 suggest that with "ulimit -s unlimited", the stack might not be right up near the 1<<44 or 1<<46 byte limit, so maybe it would be a bit safer to do something like:

if (GET_CURRENT_FRAME() < 1ULL << 44)
  return (1ULL << 44) - 1;
else
  return (1ULL << 46) - 1;

In practice, on my machine, this doesn't seem to be a problem. Even with "ulimit -s unlimited" the stack is still very close to 1<<46.

Right. similar experiments here, on an assortment of systems has not shown that 'ulimit -s unlimited' affects the values coming out of the *frame_address(0) calls.

[foad]

I can also confirm that the asan_mapping.h changes get things working for me on little-endian PPC64 Ubuntu 14.04.

I'm not a reviewer but it LGTM!

[samsonov]

In D6044#9, @kcc wrote:

I would add a test similar to test/asan/TestCases/debug_mapping.cc that will use ASAN_OPTIONS=verbosity=1
to check that the mapping is what's expected on the given system.

The test will need the REQUIRES: clause similar to the one below, but for powerpc
// REQUIRES: x86_64-supported-target
( I am not sure what exactly that should be. Alexey?)

Should be powerpc64-supported-target

[willschm]

Add a testcase that covers validation of address mapping for both old and new kernels.
Also some cosmetic touch-ups per previous feedback.

[kcc]

LGTM

[foad]

Just one more inline comment.

lib/sanitizer_common/sanitizer_posix.cc

85–87

comment should be tweaked or removed

[willschm]

Closed by commit rL221457 (authored by @willschm).

lib/asan/asan_mapping.h

 
 static const u64 kDefaultShadowScale = 3;
 static const u64 kDefaultShadowOffset32 = 1ULL << 29;  // 0x20000000
 static const u64 kIosShadowOffset32 = 1ULL << 30;  // 0x40000000
 static const u64 kDefaultShadowOffset64 = 1ULL << 44;
 static const u64 kDefaultShort64bitShadowOffset = 0x7FFF8000;  // < 2G.
 static const u64 kAArch64_ShadowOffset64 = 1ULL << 36;
 static const u64 kMIPS32_ShadowOffset32 = 0x0aaa8000;
-#if defined(__powerpc64__) && defined(__BIG_ENDIAN__)
 static const u64 kPPC64_ShadowOffset64 = 1ULL << 41;
-#elif defined(__powerpc64__) && defined(__LITTLE_ENDIAN__)
-static const u64 kPPC64_ShadowOffset64 = 1ULL << 43;
-#endif
 static const u64 kFreeBSD_ShadowOffset32 = 1ULL << 30;  // 0x40000000
 static const u64 kFreeBSD_ShadowOffset64 = 1ULL << 46;  // 0x400000000000
 
 #define SHADOW_SCALE kDefaultShadowScale
 #if SANITIZER_ANDROID
 # define SHADOW_OFFSET (0)
 #else
 # if SANITIZER_WORDSIZE == 32

lib/sanitizer_common/sanitizer_posix.cc

 #else
   return 0;
 #endif  // SANITIZER_LINUX
 }
 #endif  // SANITIZER_WORDSIZE == 32
 
 uptr GetMaxVirtualAddress() {
 #if SANITIZER_WORDSIZE == 64
-# if defined(__powerpc64__) && defined(__BIG_ENDIAN__)
+# if defined(__powerpc64__)
   // On PowerPC64 we have two different address space layouts: 44- and 46-bit.
   // We somehow need to figure out which one we are using now and choose
   // one of 0x00000fffffffffffUL and 0x00003fffffffffffUL.
   // Note that with 'ulimit -s unlimited' the stack is moved away from the top
   // of the address space, so simply checking the stack address is not enough.
-  return (1ULL << 44) - 1;  // 0x00000fffffffffffUL
-# elif defined(__powerpc64__) && defined(__LITTLE_ENDIAN__)
-  return (1ULL << 46) - 1;  // 0x00003fffffffffffUL
+  // Using helper macros to determine the frame and handle the bit-shifting.
+  // This should work for both PowerPC64 Endian modes.
+  return (1ULL << (MostSignificantSetBitIndex(GET_CURRENT_FRAME())+1)) -1;
 # elif defined(__aarch64__)
   return (1ULL << 39) - 1;
 # else
   return (1ULL << 47) - 1;  // 0x00007fffffffffffUL;
 # endif
 #else  // SANITIZER_WORDSIZE == 32
   uptr res = (1ULL << 32) - 1;  // 0xffffffff;
   if (!common_flags()->full_address_space)
     res -= GetKernelAreaSize();
   CHECK_LT(reinterpret_cast<uptr>(&res), res);