[llvm-branch-commits] [clang] [clang-tools-extra] [llvm] [compiler-rt] [sanitizer_symbolizer] Add end to end test for symbolizer markup. (PR #77702)

2024-01-11 Thread Paul Kirth via llvm-branch-commits
=?utf-8?q?Andrés?= Villegas ,
=?utf-8?q?Andrés?= Villegas ,
=?utf-8?q?Andrés?= Villegas 
Message-ID:
In-Reply-To: 



@@ -0,0 +1,58 @@
+// REQUIRES: linux
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/.build-id/12
+// RUN: %clangxx_tsan %s -Wl,--build-id=0x12345678 -O1 -o %t/main
+// RUN: cp %t/main %t/.build-id/12/345678.debug
+// RUN: %env_tsan_opts=enable_symbolizer_markup=1 %deflake %run %t/main 
>%t/sanitizer.out
+// RUN: llvm-symbolizer --filter-markup --debug-file-directory=%t < 
%t/sanitizer.out | FileCheck %s
+
+#include "test.h"
+
+int Global;
+
+[[gnu::noinline]] void foo1() { Global = 42; }
+
+[[gnu::noinline]] void bar1() {
+  volatile int tmp = 42;
+  int tmp2 = tmp;
+  (void)tmp2;
+  foo1();
+}
+
+[[gnu::noinline]] void foo2() {
+  volatile int tmp = Global;
+  int tmp2 = tmp;
+  (void)tmp2;
+}
+
+[[gnu::noinline]] void bar2() {
+  volatile int tmp = 42;
+  int tmp2 = tmp;
+  (void)tmp2;
+  foo2();
+}
+
+void *Thread1(void *x) {
+  barrier_wait();
+  bar1();
+  return NULL;
+}
+
+int main() {
+  barrier_init(, 2);
+  pthread_t t;
+  pthread_create(, NULL, Thread1, NULL);
+  bar2();
+  barrier_wait();
+  pthread_join(t, NULL);
+}
+
+//  CHECK: WARNING: ThreadSanitizer: data race
+//  CHECK:   Write of size 4 at {{.*}} by thread T1:
+//  CHECK-DAG: #0 {{0x.*}} foo1{{.*}} 
{{.*}}simple_stack_symbolizer_markup.cpp:[[#@LINE - 39]]
+// CHECK-NEXT: #1 {{0x.*}} bar1{{.*}} 
{{.*}}simple_stack_symbolizer_markup.cpp:[[#@LINE - 34]]
+// CHECK-NEXT: #2 {{0x.*}} Thread1{{.*}} 
{{.*}}simple_stack_symbolizer_markup.cpp:[[#@LINE - 17]]
+//  CHECK-DAG:   Previous read of size 4 at {{.*}} by main thread:
+//  CHECK-DAG: #0 {{0x.*}} foo2{{.*}} 
{{.*}}simple_stack_symbolizer_markup.cpp:[[#@LINE - 33]]
+// CHECK-NEXT: #1 {{0x.*}} bar2{{.*}} 
{{.*}}simple_stack_symbolizer_markup.cpp:[[#@LINE - 25]]

ilovepi wrote:

matching the name is a good idea, since they should be all the same, and 
matching it will make the test robust to renaming. 

https://github.com/llvm/llvm-project/pull/77702
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [clang] [clang-tools-extra] [llvm] [compiler-rt] [sanitizer_symbolizer] Add end to end test for symbolizer markup. (PR #77702)

2024-01-11 Thread Paul Kirth via llvm-branch-commits
=?utf-8?q?Andrés?= Villegas ,
=?utf-8?q?Andrés?= Villegas ,
=?utf-8?q?Andrés?= Villegas 
Message-ID:
In-Reply-To: 



@@ -0,0 +1,58 @@
+// REQUIRES: linux
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/.build-id/12
+// RUN: %clangxx_tsan %s -Wl,--build-id=0x12345678 -O1 -o %t/main
+// RUN: cp %t/main %t/.build-id/12/345678.debug
+// RUN: %env_tsan_opts=enable_symbolizer_markup=1 %deflake %run %t/main 
>%t/sanitizer.out
+// RUN: llvm-symbolizer --filter-markup --debug-file-directory=%t < 
%t/sanitizer.out | FileCheck %s
+
+#include "test.h"
+
+int Global;
+
+[[gnu::noinline]] void foo1() { Global = 42; }
+
+[[gnu::noinline]] void bar1() {
+  volatile int tmp = 42;
+  int tmp2 = tmp;
+  (void)tmp2;
+  foo1();
+}
+
+[[gnu::noinline]] void foo2() {
+  volatile int tmp = Global;
+  int tmp2 = tmp;
+  (void)tmp2;
+}
+
+[[gnu::noinline]] void bar2() {
+  volatile int tmp = 42;
+  int tmp2 = tmp;
+  (void)tmp2;
+  foo2();
+}
+
+void *Thread1(void *x) {
+  barrier_wait();
+  bar1();
+  return NULL;
+}
+
+int main() {
+  barrier_init(, 2);
+  pthread_t t;
+  pthread_create(, NULL, Thread1, NULL);
+  bar2();
+  barrier_wait();
+  pthread_join(t, NULL);
+}
+
+//  CHECK: WARNING: ThreadSanitizer: data race
+//  CHECK:   Write of size 4 at {{.*}} by thread T1:
+//  CHECK-DAG: #0 {{0x.*}} foo1{{.*}} 
{{.*}}simple_stack_symbolizer_markup.cpp:[[#@LINE - 39]]

ilovepi wrote:




> `{{.*}}simple_stack_symbolizer_markup.cpp` is not for name mangling or 
> matching spaces. It is to avoid having to match the whole file path. The 
> markup filter uses absolute paths when printing file names.

You can probably omit the filename, since you're going to match the LINE. That 
is a pretty common approach, but I don't feel strongly one way or another on 
that. So its up to you.

> `foo1{{.*}}` is necessary because depending on some flags it is possible that 
> `foo1` is printed slightly different, so I am trying to make the test less 
> fragile.

If there is different output based on flags, its probably worth another test 
with and without those flags. Well, unless that's tested elsewhere. But even if 
that's the case, you should probably just set one of those flags to make it 
print the way you expect, and add a comment about why you only want to test the 
single config here.

> I considered `0x[[%.8X]]` but was worried that not all targets might print 8 
> byte addresses and went with the less specific version and try to match 
> anything after the `0x`, Do you think this might be a problem?

well, you can just match `0x[[#]]` if its a problem, but this is only supported 
on Linux, right? Its probably OK to assume 64bit addresses in that case. I'm 
fine either way.


https://github.com/llvm/llvm-project/pull/77702
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [clang] [clang-tools-extra] [llvm] [compiler-rt] [sanitizer_symbolizer] Add end to end test for symbolizer markup. (PR #77702)

2024-01-11 Thread Andres Villegas via llvm-branch-commits
=?utf-8?q?Andrés?= Villegas ,
=?utf-8?q?Andrés?= Villegas ,
=?utf-8?q?Andrés?= Villegas 
Message-ID:
In-Reply-To: 



@@ -0,0 +1,58 @@
+// REQUIRES: linux
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/.build-id/12
+// RUN: %clangxx_tsan %s -Wl,--build-id=0x12345678 -O1 -o %t/main
+// RUN: cp %t/main %t/.build-id/12/345678.debug
+// RUN: %env_tsan_opts=enable_symbolizer_markup=1 %deflake %run %t/main 
>%t/sanitizer.out
+// RUN: llvm-symbolizer --filter-markup --debug-file-directory=%t < 
%t/sanitizer.out | FileCheck %s
+
+#include "test.h"
+
+int Global;
+
+[[gnu::noinline]] void foo1() { Global = 42; }
+
+[[gnu::noinline]] void bar1() {
+  volatile int tmp = 42;
+  int tmp2 = tmp;
+  (void)tmp2;
+  foo1();
+}
+
+[[gnu::noinline]] void foo2() {
+  volatile int tmp = Global;
+  int tmp2 = tmp;
+  (void)tmp2;
+}
+
+[[gnu::noinline]] void bar2() {
+  volatile int tmp = 42;
+  int tmp2 = tmp;
+  (void)tmp2;
+  foo2();
+}
+
+void *Thread1(void *x) {
+  barrier_wait();
+  bar1();
+  return NULL;
+}
+
+int main() {
+  barrier_init(, 2);
+  pthread_t t;
+  pthread_create(, NULL, Thread1, NULL);
+  bar2();
+  barrier_wait();
+  pthread_join(t, NULL);
+}
+
+//  CHECK: WARNING: ThreadSanitizer: data race
+//  CHECK:   Write of size 4 at {{.*}} by thread T1:
+//  CHECK-DAG: #0 {{0x.*}} foo1{{.*}} 
{{.*}}simple_stack_symbolizer_markup.cpp:[[#@LINE - 39]]
+// CHECK-NEXT: #1 {{0x.*}} bar1{{.*}} 
{{.*}}simple_stack_symbolizer_markup.cpp:[[#@LINE - 34]]
+// CHECK-NEXT: #2 {{0x.*}} Thread1{{.*}} 
{{.*}}simple_stack_symbolizer_markup.cpp:[[#@LINE - 17]]
+//  CHECK-DAG:   Previous read of size 4 at {{.*}} by main thread:
+//  CHECK-DAG: #0 {{0x.*}} foo2{{.*}} 
{{.*}}simple_stack_symbolizer_markup.cpp:[[#@LINE - 33]]

avillega wrote:

Not necessarily. Contextual elements might get emitted any time a call to print 
a non contextual element happens. So it is possible that in between `Previous 
read of size 4 at {{.*}} by main thread:` and `#0...` some contextual elements 
get printed.

https://github.com/llvm/llvm-project/pull/77702
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [clang] [clang-tools-extra] [llvm] [compiler-rt] [sanitizer_symbolizer] Add end to end test for symbolizer markup. (PR #77702)

2024-01-11 Thread Andres Villegas via llvm-branch-commits
=?utf-8?q?Andrés?= Villegas ,
=?utf-8?q?Andrés?= Villegas ,
=?utf-8?q?Andrés?= Villegas 
Message-ID:
In-Reply-To: 



@@ -0,0 +1,58 @@
+// REQUIRES: linux
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/.build-id/12
+// RUN: %clangxx_tsan %s -Wl,--build-id=0x12345678 -O1 -o %t/main
+// RUN: cp %t/main %t/.build-id/12/345678.debug
+// RUN: %env_tsan_opts=enable_symbolizer_markup=1 %deflake %run %t/main 
>%t/sanitizer.out
+// RUN: llvm-symbolizer --filter-markup --debug-file-directory=%t < 
%t/sanitizer.out | FileCheck %s
+
+#include "test.h"
+
+int Global;
+
+[[gnu::noinline]] void foo1() { Global = 42; }
+
+[[gnu::noinline]] void bar1() {
+  volatile int tmp = 42;
+  int tmp2 = tmp;
+  (void)tmp2;
+  foo1();
+}
+
+[[gnu::noinline]] void foo2() {
+  volatile int tmp = Global;
+  int tmp2 = tmp;
+  (void)tmp2;
+}
+
+[[gnu::noinline]] void bar2() {
+  volatile int tmp = 42;
+  int tmp2 = tmp;
+  (void)tmp2;
+  foo2();
+}
+
+void *Thread1(void *x) {
+  barrier_wait();
+  bar1();
+  return NULL;
+}
+
+int main() {
+  barrier_init(, 2);
+  pthread_t t;
+  pthread_create(, NULL, Thread1, NULL);
+  bar2();
+  barrier_wait();
+  pthread_join(t, NULL);
+}
+
+//  CHECK: WARNING: ThreadSanitizer: data race
+//  CHECK:   Write of size 4 at {{.*}} by thread T1:
+//  CHECK-DAG: #0 {{0x.*}} foo1{{.*}} 
{{.*}}simple_stack_symbolizer_markup.cpp:[[#@LINE - 39]]
+// CHECK-NEXT: #1 {{0x.*}} bar1{{.*}} 
{{.*}}simple_stack_symbolizer_markup.cpp:[[#@LINE - 34]]
+// CHECK-NEXT: #2 {{0x.*}} Thread1{{.*}} 
{{.*}}simple_stack_symbolizer_markup.cpp:[[#@LINE - 17]]
+//  CHECK-DAG:   Previous read of size 4 at {{.*}} by main thread:
+//  CHECK-DAG: #0 {{0x.*}} foo2{{.*}} 
{{.*}}simple_stack_symbolizer_markup.cpp:[[#@LINE - 33]]
+// CHECK-NEXT: #1 {{0x.*}} bar2{{.*}} 
{{.*}}simple_stack_symbolizer_markup.cpp:[[#@LINE - 25]]

avillega wrote:

Given that the file name is added by the markup filter, I think is nice to have 
it. I could probably capture the name and avoid some repetition.

https://github.com/llvm/llvm-project/pull/77702
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits