[PATCH] D32382: [tsan] Track external tags in thread traces

[Dmitry Vyukov via Phabricator via llvm-commits]

dvyukov added inline comments.


================
Comment at: lib/tsan/rtl/tsan_rtl_report.cc:669
+    VarSizeStackTrace trace;
+    trace.Init(traces[i].trace, traces[i].size);
+    uptr tag = RetrieveTagFromStackTrace(&trace);
----------------
I think it's better to separate stack and tag directly in ObtainCurrentStack and RestoreStack because:
1. We will not need to copy traces here (which is ugly and confusing at first glance).
2. No chance that the fake PCs pop up somewhere else because we forgot to remove them.
3. Racy stacks will be more difficult to break (currently a future change to this function can lead to a situation when we remember a stripped stack but check against non-stripped).
4. It will be easier to figure out report type without creating ScopedReport first and then fixing up the type with SetType.


================
Comment at: lib/tsan/rtl/tsan_rtl_report.cc:672
+    if (tag != kExternalTagNone) {
+      rep.SetType(ReportTypeExternalRace);
+    }
----------------
I wonder if we need ReportTypeExternalRace at all.
We could always use ReportTypeRace, but attach tag to the report instead of memory accesses. Then we don't need SetType and PrintReport will be able to specialize report header with e.g. "data race on curl handle", because then it will know that the whole report is about an external access.
What do you think?


Repository:
  rL LLVM

https://reviews.llvm.org/D32382