Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tsan] Replace ALIGNED with alignas #98959

Merged
merged 4 commits into from
Jul 16, 2024

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Jul 15, 2024

Similar to #98958.

Created using spr 1.3.5-bogner

[skip ci]
Created using spr 1.3.5-bogner
@llvmbot
Copy link
Collaborator

llvmbot commented Jul 15, 2024

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

Author: Fangrui Song (MaskRay)

Changes

Similar to #98958.


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

9 Files Affected:

  • (modified) compiler-rt/lib/tsan/rtl/tsan_defs.h (+1-1)
  • (modified) compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp (+2-2)
  • (modified) compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp (+1-1)
  • (modified) compiler-rt/lib/tsan/rtl/tsan_mman.cpp (+2-2)
  • (modified) compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp (+1-1)
  • (modified) compiler-rt/lib/tsan/rtl/tsan_rtl.cpp (+4-5)
  • (modified) compiler-rt/lib/tsan/rtl/tsan_rtl.h (+4-4)
  • (modified) compiler-rt/lib/tsan/rtl/tsan_suppressions.cpp (+1-1)
  • (modified) compiler-rt/lib/tsan/rtl/tsan_vector_clock.h (+1-1)
diff --git a/compiler-rt/lib/tsan/rtl/tsan_defs.h b/compiler-rt/lib/tsan/rtl/tsan_defs.h
index 1ffa3d6aec40b..270d441dc90b7 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_defs.h
+++ b/compiler-rt/lib/tsan/rtl/tsan_defs.h
@@ -30,7 +30,7 @@
 #  define __MM_MALLOC_H
 #  include <emmintrin.h>
 #  include <smmintrin.h>
-#  define VECTOR_ALIGNED ALIGNED(16)
+#  define VECTOR_ALIGNED alignas(16)
 typedef __m128i m128;
 #else
 #  define VECTOR_ALIGNED
diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
index 034ae3d322b56..9cab2a3727128 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
@@ -208,7 +208,7 @@ struct AtExitCtx {
 struct InterceptorContext {
   // The object is 64-byte aligned, because we want hot data to be located
   // in a single cache line if possible (it's accessed in every interceptor).
-  ALIGNED(64) LibIgnore libignore;
+  alignas(64) LibIgnore libignore;
   __sanitizer_sigaction sigactions[kSigCount];
 #if !SANITIZER_APPLE && !SANITIZER_NETBSD
   unsigned finalize_key;
@@ -220,7 +220,7 @@ struct InterceptorContext {
   InterceptorContext() : libignore(LINKER_INITIALIZED), atexit_mu(MutexTypeAtExit), AtExitStack() {}
 };
 
-static ALIGNED(64) char interceptor_placeholder[sizeof(InterceptorContext)];
+alignas(64) static char interceptor_placeholder[sizeof(InterceptorContext)];
 InterceptorContext *interceptor_ctx() {
   return reinterpret_cast<InterceptorContext*>(&interceptor_placeholder[0]);
 }
diff --git a/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp b/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp
index 5154662034c56..befd6a369026d 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp
@@ -76,7 +76,7 @@ struct DynamicAnnContext {
 };
 
 static DynamicAnnContext *dyn_ann_ctx;
-static char dyn_ann_ctx_placeholder[sizeof(DynamicAnnContext)] ALIGNED(64);
+alignas(64) static char dyn_ann_ctx_placeholder[sizeof(DynamicAnnContext)];
 
 static void AddExpectRace(ExpectRace *list,
     char *f, int l, uptr addr, uptr size, char *desc) {
diff --git a/compiler-rt/lib/tsan/rtl/tsan_mman.cpp b/compiler-rt/lib/tsan/rtl/tsan_mman.cpp
index e129e9af272f5..0705365d77427 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_mman.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_mman.cpp
@@ -54,7 +54,7 @@ struct MapUnmapCallback {
   }
 };
 
-static char allocator_placeholder[sizeof(Allocator)] ALIGNED(64);
+alignas(64) static char allocator_placeholder[sizeof(Allocator)];
 Allocator *allocator() {
   return reinterpret_cast<Allocator*>(&allocator_placeholder);
 }
@@ -75,7 +75,7 @@ struct GlobalProc {
         internal_alloc_mtx(MutexTypeInternalAlloc) {}
 };
 
-static char global_proc_placeholder[sizeof(GlobalProc)] ALIGNED(64);
+alignas(64) static char global_proc_placeholder[sizeof(GlobalProc)];
 GlobalProc *global_proc() {
   return reinterpret_cast<GlobalProc*>(&global_proc_placeholder);
 }
diff --git a/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp b/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp
index 07d83e1a9a9ff..c8a66e60a69f1 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp
@@ -46,7 +46,7 @@
 namespace __tsan {
 
 #if !SANITIZER_GO
-static char main_thread_state[sizeof(ThreadState)] ALIGNED(
+static char main_thread_state[sizeof(ThreadState)] alignas(
     SANITIZER_CACHE_LINE_SIZE);
 static ThreadState *dead_thread_state;
 static pthread_key_t thread_state_key;
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
index e5ebb65754b32..b5a4abb87ec0b 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
@@ -48,12 +48,11 @@ int (*on_finalize)(int);
 #endif
 
 #if !SANITIZER_GO && !SANITIZER_APPLE
-__attribute__((tls_model("initial-exec")))
-THREADLOCAL char cur_thread_placeholder[sizeof(ThreadState)] ALIGNED(
-    SANITIZER_CACHE_LINE_SIZE);
+alignas(SANITIZER_CACHE_LINE_SIZE) THREADLOCAL __attribute__((tls_model(
+    "initial-exec"))) char cur_thread_placeholder[sizeof(ThreadState)];
 #endif
-static char ctx_placeholder[sizeof(Context)] ALIGNED(SANITIZER_CACHE_LINE_SIZE);
-Context *ctx;
+alignas(SANITIZER_CACHE_LINE_SIZE) static char ctx_placeholder[sizeof(Context)];
+Context* ctx;
 
 // Can be overriden by a front-end.
 #ifdef TSAN_EXTERNAL_HOOKS
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl.h b/compiler-rt/lib/tsan/rtl/tsan_rtl.h
index de4ea0bb5f487..f48be8e0a4fe0 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl.h
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl.h
@@ -136,7 +136,7 @@ struct TidEpoch {
   Epoch epoch;
 };
 
-struct TidSlot {
+struct alignas(SANITIZER_CACHE_LINE_SIZE) TidSlot {
   Mutex mtx;
   Sid sid;
   atomic_uint32_t raw_epoch;
@@ -153,10 +153,10 @@ struct TidSlot {
   }
 
   TidSlot();
-} ALIGNED(SANITIZER_CACHE_LINE_SIZE);
+};
 
 // This struct is stored in TLS.
-struct ThreadState {
+struct alignas(SANITIZER_CACHE_LINE_SIZE) ThreadState {
   FastState fast_state;
   int ignore_sync;
 #if !SANITIZER_GO
@@ -234,7 +234,7 @@ struct ThreadState {
   const ReportDesc *current_report;
 
   explicit ThreadState(Tid tid);
-} ALIGNED(SANITIZER_CACHE_LINE_SIZE);
+};
 
 #if !SANITIZER_GO
 #if SANITIZER_APPLE || SANITIZER_ANDROID
diff --git a/compiler-rt/lib/tsan/rtl/tsan_suppressions.cpp b/compiler-rt/lib/tsan/rtl/tsan_suppressions.cpp
index 70642124990d7..0559df06e7e2e 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_suppressions.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_suppressions.cpp
@@ -42,7 +42,7 @@ const char *__tsan_default_suppressions() {
 
 namespace __tsan {
 
-ALIGNED(64) static char suppression_placeholder[sizeof(SuppressionContext)];
+alignas(64) static char suppression_placeholder[sizeof(SuppressionContext)];
 static SuppressionContext *suppression_ctx = nullptr;
 static const char *kSuppressionTypes[] = {
     kSuppressionRace,   kSuppressionRaceTop, kSuppressionMutex,
diff --git a/compiler-rt/lib/tsan/rtl/tsan_vector_clock.h b/compiler-rt/lib/tsan/rtl/tsan_vector_clock.h
index 63b206302190d..51d98113d8e78 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_vector_clock.h
+++ b/compiler-rt/lib/tsan/rtl/tsan_vector_clock.h
@@ -34,7 +34,7 @@ class VectorClock {
   VectorClock& operator=(const VectorClock& other);
 
  private:
-  Epoch clk_[kThreadSlotCount] VECTOR_ALIGNED;
+  VECTOR_ALIGNED Epoch clk_[kThreadSlotCount];
 };
 
 ALWAYS_INLINE Epoch VectorClock::Get(Sid sid) const {

Created using spr 1.3.5-bogner

[skip ci]
Created using spr 1.3.5-bogner
@MaskRay MaskRay changed the base branch from users/MaskRay/spr/main.tsan-replace-aligned-with-alignas to main July 16, 2024 17:39
@MaskRay MaskRay merged commit 656f617 into main Jul 16, 2024
5 of 6 checks passed
@MaskRay MaskRay deleted the users/MaskRay/spr/tsan-replace-aligned-with-alignas branch July 16, 2024 17:39
@zeroomega
Copy link
Contributor

Hi, we started to see build errors on our mac bots after this patch was landed

Error message:

FAILED: compiler-rt/lib/tsan/rtl/CMakeFiles/clang_rt.tsan_osx_dynamic.dir/tsan_platform_mac.cpp.o 
/Volumes/Work/s/w/ir/x/w/llvm_build/./bin/clang++ --target=x86_64-apple-darwin22.6.0 --sysroot=/Volumes/Work/s/w/ir/cache/macos_sdk/XCode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.3.sdk -DCOMPILER_RT_SHARED_LIB -D_DEBUG -D_GLIBCXX_ASSERTIONS -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Dclang_rt_tsan_osx_dynamic_EXPORTS -I/Volumes/Work/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/tsan/rtl/../.. -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffile-prefix-map=/Volumes/Work/s/w/ir/x/w/llvm_build/runtimes/runtimes-bins=../../../llvm-llvm-project -ffile-prefix-map=/Volumes/Work/s/w/ir/x/w/llvm-llvm-project/= -no-canonical-prefixes -Wall -Wno-unused-parameter -O3 -DNDEBUG -std=c++17 -arch arm64 -arch x86_64 -isysroot /Volumes/Work/s/w/ir/cache/macos_sdk/XCode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.3.sdk -fPIC -stdlib=libc++ -mmacosx-version-min=10.13 -isysroot /Volumes/Work/s/w/ir/cache/macos_sdk/XCode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.3.sdk -fno-lto -fPIC -fno-builtin -fno-exceptions -funwind-tables -fno-stack-protector -fno-sanitize=safe-stack -fvisibility=hidden -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta -O3 -g -Wno-gnu -Wno-variadic-macros -Wno-c99-extensions -ftrivial-auto-var-init=pattern -nostdinc++ -fPIE -fno-rtti -msse4.2 -Wframe-larger-than=530 -Wglobal-constructors -MD -MT compiler-rt/lib/tsan/rtl/CMakeFiles/clang_rt.tsan_osx_dynamic.dir/tsan_platform_mac.cpp.o -MF compiler-rt/lib/tsan/rtl/CMakeFiles/clang_rt.tsan_osx_dynamic.dir/tsan_platform_mac.cpp.o.d -o compiler-rt/lib/tsan/rtl/CMakeFiles/clang_rt.tsan_osx_dynamic.dir/tsan_platform_mac.cpp.o -c /Volumes/Work/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp
/Volumes/Work/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp:49:52: error: 'alignas' attribute cannot be applied to types
   49 | static char main_thread_state[sizeof(ThreadState)] alignas(
      |                                                    ^
/Volumes/Work/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp:252:3: warning: missing field 'start' initializer [-Wmissing-designated-field-initializers]
  252 |   };
      |   ^
1 warning and 1 error generated.

Could you take a look please? If it takes a bit of time could you revert it first?

sayhaan pushed a commit to sayhaan/llvm-project that referenced this pull request Jul 16, 2024
Summary:
Similar to llvm#98958.

Pull Request: llvm#98959

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D59822444
@JDevlieghere
Copy link
Member

@mysterymath
Copy link
Contributor

Reverting due to lab bot breakage.

mysterymath added a commit that referenced this pull request Jul 16, 2024
MaskRay added a commit that referenced this pull request Jul 16, 2024
Similar to #98958.

The relands 656f617 , which
was reverted due to an issue in tsan_platform_mac.cpp

Pull Request: #98959
@MaskRay
Copy link
Member Author

MaskRay commented Jul 16, 2024

Hi, we started to see build errors on our mac bots after this patch was landed

Error message:

FAILED: compiler-rt/lib/tsan/rtl/CMakeFiles/clang_rt.tsan_osx_dynamic.dir/tsan_platform_mac.cpp.o 
/Volumes/Work/s/w/ir/x/w/llvm_build/./bin/clang++ --target=x86_64-apple-darwin22.6.0 --sysroot=/Volumes/Work/s/w/ir/cache/macos_sdk/XCode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.3.sdk -DCOMPILER_RT_SHARED_LIB -D_DEBUG -D_GLIBCXX_ASSERTIONS -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Dclang_rt_tsan_osx_dynamic_EXPORTS -I/Volumes/Work/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/tsan/rtl/../.. -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffile-prefix-map=/Volumes/Work/s/w/ir/x/w/llvm_build/runtimes/runtimes-bins=../../../llvm-llvm-project -ffile-prefix-map=/Volumes/Work/s/w/ir/x/w/llvm-llvm-project/= -no-canonical-prefixes -Wall -Wno-unused-parameter -O3 -DNDEBUG -std=c++17 -arch arm64 -arch x86_64 -isysroot /Volumes/Work/s/w/ir/cache/macos_sdk/XCode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.3.sdk -fPIC -stdlib=libc++ -mmacosx-version-min=10.13 -isysroot /Volumes/Work/s/w/ir/cache/macos_sdk/XCode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.3.sdk -fno-lto -fPIC -fno-builtin -fno-exceptions -funwind-tables -fno-stack-protector -fno-sanitize=safe-stack -fvisibility=hidden -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta -O3 -g -Wno-gnu -Wno-variadic-macros -Wno-c99-extensions -ftrivial-auto-var-init=pattern -nostdinc++ -fPIE -fno-rtti -msse4.2 -Wframe-larger-than=530 -Wglobal-constructors -MD -MT compiler-rt/lib/tsan/rtl/CMakeFiles/clang_rt.tsan_osx_dynamic.dir/tsan_platform_mac.cpp.o -MF compiler-rt/lib/tsan/rtl/CMakeFiles/clang_rt.tsan_osx_dynamic.dir/tsan_platform_mac.cpp.o.d -o compiler-rt/lib/tsan/rtl/CMakeFiles/clang_rt.tsan_osx_dynamic.dir/tsan_platform_mac.cpp.o -c /Volumes/Work/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp
/Volumes/Work/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp:49:52: error: 'alignas' attribute cannot be applied to types
   49 | static char main_thread_state[sizeof(ThreadState)] alignas(
      |                                                    ^
/Volumes/Work/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp:252:3: warning: missing field 'start' initializer [-Wmissing-designated-field-initializers]
  252 |   };
      |   ^
1 warning and 1 error generated.

Could you take a look please? If it takes a bit of time could you revert it first?

Sorry. The tsan_platform_mac.cpp issue was due to the placement of alignas. Unlike __attribute__((align(...))), alignas must precede static.

I've fixed tsan_platform_mac.cpp in a separate commit and reland this PR without modifying tsan_platform_mac.cpp

sgundapa pushed a commit to sgundapa/upstream_effort that referenced this pull request Jul 23, 2024
sgundapa pushed a commit to sgundapa/upstream_effort that referenced this pull request Jul 23, 2024
sgundapa pushed a commit to sgundapa/upstream_effort that referenced this pull request Jul 23, 2024
Similar to llvm#98958.

The relands 656f617 , which
was reverted due to an issue in tsan_platform_mac.cpp

Pull Request: llvm#98959
swift-ci pushed a commit to swiftlang/llvm-project that referenced this pull request Jul 25, 2024
Reverts llvm#98959

Cherry-picked ahead of time to unblock auto-merger due to build failures
rdar://132463432
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary:
Similar to #98958.

Pull Request: #98959

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251655
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary: Reverts #98959

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251556
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary:
Similar to #98958.

The relands 656f617 , which
was reverted due to an issue in tsan_platform_mac.cpp

Pull Request: #98959

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60250841
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants