<div class="gmail_quote">On Thu, May 17, 2012 at 2:51 PM,  <span dir="ltr"><<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Great idea!<br>
But I don't like to hide the "freed" bit inside TID.<br>
can we make this bit more explicit?<br></blockquote><div><br></div><div>I think yes. I am not very happy with current variant as well.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<br>
<br>
<a href="http://codereview.appspot.com/6214052/diff/1/output_tests/free_race2.c" target="_blank">http://codereview.appspot.com/<u></u>6214052/diff/1/output_tests/<u></u>free_race2.c</a><br>
File output_tests/free_race2.c (right):<br>
<br>
<a href="http://codereview.appspot.com/6214052/diff/1/output_tests/free_race2.c#newcode3" target="_blank">http://codereview.appspot.com/<u></u>6214052/diff/1/output_tests/<u></u>free_race2.c#newcode3</a><br>
output_tests/free_race2.c:3: void foo(int *mem) {<br>
you will need to disable inlining.<br>
This can be done later once we star running all tests with -O2<br></blockquote><div><br></div><div>Do we actually need to run output tests with -finline?</div><div>For functions for which it is irrelevant, it it irrelevant.</div>
<div>For functions that require no-inline, we will need to add attributes.</div><div>What's the point?</div><div>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.</div>
<div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<a href="http://codereview.appspot.com/6214052/diff/1/rtl/tsan_clock.h" target="_blank">http://codereview.appspot.com/<u></u>6214052/diff/1/rtl/tsan_clock.<u></u>h</a><br>
File rtl/tsan_clock.h (right):<br>
<br>
<a href="http://codereview.appspot.com/6214052/diff/1/rtl/tsan_clock.h#newcode75" target="_blank">http://codereview.appspot.com/<u></u>6214052/diff/1/rtl/tsan_clock.<u></u>h#newcode75</a><br>
rtl/tsan_clock.h:75: u64 clk_[kMaxTid * 2];<br>
Maybe we can use another kSomething?<br>
<br>
<a href="http://codereview.appspot.com/6214052/diff/1/rtl/tsan_defs.h" target="_blank">http://codereview.appspot.com/<u></u>6214052/diff/1/rtl/tsan_defs.h</a><br>
File rtl/tsan_defs.h (right):<br>
<br>
<a href="http://codereview.appspot.com/6214052/diff/1/rtl/tsan_defs.h#newcode32" target="_blank">http://codereview.appspot.com/<u></u>6214052/diff/1/rtl/tsan_defs.<u></u>h#newcode32</a><br>
rtl/tsan_defs.h:32: const int kTidBits = 16;<br>
I don't really like this.<br>
Can we have a separate free-ed bit.<br>
It can bit adjacent to tid so that the rest of the logic works.<br>
<br>
<a href="http://codereview.appspot.com/6214052/diff/1/rtl/tsan_report.cc" target="_blank">http://codereview.appspot.com/<u></u>6214052/diff/1/rtl/tsan_<u></u>report.cc</a><br>
File rtl/tsan_report.cc (right):<br>
<br>
<a href="http://codereview.appspot.com/6214052/diff/1/rtl/tsan_report.cc#newcode36" target="_blank">http://codereview.appspot.com/<u></u>6214052/diff/1/rtl/tsan_<u></u>report.cc#newcode36</a><br>
rtl/tsan_report.cc:36: Printf("use after free");<br>
maybe print "heap-use-after-free" to match asan's output?<br></blockquote><div><br></div><div>Will do.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<br>
<a href="http://codereview.appspot.com/6214052/diff/1/rtl/tsan_rtl.h" target="_blank">http://codereview.appspot.com/<u></u>6214052/diff/1/rtl/tsan_rtl.h</a><br>
File rtl/tsan_rtl.h (right):<br>
<br>
<a href="http://codereview.appspot.com/6214052/diff/1/rtl/tsan_rtl.h#newcode94" target="_blank">http://codereview.appspot.com/<u></u>6214052/diff/1/rtl/tsan_rtl.h#<u></u>newcode94</a><br>
rtl/tsan_rtl.h:94: // will race with it.<br>
Too hackish. I'd prefer a separate bit.<br>
<br>
<a href="http://codereview.appspot.com/6214052/" target="_blank">http://codereview.appspot.com/<u></u>6214052/</a><br>
</blockquote></div><br><div>I am thinking how to refactor the bit.</div><div><br></div>