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

[Paul Kirth via llvm-branch-commits]

=?utf-8?q?Andrés?= Villegas <andresvi at google.com>,
=?utf-8?q?Andrés?= Villegas <andresvi at google.com>,
=?utf-8?q?Andrés?= Villegas <andresvi at google.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/77702 at github.com>


================
@@ -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(&barrier);
+  bar1();
+  return NULL;
+}
+
+int main() {
+  barrier_init(&barrier, 2);
+  pthread_t t;
+  pthread_create(&t, NULL, Thread1, NULL);
+  bar2();
+  barrier_wait(&barrier);
+  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