[PATCH] D25858: [tsan][llvm] Implement the function attribute to disable TSan checking at run time

[Dmitry Vyukov via llvm-commits]

dvyukov added inline comments.


================
Comment at: lib/Transforms/Instrumentation/ThreadSanitizer.cpp:390
+    return true;
+  return false;
+}
----------------
Isn't this just "return F.hasFnAttribute("sanitize_thread_no_checking_at_run_time")"?
If you remove the Attribute::SanitizeThread attribute when adding this new attribute, then isNoCheckingAtRunTime will be called at only 1 place. So it will be simpler to just use "F.hasFnAttribute("sanitize_thread_no_checking_at_run_time")" instead of it.


================
Comment at: lib/Transforms/Instrumentation/ThreadSanitizer.cpp:393
+
+static bool shouldSanitize(Function &F) {
+  if (!F.hasFnAttribute(Attribute::SanitizeThread))
----------------
This will go away if you remove the Attribute::SanitizeThread attribute when adding this new attribute.


================
Comment at: lib/Transforms/Instrumentation/ThreadSanitizer.cpp:410
+  }
+  for (auto NoRetCallInst : NoRetCalls) {
+    IRBuilder<> IRB(NoRetCallInst);
----------------
Do you have any particular use case for noreturn calls? I don't think it will work as expected. We don't return from that function, not from this function. Consider that we longjmp solely within this function, we will still exit from this function and call TsanIgnoreEnd on normal return path.
Moreover, most of the functions that recursively call noreturn function are not noreturn themselves. So if we call a function that recursively calls longjmp, we will fail to call TsanIgnoreEnd.



https://reviews.llvm.org/D25858