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

[Kuba (Brecka) Mracek via Phabricator via llvm-commits]

kubamracek added inline comments.


================
Comment at: lib/tsan/rtl/tsan_debugging.cc:89
+  *tag = rep->tag;
+  return 1;
+}
----------------
delcypher wrote:
> Why return 1? What does that mean?
All the APIs return 1 on success, 0 on failure.


================
Comment at: lib/tsan/rtl/tsan_interface.h:122
+SANITIZER_INTERFACE_ATTRIBUTE
+int __tsan_get_report_tag(void *report, uptr *tag);
+
----------------
delcypher wrote:
> There's no documentation on the meaning of the arguments or the return value here.
> 
See comments around. "report" is an opaque pointer representing the report we want to extract data from, "tag" is the output variable. All these APIs return non-zero on success, zero on failure.

Sorry about the lack of formal documentation here, it's because this is only for debugger integration (and not really a public API) and this header file isn't shipped anywhere.


================
Comment at: test/tsan/Darwin/external-swift-debugging.cc:10
+extern "C" {
+int __tsan_get_report_data(void *report, const char **description, int *count,
+                           int *stack_count, int *mop_count, int *loc_count,
----------------
delcypher wrote:
> I know for ASan we ship the interface header with Clang. Do we not do that with TSan? If we do can you just include that so you don't have to redefine this here?
Yeah, we do not. The reason is that these are not really stable APIs.


================
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 {{.*}}
----------------
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.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D46661