[test][tsan] Remove weak, they are from headers now

[vitalybuka]

@rsundahl Do you thing it's going to work with D146469 ?

[dvyukov]

Remove weak, they are from headers now

Should 'weak' be in interface headers?
All user definitions of the callbacks must be non-weak to override our weak definitions, and the interface header is for users.

[dvyukov]

In D146471#4208808, @dvyukov wrote:

Remove weak, they are from headers now

Should 'weak' be in interface headers?
All user definitions of the callbacks must be non-weak to override our weak definitions, and the interface header is for users.

If we add weak tsan_default_options/tsan_default_suppressions it may break existing users who included the header and provided non-weak versions. Now they suddenly define weak ones that don't override ours.

[rsundahl]

In D146471#4207971, @vitalybuka wrote:

@rsundahl Do you thing it's going to work with D146469 ?

Thanks for including me @vitalybuka... To understand why these tests have the conditional code making the overrides "weak" please see https://reviews.llvm.org/D127929 (show older changes) for the discussion. To summarize here though:
The idea of a "strong" overriding "weak" symbols is reasonably well understood and iirc is specified by the standard(s) but only in the context of static linking. The problem is complicated with dynamic linking. For a worst case thought experience, think about what should happen to weakly bound symbols when an application calls dlopen() on a dll that contains possible overrides or additional weak symbols. dyld uses two-level name space linking which essentially precludes a strong symbol in a different module from ever replacing a weak symbol, but it has a slight variation on this behavior in that during this "weak-def coalescing", it does consider any other modules containing weak definitions. The weak attributes on the "overrides" in the tests above are there only to cause the module (the test program) to be considered during weak-def coalescing at which time the weak symbol in the test program overrides the weak symbol reference from the dll. Yes, in this case it's a weak overriding a weak but with dyld the way to think of it is that if a symbol is marked weak, then all definitions of that symbol are treated as weak during weak-def coalescing. It would be just as effective to define a dummy unused weak symbol in the test program:

__attribute__((weak)) int foo;

This too would mark the dll as having a weak symbol and so then all the symbols in the test program would be seen during weak-def coalescing.

[rsundahl]

In D146471#4207971, @vitalybuka wrote:

@rsundahl Do you thing it's going to work with D146469 ?

I'm not sure exactly what you're asking vis-a-vis https://reviews.llvm.org/D146469 @vitalybuka but I think it's a subtlety. Could you elaborate? I'm adding @thetruestblue as she has been working recently on these interceptors and may spot something that I don't.

[rsundahl]

In D146471#4207971, @vitalybuka wrote:

@rsundahl Do you thing it's going to work with D146469 ?

Thanks for including me @vitalybuka... To understand why these tests have the conditional code making the overrides "weak" please see https://reviews.llvm.org/D127929 (show older changes) for the discussion. To summarize here though:
The idea of a "strong" overriding "weak" symbols is reasonably well understood and iirc is specified by the standard(s) but only in the context of static linking. The problem is complicated with dynamic linking. For a worst case thought experience, think about what should happen to weakly bound symbols when an application calls dlopen() on a dll that contains possible overrides or additional weak symbols. dyld uses two-level name space linking which essentially precludes a strong symbol in a different module from ever replacing a weak symbol, but it has a slight variation on this behavior in that during this "weak-def coalescing", it does consider any other modules containing weak definitions. The weak attributes on the "overrides" in the tests above are there only to cause the module (the test program) to be considered during weak-def coalescing at which time the weak symbol in the test program overrides the weak symbol reference from the dll. Yes, in this case it's a weak overriding a weak but with dyld the way to think of it is that if a symbol is marked weak, then all definitions of that symbol are treated as weak during weak-def coalescing. It would be just as effective to define a dummy unused weak symbol in the test program:

__attribute__((weak)) int foo;

This too would mark the dll as having a weak symbol and so then all the symbols in the test program would be seen during weak-def coalescing.

compiler-rt/test/tsan/Darwin/debug_external.cpp

int main() {
   pthread_create(&t, 0, Thread, obj);
   *obj = 41;
   barrier_wait(&barrier);
   pthread_join(t, 0);
   fprintf(stderr, "Done.\n");
   return 0;
 }
 
-// Required for dyld macOS 12.0+
-#if (__APPLE__)
-__attribute__((weak))
-#endif
 __attribute__((disable_sanitizer_instrumentation))
 extern "C" void
 __tsan_on_report(void *report) {
   const char *type;
   void *addr;
   void *start;
   unsigned long size;
   int tid, fd, suppressable;

compiler-rt/test/tsan/Darwin/main_tid.mm

 // RUN: %clang_tsan %s -o %t -framework Foundation
 // RUN: %deflake %run %t 2>&1 | FileCheck %s
 
 #import <pthread.h>
 #import <stdio.h>
 #import <stdlib.h>
 
 extern "C" {
 void __tsan_on_report(void *report);
 int __tsan_get_report_thread(void *report, unsigned long idx, int *tid,
                              uint64_t *os_id, int *running,
                              const char **name, int *parent_tid, void **trace,
                              unsigned long trace_size);
 }
 
-// Required for dyld macOS 12.0+
-#if (__APPLE__)
-__attribute__((weak))
-#endif
 __attribute__((disable_sanitizer_instrumentation))
 extern "C" void
 __tsan_on_report(void *report) {
   fprintf(stderr, "__tsan_on_report(%p)\n", report);
 
   int tid;
   uint64_t os_id;
   int running;

compiler-rt/test/tsan/cxa_guard_acquire.cpp

 // RUN: %clangxx_tsan -O1 %s -o %t && %run %t 2>&1 | FileCheck %s
 
 #include <sanitizer/tsan_interface.h>
 #include <stdio.h>
 
 namespace __tsan {
 
-#if (__APPLE__)
-__attribute__((weak))
-#endif
 void OnPotentiallyBlockingRegionBegin() {
   printf("Enter __cxa_guard_acquire\n");
 }
 
-#if (__APPLE__)
-__attribute__((weak))
-#endif
 void OnPotentiallyBlockingRegionEnd() { printf("Exit __cxa_guard_acquire\n"); }
 
 } // namespace __tsan
 
 int main(int argc, char **argv) {
   // CHECK: Enter main
   printf("Enter main\n");
   // CHECK-NEXT: Enter __cxa_guard_acquire
   // CHECK-NEXT: Exit __cxa_guard_acquire
   static int s = argc;
   (void)s;
   // CHECK-NEXT: Exit main
   printf("Exit main\n");
   return 0;
 }

compiler-rt/test/tsan/debugging.cpp

int main() {
   pthread_t t;
   pthread_create(&t, 0, Thread, 0);
   my_global = 41;
   barrier_wait(&barrier);
   pthread_join(t, 0);
   fprintf(stderr, "Done.\n");
 }
 
-// Required for dyld macOS 12.0+
-#if (__APPLE__)
-__attribute__((weak))
-#endif
 __attribute__((disable_sanitizer_instrumentation))
 extern "C" void
 __tsan_on_report(void *report) {
   fprintf(stderr, "__tsan_on_report(%p)\n", report);
   fprintf(stderr, "__tsan_get_current_report() = %p\n",
           __tsan_get_current_report());
   // CHECK: __tsan_on_report([[REPORT:0x[0-9a-f]+]])
   // CHECK: __tsan_get_current_report() = [[REPORT]]

compiler-rt/test/tsan/default_options.cpp

 // RUN: %clang_tsan -O1 %s -o %t && %run %t 2>&1 | FileCheck %s
 #include <pthread.h>
 #include <sanitizer/tsan_interface.h>
 #include <stdio.h>
 
-#if (__APPLE__)
-__attribute__((weak))
-#endif
 extern "C" const char *__tsan_default_options() {
   return "report_bugs=0";
 }
 
 int Global;
 
 void *Thread1(void *x) {
   Global = 42;

compiler-rt/test/tsan/java_symbolization.cpp

 // RUN: %clangxx_tsan -O1 %s -o %t && %deflake %run %t | FileCheck %s
 #include "java.h"
 #include <memory.h>
 #include <sanitizer/tsan_interface.h>
 
-#if (__APPLE__)
-__attribute__((weak)) // Required for dyld macOS 12.0+
-#endif
 __attribute__((disable_sanitizer_instrumentation))
 extern "C" void
 __tsan_symbolize_external_ex(jptr pc,
                              void (*add_frame)(void *, const char *,
                                                const char *, int, int),
                              void *ctx) {
   if (pc == (1234 | kExternalPCBit)) {
     add_frame(ctx, "MyInnerFunc", "MyInnerFile.java", 1234, 56);

compiler-rt/test/tsan/java_symbolization_legacy.cpp

 // RUN: %clangxx_tsan -O1 %s -o %t && %deflake %run %t | FileCheck %s
 #include "java.h"
 #include <memory.h>
 #include <sanitizer/tsan_interface.h>
 
-#if (__APPLE__)
-__attribute__((weak)) // Required for dyld macOS 12.0+
-#endif
 __attribute__((disable_sanitizer_instrumentation))
 extern "C" bool
 __tsan_symbolize_external(jptr pc, char *func_buf, jptr func_siz,
                           char *file_buf, jptr file_siz, int *line, int *col) {
   if (pc == (1234 | kExternalPCBit)) {
     memcpy(func_buf, "MyFunc", sizeof("MyFunc"));
     memcpy(file_buf, "MyFile.java", sizeof("MyFile.java"));
     *line = 1234;