[PATCH] D25857: [tsan][clang] Introduce a function attribute to disable TSan checking at run time

[Anna Zaks via cfe-commits]

zaks.anna marked 4 inline comments as done.
zaks.anna added inline comments.


================
Comment at: test/CodeGen/sanitize-thread-no-checking-at-run-time.m:1
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -x objective-c++ -emit-llvm -o - %s | FileCheck -check-prefix=WITHOUT %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -x objective-c++ -emit-llvm -o - %s -fsanitize=thread | FileCheck -check-prefix=TSAN %s
----------------
dvyukov wrote:
> Are you sure this is the right location for the test?
> test/CodeGen does not seem to contain any tests, only subdirs.
You are looking at the llvm/test/CodeGen instead of the clang/test/CodeGen. The latter contains individual test files and I think it's the right place for the test.


================
Comment at: test/CodeGen/sanitize-thread-no-checking-at-run-time.m:35
+// TSAN: attributes [[ATTR]] = { nounwind {{.*}} "sanitize_thread_no_checking_at_run_time" {{.*}} }
+// TSAN-NOT: sanitize_thread
----------------
dvyukov wrote:
> Does this check actually work?
> I would expect that sanitize_thread, if present, will be eaten by the previous line.
> Not sure what's the best way to fix it. What is the exact list of attributes on the previous line? Maybe we can just specify them all without using {{.*}}?
Listing all attributes won't be low maintenance as more attributes could be added. Here is the list we have now:

attributes #0 = { nounwind sanitize_thread "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "target-features"="+mmx,+sse,+sse2" "unsafe-fp-math"="false" "use-soft-float"="false" }

I could check that there are double quotes after nounwind:
TSAN: attributes [[ATTR]] = { nounwind "{{.*}} "sanitize_thread_no_checking_at_run_time" {{.*}} }

Alternatively, I could drop this aspect of the test altogether since I have an assert in the llvm pass.


https://reviews.llvm.org/D25857