[sanitizer] Relax the restriction on SizeClassAllocator64::kAllocatorSize

[MaskRay]

Summary

Commit 278ccdacdcbde3399f1fa4b3ab929212e4e0322c says that kAllocatorSize
must be >= (1<<32), but this is not accurate. This static_assert causes 128GiB
kAllocatorSize to be unable to select DefaultSizeClassMap (kRegionSize is
1<<31).

Relax the restriction to be able to satisfy the largest size class. This allows
DefaultSizeClassMap to be usable with 128GiB kAllocatorSize, with
check-{asan,lsan,sanitizer} passing.

[aganea]

compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h

639

@MaskRay I was seeing this when building on Windows with ToT:

C:\git\llvm-project>ninja check-asan -C stage1_msvc
ninja: Entering directory `stage1_msvc'
[33/56] Generating ASAN_NOINST_TEST_OBJECTS.asan_noinst_test.cpp.x86_64-inline.o
In file included from C:/git/llvm-project/compiler-rt/lib/asan/tests/asan_noinst_test.cpp:24:
In file included from C:/git/llvm-project/compiler-rt/lib/asan\asan_allocator.h:20:
In file included from C:/git/llvm-project/compiler-rt/lib\sanitizer_common/sanitizer_allocator.h:74:
C:/git/llvm-project/compiler-rt/lib\sanitizer_common\sanitizer_allocator_primary64.h:639:54: warning: 'static_assert' with no message is a C++17 extension [-Wc++17-extensions]
  639 |   static_assert(kRegionSize >= SizeClassMap::kMaxSize);
      |                                                      ^
      |                                                      , ""
1 warning generated.

I fixed it in https://reviews.llvm.org/rGd58fb40670ea

[vitalybuka]

Looks unnececary
https://llvm.org/docs/CodingStandards.html#id3

[aganea]

In D153664#4652461, @vitalybuka wrote:

Looks unnececary
https://llvm.org/docs/CodingStandards.html#id3

static_assert with no message requires --std=c++17 which is not present for this test object. Otherwise we could add that here. The build command for that object is currently:

COMMAND = cmd.exe /C "cd /D C:\git\llvm-project\stage1_msvc\projects\compiler-rt\lib\asan\tests && C:\git\llvm-project\stage1_msvc\.\bin\clang.exe -DGTEST_NO_LLVM_SUPPORT=1 -DGTEST_HAS_RTTI=0 -IC:/git/llvm-project/llvm/../third-party/unittest/googletest/include -IC:/git/llvm-project/llvm/../third-party/unittest/googletest -Wno-deprecated-declarations -IC:/git/llvm-project/compiler-rt/include -IC:/git/llvm-project/compiler-rt/lib -IC:/git/llvm-project/compiler-rt/lib/asan -IC:/git/llvm-project/compiler-rt/lib/sanitizer_common/tests -DSANITIZER_COMMON_NO_REDEFINE_BUILTINS -fno-rtti -O2 -Wno-format -Werror=sign-compare -gline-tables-only -gcodeview -DASAN_HAS_IGNORELIST=1 -DASAN_HAS_EXCEPTIONS=1 -DASAN_UAR=0 -c -o ASAN_NOINST_TEST_OBJECTS.asan_noinst_test.cpp.x86_64-inline.o C:/git/llvm-project/compiler-rt/lib/asan/tests/asan_noinst_test.cpp"

[vitalybuka]

In D153664#4652465, @aganea wrote:

In D153664#4652461, @vitalybuka wrote:

Looks unnececary
https://llvm.org/docs/CodingStandards.html#id3

static_assert with no message requires --std=c++17 which is not present for this test object. Otherwise we could add that here. The build command for that object is currently:

COMMAND = cmd.exe /C "cd /D C:\git\llvm-project\stage1_msvc\projects\compiler-rt\lib\asan\tests && C:\git\llvm-project\stage1_msvc\.\bin\clang.exe -DGTEST_NO_LLVM_SUPPORT=1 -DGTEST_HAS_RTTI=0 -IC:/git/llvm-project/llvm/../third-party/unittest/googletest/include -IC:/git/llvm-project/llvm/../third-party/unittest/googletest -Wno-deprecated-declarations -IC:/git/llvm-project/compiler-rt/include -IC:/git/llvm-project/compiler-rt/lib -IC:/git/llvm-project/compiler-rt/lib/asan -IC:/git/llvm-project/compiler-rt/lib/sanitizer_common/tests -DSANITIZER_COMMON_NO_REDEFINE_BUILTINS -fno-rtti -O2 -Wno-format -Werror=sign-compare -gline-tables-only -gcodeview -DASAN_HAS_IGNORELIST=1 -DASAN_HAS_EXCEPTIONS=1 -DASAN_UAR=0 -c -o ASAN_NOINST_TEST_OBJECTS.asan_noinst_test.cpp.x86_64-inline.o C:/git/llvm-project/compiler-rt/lib/asan/tests/asan_noinst_test.cpp"

Yes, looks like cmake issue, but I am fine either way. We can let cmake fix to those who will need more important c++17 features.

[MaskRay]

In D153664#4652466, @vitalybuka wrote:

In D153664#4652465, @aganea wrote:

In D153664#4652461, @vitalybuka wrote:

Looks unnececary
https://llvm.org/docs/CodingStandards.html#id3

static_assert with no message requires --std=c++17 which is not present for this test object. Otherwise we could add that here. The build command for that object is currently:

COMMAND = cmd.exe /C "cd /D C:\git\llvm-project\stage1_msvc\projects\compiler-rt\lib\asan\tests && C:\git\llvm-project\stage1_msvc\.\bin\clang.exe -DGTEST_NO_LLVM_SUPPORT=1 -DGTEST_HAS_RTTI=0 -IC:/git/llvm-project/llvm/../third-party/unittest/googletest/include -IC:/git/llvm-project/llvm/../third-party/unittest/googletest -Wno-deprecated-declarations -IC:/git/llvm-project/compiler-rt/include -IC:/git/llvm-project/compiler-rt/lib -IC:/git/llvm-project/compiler-rt/lib/asan -IC:/git/llvm-project/compiler-rt/lib/sanitizer_common/tests -DSANITIZER_COMMON_NO_REDEFINE_BUILTINS -fno-rtti -O2 -Wno-format -Werror=sign-compare -gline-tables-only -gcodeview -DASAN_HAS_IGNORELIST=1 -DASAN_HAS_EXCEPTIONS=1 -DASAN_UAR=0 -c -o ASAN_NOINST_TEST_OBJECTS.asan_noinst_test.cpp.x86_64-inline.o C:/git/llvm-project/compiler-rt/lib/asan/tests/asan_noinst_test.cpp"

Yes, looks like cmake issue, but I am fine either way. We can let cmake fix to those who will need more important c++17 features.

@aganea https://reviews.llvm.org/rGd58fb40670ea looks fine to me, though I'd hope that we can fix the CMake issue: we pass -std=c++17 to other files but somehow lose -std=c++17 for ASAN_NOINST_TEST_OBJECTS.asan_noinst_test.cpp.x86_64-inline.o?
The current CMake issue would cause C++14 vs C++17 differences depending on the host compiler's default -std=.

compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h

private:
   static const uptr kFreeArraySize = kRegionSize / 8;
 
   static const bool kUsingConstantSpaceBeg = kSpaceBeg != ~(uptr)0;
   uptr NonConstSpaceBeg;
   uptr SpaceBeg() const {
     return kUsingConstantSpaceBeg ? kSpaceBeg : NonConstSpaceBeg;
   }
   uptr SpaceEnd() const { return  SpaceBeg() + kSpaceSize; }
-  // kRegionSize must be >= 2^32.
-  COMPILER_CHECK((kRegionSize) >= (1ULL << (SANITIZER_WORDSIZE / 2)));
+  // kRegionSize should be able to satisfy the largest size class.
+  static_assert(kRegionSize >= SizeClassMap::kMaxSize);
   // kRegionSize must be <= 2^36, see CompactPtrT.
   COMPILER_CHECK((kRegionSize) <= (1ULL << (SANITIZER_WORDSIZE / 2 + 4)));
   // Call mmap for user memory with at least this size.
   static const uptr kUserMapSize = 1 << 16;
   // Call mmap for metadata memory with at least this size.
   static const uptr kMetaMapSize = 1 << 16;
   // Call mmap for free array memory with at least this size.
   static const uptr kFreeArrayMapSize = 1 << 16;