[PATCH] D91622: [sanitizer_common][test] Disable CombinedAllocator32Compact etc. on Solaris/sparcv9

[Rainer Orth via Phabricator via llvm-commits]

ro added a comment.

In D91622#2400952 <https://reviews.llvm.org/D91622#2400952>, @vitalybuka wrote:

> Have you tried to change kAddressSpaceSize above?

Not yet: TBH I was quite confused by how target-specific configuration was sprinkled over both the allocator implementation (where I'd expect it) and the allocator test (where I wouldn't) and how the two interact.

I now gave it a try:

- set `SANITIZER_MMAP_RANGE_SIZE` for Solaris/sparcv9 to `FIRST_32_SECOND_64(1ULL << 32, 0xFFFFFFFFFFFFFFFF)`
- keep `SANITIZER_SIGN_EXTENDED_ADDRESSES` as 0 for Solaris/sparcv9 (IIUC this is only needed/correct on Linux/sparc64 which only uses a low part of the 64-bit address space albeit with the VA hole in the middle just like Solaris)
- set `kAddressSpaceSize`, keeping the rest as in the default case:

  +#elif SANITIZER_SOLARIS && defined(__sparcv9)
  +static const uptr kAllocatorSpace = 0x700000000000ULL;
  +static const uptr kAllocatorSize  = 0x010000000000ULL;  // 1T.
  +static const u64 kAddressSpaceSize = 0xFFFFFFFFFFFFFFFF;
  +typedef DefaultSizeClassMap SizeClassMap;

While this allowed `Sanitizer-sparcv9-test` to compile, both affected tests still `FAIL`, though in a different way:

    [ RUN      ] SanitizerCommon.SizeClassAllocator32Compact
  ==15161==Sanitizer CHECK failed: /vol/llvm/src/llvm-project/local/compiler-rt/lib/sanitizer_common/sanitizer_allocator_bytemap.h:69 ((idx)) < ((kSize1 * kSize2)) (1099511627568, 1099511623680)
  
  [ RUN      ] SanitizerCommon.SizeClassAllocator32Iteration
  ==15162==Sanitizer CHECK failed: /vol/llvm/src/llvm-project/local/compiler-rt/lib/sanitizer_common/sanitizer_allocator_bytemap.h:69 ((idx)) < ((kSize1 * kSize2)) (1099511627644, 1099511623680)





================
Comment at: compiler-rt/lib/sanitizer_common/tests/sanitizer_allocator_test.cpp:942
 
+#if !(SANITIZER_SOLARIS && defined(__sparcv9))
 TEST(SanitizerCommon, SizeClassAllocator32Iteration) {
----------------
vitalybuka wrote:
> Better approach is to use:
> ```
> #if defined(_MSC_VER) && defined(_DLL)
> #define MAYBE_StrDupOOBTest DISABLED_StrDupOOBTest
> #else
> #define MAYBE_StrDupOOBTest StrDupOOBTest
> #endif
> 
> TEST(AddressSanitizer, MAYBE_StrDupOOBTest) {
> ```
> this way the test at least compiles on the platform.
> 
> But it's going to make the file inconsistent.
> So you like you can update the file with MAYBE_ in a separate patch.
> But feel free to keep as-is.
> 
> 
> 
> 
While I like this approach (I'm effectively doing the same with ASan on SPARC: keep it compiling in `compiler-rt` for the benefit of `gcc` where it works, despite being unusable with `clang` due to a compiler bug/limitation).

However, that's a considerable change and I'm running out of time for LLVM work (GCC just entered stage3 for the GCC 11 release and I'll soon will have to switch attention there), so I'd like to keep my patch as is at least for the moment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91622