search

yibaini / thread-sanitizer

Automatically exported from code.google.com/p/thread-sanitizer
0 stars 0 forks source link

__attribute__((no_sanitize_thread)) is not supported even though the documentation says it is #35

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
Build with tsan and run this: https://pastebin.mozilla.org/3271918

The simplest possible fix is this:
--- lib/Transforms/Instrumentation/ThreadSanitizer.cpp  (revision 192795)
+++ lib/Transforms/Instrumentation/ThreadSanitizer.cpp  (working copy)
@@ -358,7 +358,7 @@
   // (e.g. variables that do not escape, etc).

   // Instrument memory accesses.
-  if (ClInstrumentMemoryAccesses)
+  if (ClInstrumentMemoryAccesses && 
F.hasFnAttribute(Attribute::SanitizeThread))
     for (size_t i = 0, n = AllLoadsAndStores.size(); i < n; ++i) {
       Res |= instrumentLoadOrStore(AllLoadsAndStores[i]);
     }

Or maybe we should check the attribute near 
  if (BL->isIn(F)) return false;

Anyway, Dmitry, please fix (with lit tests)

Original issue reported on code.google.com by konstant...@gmail.com on 16 Oct 2013 at 2:43

GoogleCodeExporter commented 9 years ago 
Kostya, have you fixed inlining of no_sanitize_thread/no_sanitize_address 
functions?
Such functions must not be inlined and preferably nothing must be inlined into 
them.

Original comment by dvyu...@google.com on 16 Oct 2013 at 3:20

GoogleCodeExporter commented 9 years ago 
I can confirm that this patch works :)

We're usually using the attributes together with __attribute__((noinline)) to 
prevent inlining. So even if it doesn't work yet without that, it would be 
worth applying the fix for now.

Original comment by decoder...@googlemail.com on 16 Oct 2013 at 3:28

GoogleCodeExporter commented 9 years ago 
Thanks for testing!
Are you using clang tip for asan/tsan testing?

Original comment by dvyu...@google.com on 16 Oct 2013 at 3:36

GoogleCodeExporter commented 9 years ago 
Yes, I think eugenis@ fixed inlining issue a while ago (function with 
no_sanitize_foo attribute can't be inlined into function without such 
attribute, and vice versa).

Original comment by samso...@google.com on 16 Oct 2013 at 3:46

GoogleCodeExporter commented 9 years ago 
I've used clang tip to confirm the patch works, and I'm currently trying to 
compile mozilla-central with that. But on our build infrastructure, we use an 
older version for ASan testing.

Original comment by decoder...@googlemail.com on 16 Oct 2013 at 3:49

GoogleCodeExporter commented 9 years ago 
>Yes, I think eugenis@ fixed inlining issue a while ago (function with 
no_sanitize_foo attribute can't be inlined into function without such 
attribute, and vice versa).

Great, thanks!

Original comment by dvyu...@google.com on 16 Oct 2013 at 4:04

GoogleCodeExporter commented 9 years ago 
>I've used clang tip to confirm the patch works, and I'm currently trying to 
compile mozilla-central with that. But on our build infrastructure, we use an 
older version for ASan testing.

I see.

Original comment by dvyu...@google.com on 16 Oct 2013 at 4:04

GoogleCodeExporter commented 9 years ago 
Mailed http://llvm-reviews.chandlerc.com/D1952

Original comment by dvyu...@google.com on 16 Oct 2013 at 4:35

GoogleCodeExporter commented 9 years ago 
Fixed by r 192871, 192872.

Original comment by dvyu...@google.com on 17 Oct 2013 at 8:10

GoogleCodeExporter commented 9 years ago 
Adding Project:ThreadSanitizer as part of GitHub migration.

Original comment by gli...@google.com on 30 Jul 2015 at 9:21