[PATCH] D46661: [tsan] Add debugging API to retrieve the "external tag" from reports

Dan Liew via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 10 14:11:51 PDT 2018


delcypher added inline comments.


================
Comment at: test/tsan/Darwin/external-swift-debugging.cc:18
+
+static void *tag = (void *)0x1;
+
----------------
Why does `tag` need to be a global here? Can't it just be a literal in `ExternalWrite` which is the only place its used.


================
Comment at: test/tsan/Darwin/external-swift-debugging.cc:39
+  });
+  // CHECK: WARNING: ThreadSanitizer: Swift access race
+  // CHECK: Modifying access of Swift variable at {{.*}} by thread {{.*}}
----------------
kubamracek wrote:
> delcypher wrote:
> > I don't understand this. There's no Swift code in this test case so how is this a Swift access race?
> The test is faking what a Swift code would do via the `__tsan_external_write` call.
This is still incredibly unclear. Why when using `__tsan_external_write`

- Is it okay to pass nullptr as `caller_pc`?
- The fact that `0x1` means a swift race is not all obvious. A bit of grepping shows this is defined in `lib/tsan/rtl/tsan_defs.h` but this is not at all obvious.

Please add some comments to `ExternalWrite` to explain what is going on (i.e. that you're faking a swift access will a special tag that is defined in `lib/tsan/rtl/tsan_defs.h` ).


================
Comment at: test/tsan/Darwin/external-swift-debugging.cc:62
+
+  unsigned long tag;
+  __tsan_get_report_tag(report, &tag);
----------------
This variable is hiding the `tag` global variable of the same name. Removing the global variable will fix this.


https://reviews.llvm.org/D46661





More information about the llvm-commits mailing list