[llvm-commits] Please review: adding Function attribute "ThreadSafety"

[Kostya Serebryany]

On Thu, Mar 15, 2012 at 2:07 AM, Chris Lattner <clattner at apple.com> wrote:

>
> On Mar 14, 2012, at 6:08 PM, Kostya Serebryany wrote:
>
> Hello,
>
> I would like to add one more function attribute: "ThreadSafety".
> Functions with this attribute will be instrumented by run-time
> thread-safety analysis tools (e.g. with ThreadSanitizer).
> This is similar to "AddressSafety" attribute used by AddressSanitizer (and
> SAFECode?).
>
> Clang already has the attribute
> "__attribute__((no_thread_safety_analysis))" which is used to disable
> static thread-safety analysis.
> http://clang.llvm.org/docs/LanguageExtensions.html#threadsafety
> In a separate change we will teach clang to unset ThreadSafety
> when no_thread_safety_analysis is present.
> (exactly as we do for no_address_safety_analysis and AddressSafety)
>
>
> Hi Kostya,
>
> I'm skeptical of adding LLVM IR function attributes for each
> instrumentation pass.  Have you investigated using module flags metadata to
> track this information?
>

I didn't, thanks for the link.
It looks like this would work too, although an attribute still sounds more
compelling to me.
It's just one bit (so it is free, as long as you have spare bits).
Do you dislike it because there are just too few Attribute bits available
(31 left), or for other reasons?

My (perhaps incorrect) understanding of the metadata system is that if the
metadata is removed, the correctness of the code is not affected.
In this case, loosing metadata will mean introducing
incorrect instrumentation.

Also, if I use metadata for tsan and an attribute for asan, it will look
weird.
Would you suggest to use metadata for asan as well (instead of
AddressSafety attribute)?

The AddressSafety attribute is checked at least in one place outside of
asan code (lib/Analysis/MemoryDependenceAnalysis.cpp).
The ThreadSafety attribute (or equivalent metadata) will need to be checked
in the same place too:
struct X {
   uint64_t for_alignment;
   char a, b, c, d;
};

Thread1(X *x) {
   ... = X.a + X.d
}

Thread2(X *x) {
   X.b = ...
}

If GVN widens X.a + X.d, tsan will start reporting a non-existing race.

Thanks,

--kcc



> It is designed to be extensible for these sorts of uses.
>
> http://llvm.org/docs/LangRef.html#module_flags
>



>
> -Chris
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120315/9284d4ed/attachment.html>