[nsan] Swap alignas and visibility order (NFC)

[nikic]

Use alignas(16) SANITIZER_INTERFACE_ATTRIBUTE instead of SANITIZER_INTERFACE_ATTRIBUTE alignas(16), as the former is not supported prior to clang 16. See https://clang.godbolt.org/z/Wj1193xWK.

This was broken by #96142 as part of other style changes.

[llvmbot]

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Nikita Popov (nikic)

Changes

When preceded by SANITIZER_INTERFACE_ATTRIBUTE, use the ALIGNED macro instead of alignas, because clang 15 and older do not support this. See https://clang.godbolt.org/z/Wj1193xWK.

This was broken by #96142 as part of other style changes.

---

Full diff: https://github.com/llvm/llvm-project/pull/98933.diff

1 Files Affected:

• (modified) compiler-rt/lib/nsan/nsan.cpp (+7-7)

diff --git a/compiler-rt/lib/nsan/nsan.cpp b/compiler-rt/lib/nsan/nsan.cpp
index e5a9b7a036e0e..10010495e3ee9 100644
--- a/compiler-rt/lib/nsan/nsan.cpp
+++ b/compiler-rt/lib/nsan/nsan.cpp
@@ -391,23 +391,23 @@ __nsan_dump_shadow_mem(const u8 *addr, size_t size_bytes, size_t bytes_per_line,
 }
 
 SANITIZER_INTERFACE_ATTRIBUTE
-alignas(16) thread_local uptr __nsan_shadow_ret_tag = 0;
+ALIGNED(16) thread_local uptr __nsan_shadow_ret_tag = 0;
 
 SANITIZER_INTERFACE_ATTRIBUTE
-alignas(16) thread_local char __nsan_shadow_ret_ptr[kMaxVectorWidth *
-                                                    sizeof(__float128)];
+ALIGNED(16)
+thread_local char __nsan_shadow_ret_ptr[kMaxVectorWidth * sizeof(__float128)];
 
 SANITIZER_INTERFACE_ATTRIBUTE
-alignas(16) thread_local uptr __nsan_shadow_args_tag = 0;
+ALIGNED(16) thread_local uptr __nsan_shadow_args_tag = 0;
 
 // Maximum number of args. This should be enough for anyone (tm). An alternate
 // scheme is to have the generated code create an alloca and make
 // __nsan_shadow_args_ptr point ot the alloca.
 constexpr const int kMaxNumArgs = 128;
 SANITIZER_INTERFACE_ATTRIBUTE
-alignas(
-    16) thread_local char __nsan_shadow_args_ptr[kMaxVectorWidth * kMaxNumArgs *
-                                                 sizeof(__float128)];
+ALIGNED(16)
+thread_local char __nsan_shadow_args_ptr[kMaxVectorWidth * kMaxNumArgs *
+                                         sizeof(__float128)];
 
 enum ContinuationType { // Keep in sync with instrumentation pass.
   kContinueWithShadow = 0,

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[MaskRay]

We already require support for alignas in some compiler-rt library files: rg -w alignas compiler-rt/lib (lsan, scudo, gwp_asan, xray, etc).
Old compilers can use LLVM_ENABLE_RUNTIMES=compiler-rt, which will use just built Clang to build the runtime libraries.

We could also make COMPILER_RT_HAS_* false when Clang is too old.

[MaskRay]

.

[alexander-shaposhnikov]

cc: @vitalybuka

[nikic]

We already require support for alignas in some compiler-rt library files: rg -w alignas compiler-rt/lib (lsan, scudo, gwp_asan, xray, etc).

The problem here is not alignas in general, but a very specific pattern that only occurs inside nsan. You want to look for git grep -B2 alignas compiler-rt | grep SANITIZER_INTERFACE_ATTRIBUTE.

Old compilers can use LLVM_ENABLE_RUNTIMES=compiler-rt, which will use just built Clang to build the runtime libraries.

As far as I know, compiler-rt is subject to the LLVM monorepo host compiler requirements (https://llvm.org/docs/GettingStarted.html#host-c-toolchain-both-compiler-and-standard-library). Only the libcxx and libcxxabi projects are exempt from these.

compiler-rt is commonly used by compilers other than clang, and the host compiler used to build it has no relation to the toolchain it will be part of. For such uses, only LLVM and compiler-rt will be built, but clang will generally not be built.

In any case, changing the host compiler requirements for compiler-rt would require an RFC.

[MaskRay]

OK, I see. The issue is that SANITIZER_INTERFACE_ATTRIBUTE alignas(16) is not supported by older Clang. Other alignas(...) uses are fine. The problem only affects nsan.

alignas(16) SANITIZER_INTERFACE_ATTRIBUTE seems to work, so adjust the order instead?

[MaskRay]

OK, I see. The issue is that SANITIZER_INTERFACE_ATTRIBUTE alignas(16) is not supported by older Clang. Other alignas(...) uses are fine. The problem only affects nsan.

alignas(16) SANITIZER_INTERFACE_ATTRIBUTE seems to work, so adjust the order instead?

lib/msan/msan.cpp has a similar use. alignas(16) SANITIZER_INTERFACE_ATTRIBUTE works well there. Tested with Clang 7. (Clang 6 libc++ can no longer build llvm-project. Clang 6 with GCC 14 cannot build llvm-project. )

Created #98958 and #98959 to clean up some ALIGNED in compiler-rt.

[nikic]

alignas(16) SANITIZER_INTERFACE_ATTRIBUTE seems to work, so adjust the order instead?

Good point, done!