[llvm-commits] tsan: detect use-after-free (issue 6214052)

[Dmitry Vyukov]

On Thu, May 17, 2012 at 2:51 PM, <kcc at google.com> wrote:

> Great idea!
> But I don't like to hide the "freed" bit inside TID.
> can we make this bit more explicit?
>

I think yes. I am not very happy with current variant as well.


>
>
> http://codereview.appspot.com/**6214052/diff/1/output_tests/**free_race2.c<http://codereview.appspot.com/6214052/diff/1/output_tests/free_race2.c>
> File output_tests/free_race2.c (right):
>
> http://codereview.appspot.com/**6214052/diff/1/output_tests/**
> free_race2.c#newcode3<http://codereview.appspot.com/6214052/diff/1/output_tests/free_race2.c#newcode3>
> output_tests/free_race2.c:3: void foo(int *mem) {
> you will need to disable inlining.
> This can be done later once we star running all tests with -O2
>

Do we actually need to run output tests with -finline?
For functions for which it is irrelevant, it it irrelevant.
For functions that require no-inline, we will need to add attributes.
What's the point?
I would prefer to add __attribute__((always_inline)) to inlining tests,
if/when we have any. That will be required anyway, otherwise they will fail
with -O0.



>
> http://codereview.appspot.com/**6214052/diff/1/rtl/tsan_clock.**h<http://codereview.appspot.com/6214052/diff/1/rtl/tsan_clock.h>
> File rtl/tsan_clock.h (right):
>
> http://codereview.appspot.com/**6214052/diff/1/rtl/tsan_clock.**
> h#newcode75<http://codereview.appspot.com/6214052/diff/1/rtl/tsan_clock.h#newcode75>
> rtl/tsan_clock.h:75: u64 clk_[kMaxTid * 2];
> Maybe we can use another kSomething?
>
> http://codereview.appspot.com/**6214052/diff/1/rtl/tsan_defs.h<http://codereview.appspot.com/6214052/diff/1/rtl/tsan_defs.h>
> File rtl/tsan_defs.h (right):
>
> http://codereview.appspot.com/**6214052/diff/1/rtl/tsan_defs.**h#newcode32<http://codereview.appspot.com/6214052/diff/1/rtl/tsan_defs.h#newcode32>
> rtl/tsan_defs.h:32: const int kTidBits = 16;
> I don't really like this.
> Can we have a separate free-ed bit.
> It can bit adjacent to tid so that the rest of the logic works.
>
> http://codereview.appspot.com/**6214052/diff/1/rtl/tsan_**report.cc<http://codereview.appspot.com/6214052/diff/1/rtl/tsan_report.cc>
> File rtl/tsan_report.cc (right):
>
> http://codereview.appspot.com/**6214052/diff/1/rtl/tsan_**
> report.cc#newcode36<http://codereview.appspot.com/6214052/diff/1/rtl/tsan_report.cc#newcode36>
> rtl/tsan_report.cc:36: Printf("use after free");
> maybe print "heap-use-after-free" to match asan's output?
>

Will do.



>
> http://codereview.appspot.com/**6214052/diff/1/rtl/tsan_rtl.h<http://codereview.appspot.com/6214052/diff/1/rtl/tsan_rtl.h>
> File rtl/tsan_rtl.h (right):
>
> http://codereview.appspot.com/**6214052/diff/1/rtl/tsan_rtl.h#**newcode94<http://codereview.appspot.com/6214052/diff/1/rtl/tsan_rtl.h#newcode94>
> rtl/tsan_rtl.h:94: // will race with it.
> Too hackish. I'd prefer a separate bit.
>
> http://codereview.appspot.com/**6214052/<http://codereview.appspot.com/6214052/>
>

I am thinking how to refactor the bit.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120517/cb496747/attachment.html>