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

[Kostya Serebryany]

>
> 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)?
>
>
> Yes please! :)
>
>
Ok (once we find out what's the right way).


> 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.
>
>
> I never agreed with this patch, but let it go in to avoid blocking forward
> progress.
>

Thanks!
I agree that the patch is not perfect (all problems that you mentioned,
especially with inlining).

<shameless plug>
BTW, this particular patch unblocked the progress for mozilla (e.g.
http://www.mozilla.org/security/announce/2012/mfsa2012-14.html)
and clang+asan bootstrap (found ~5 clang/llvm bugs already, more are in my
queue)
</shameless plug>


>
> However, now that you ask :), GVN should never have checked for a "asan"
> attribute.  If you want an attribute to disable specific transformations,
> we should design this properly, and it should be a property of the load or
> store, not a property of the function.
>

I don't see how to attach the property to loads/stores.
If we use instruction-level metadata, it may be lost (as you just
explained) above.
If we add an instruction-level attribute (are there such?), what value
should be used if some phase creates a new load/store?
Again, what should be done when some code is copied (inlined, unrolled,
etc)?


--kcc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120316/0b72afd8/attachment.html>