<div style="font-family: arial, helvetica, sans-serif; font-size: 10pt">Excellent. <div>Minor comment below. <br><br><div class="gmail_quote">On Wed, Nov 28, 2012 at 4:19 PM, Dmitry Vyukov <span dir="ltr"><<a href="mailto:dvyukov@google.com" target="_blank">dvyukov@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: dvyukov<br>
Date: Wed Nov 28 06:19:50 2012<br>
New Revision: 168786<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=168786&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=168786&view=rev</a><br>
Log:<br>
tsan: dynamic history size<br>
introduces history_size parameter that can be used to control trace size at startup<br>
<br>
Modified:<br>
compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.cc<br>
compiler-rt/trunk/lib/tsan/rtl/tsan_flags.cc<br>
compiler-rt/trunk/lib/tsan/rtl/tsan_flags.h<br>
compiler-rt/trunk/lib/tsan/rtl/tsan_platform_linux.cc<br>
compiler-rt/trunk/lib/tsan/rtl/tsan_rtl.cc<br>
compiler-rt/trunk/lib/tsan/rtl/tsan_rtl.h<br>
compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_report.cc<br>
compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_thread.cc<br>
compiler-rt/trunk/lib/tsan/rtl/tsan_trace.h<br>
compiler-rt/trunk/lib/tsan/tests/unit/tsan_shadow_test.cc<br>
<br>
Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.cc<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.cc?rev=168786&r1=168785&r2=168786&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.cc?rev=168786&r1=168785&r2=168786&view=diff</a><br>
==============================================================================<br>
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.cc (original)<br>
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.cc Wed Nov 28 06:19:50 2012<br>
@@ -169,7 +169,10 @@<br>
struct rlimit rlim;<br>
rlim.rlim_cur = limit;<br>
rlim.rlim_max = limit;<br>
- CHECK_EQ(0, setrlimit(RLIMIT_STACK, &rlim));<br>
+ if (setrlimit(RLIMIT_STACK, &rlim)) {<br>
+ Report("setrlimit() failed %d\n", errno);<br>
+ Die();<br>
+ }<br>
CHECK(!StackSizeIsUnlimited());<br>
}<br>
<br>
<br>
Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_flags.cc<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_flags.cc?rev=168786&r1=168785&r2=168786&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_flags.cc?rev=168786&r1=168785&r2=168786&view=diff</a><br>
==============================================================================<br>
--- compiler-rt/trunk/lib/tsan/rtl/tsan_flags.cc (original)<br>
+++ compiler-rt/trunk/lib/tsan/rtl/tsan_flags.cc Wed Nov 28 06:19:50 2012<br>
@@ -56,6 +56,11 @@<br>
f->stop_on_start = false;<br>
f->running_on_valgrind = false;<br>
f->external_symbolizer_path = "";<br>
+ f->history_size = 2;<br>
+<br>
+#ifdef TSAN_GO<br></blockquote><div><br></div><div>For such checks I'd prefer to have if (TSAN_GO) instead of #ifdef TSAN_GO. (here and in other places). </div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ f->history_size = 1; // There are a lot of goroutines.<br>
+#endif<br>
<br>
// Let a frontend override.<br>
OverrideFlags(f);<br>
@@ -79,12 +84,19 @@<br>
ParseFlag(env, &f->flush_memory_ms, "flush_memory_ms");<br>
ParseFlag(env, &f->stop_on_start, "stop_on_start");<br>
ParseFlag(env, &f->external_symbolizer_path, "external_symbolizer_path");<br>
+ ParseFlag(env, &f->history_size, "history_size");<br>
<br>
if (!f->report_bugs) {<br>
f->report_thread_leaks = false;<br>
f->report_destroy_locked = false;<br>
f->report_signal_unsafe = false;<br>
}<br>
+<br>
+ if (f->history_size < 0 || f->history_size > 7) {<br>
+ Printf("ThreadSanitizer: incorrect value for history_size"<br>
+ " (must be [0..7])\n");<br>
+ Die();<br>
+ }<br>
}<br>
<br>
} // namespace __tsan<br>
<br>
Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_flags.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_flags.h?rev=168786&r1=168785&r2=168786&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_flags.h?rev=168786&r1=168785&r2=168786&view=diff</a><br>
==============================================================================<br>
--- compiler-rt/trunk/lib/tsan/rtl/tsan_flags.h (original)<br>
+++ compiler-rt/trunk/lib/tsan/rtl/tsan_flags.h Wed Nov 28 06:19:50 2012<br>
@@ -66,6 +66,12 @@<br>
bool running_on_valgrind;<br>
// Path to external symbolizer.<br>
const char *external_symbolizer_path;<br>
+ // Per-thread history size, controls how many previous memory accesses<br>
+ // is remembered per thread. Possible values are [0..7].<br></blockquote><div>are remembered </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ // history_size=0 amounts to 32K memory accesses. Each next value doubles<br>
+ // the amount of memory accesses, up to history_size=7 that amounts to<br>
+ // 4M memory accesses. The default value is 2 (128K memory accesses).<br>
+ int history_size;<br>
};<br>
<br>
Flags *flags();<br>
<br>
Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_platform_linux.cc<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_platform_linux.cc?rev=168786&r1=168785&r2=168786&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_platform_linux.cc?rev=168786&r1=168785&r2=168786&view=diff</a><br>
==============================================================================<br>
--- compiler-rt/trunk/lib/tsan/rtl/tsan_platform_linux.cc (original)<br>
+++ compiler-rt/trunk/lib/tsan/rtl/tsan_platform_linux.cc Wed Nov 28 06:19:50 2012<br>
@@ -202,28 +202,50 @@<br>
}<br>
#endif // #ifndef TSAN_GO<br>
<br>
+static rlim_t getlim(int res) {<br>
+ rlimit rlim;<br>
+ CHECK_EQ(0, getrlimit(res, &rlim));<br>
+ return rlim.rlim_cur;<br>
+}<br>
+<br>
+static void setlim(int res, rlim_t lim) {<br>
+ // The following magic is to prevent clang from replacing it with memset.<br>
+ volatile rlimit rlim;<br>
+ rlim.rlim_cur = lim;<br>
+ rlim.rlim_max = lim;<br>
+ setrlimit(res, (rlimit*)&rlim);<br>
+}<br>
+<br>
const char *InitializePlatform() {<br>
void *p = 0;<br>
if (sizeof(p) == 8) {<br>
// Disable core dumps, dumping of 16TB usually takes a bit long.<br>
- // The following magic is to prevent clang from replacing it with memset.<br>
- volatile rlimit lim;<br>
- lim.rlim_cur = 0;<br>
- lim.rlim_max = 0;<br>
- setrlimit(RLIMIT_CORE, (rlimit*)&lim);<br>
+ setlim(RLIMIT_CORE, 0);<br>
}<br>
+ bool reexec = false;<br>
// TSan doesn't play well with unlimited stack size (as stack<br>
// overlaps with shadow memory). If we detect unlimited stack size,<br>
// we re-exec the program with limited stack size as a best effort.<br>
- if (StackSizeIsUnlimited()) {<br>
- const uptr kMaxStackSize = 32 * 1024 * 1024; // 32 Mb<br>
+ if (getlim(RLIMIT_STACK) == (rlim_t)-1) {<br>
+ const uptr kMaxStackSize = 32 * 1024 * 1024;<br>
Report("WARNING: Program is run with unlimited stack size, which "<br>
"wouldn't work with ThreadSanitizer.\n");<br>
Report("Re-execing with stack size limited to %zd bytes.\n", kMaxStackSize);<br>
SetStackSizeLimitInBytes(kMaxStackSize);<br>
- ReExec();<br>
+ reexec = true;<br>
}<br>
<br>
+ if (getlim(RLIMIT_AS) != (rlim_t)-1) {<br>
+ Report("WARNING: Program is run with limited virtual address space, which "<br>
+ "wouldn't work with ThreadSanitizer.\n");<br>
+ Report("Re-execing with unlimited virtual address space.\n");<br>
+ setlim(RLIMIT_AS, -1);<br>
+ reexec = true;<br>
+ }<br>
+<br>
+ if (reexec)<br>
+ ReExec();<br>
+<br>
#ifndef TSAN_GO<br>
CheckPIE();<br>
g_tls_size = (uptr)InitTlsSize();<br>
<br>
Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_rtl.cc<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_rtl.cc?rev=168786&r1=168785&r2=168786&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_rtl.cc?rev=168786&r1=168785&r2=168786&view=diff</a><br>
==============================================================================<br>
--- compiler-rt/trunk/lib/tsan/rtl/tsan_rtl.cc (original)<br>
+++ compiler-rt/trunk/lib/tsan/rtl/tsan_rtl.cc Wed Nov 28 06:19:50 2012<br>
@@ -290,11 +290,14 @@<br>
<br>
uptr TraceTopPC(ThreadState *thr) {<br>
Event *events = (Event*)GetThreadTrace(thr->tid);<br>
- uptr pc = events[thr->fast_state.epoch() % kTraceSize]<br>
- & ((1ull << 61) - 1);<br>
+ uptr pc = events[thr->fast_state.GetTracePos()];<br>
return pc;<br>
}<br>
<br>
+uptr TraceSize() {<br>
+ return (uptr)(1ull << (kTracePartSizeBits + flags()->history_size + 1));<br>
+}<br>
+<br>
#ifndef TSAN_GO<br>
extern "C" void __tsan_trace_switch() {<br>
TraceSwitch(cur_thread());<br>
<br>
Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_rtl.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_rtl.h?rev=168786&r1=168785&r2=168786&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_rtl.h?rev=168786&r1=168785&r2=168786&view=diff</a><br>
==============================================================================<br>
--- compiler-rt/trunk/lib/tsan/rtl/tsan_rtl.h (original)<br>
+++ compiler-rt/trunk/lib/tsan/rtl/tsan_rtl.h Wed Nov 28 06:19:50 2012<br>
@@ -74,6 +74,7 @@<br>
// tid : kTidBits<br>
// epoch : kClkBits<br>
// unused : -<br>
+// history_size : 3<br>
class FastState {<br>
public:<br>
FastState(u64 tid, u64 epoch) {<br>
@@ -113,6 +114,27 @@<br>
void ClearIgnoreBit() { x_ &= ~kIgnoreBit; }<br>
bool GetIgnoreBit() const { return (s64)x_ < 0; }<br>
<br>
+ void SetHistorySize(int hs) {<br>
+ CHECK_GE(hs, 0);<br>
+ CHECK_LE(hs, 7);<br>
+ x_ = (x_ & ~7) | hs;<br>
+ }<br>
+<br>
+ int GetHistorySize() const {<br>
+ return (int)(x_ & 7);<br>
+ }<br>
+<br>
+ void ClearHistorySize() {<br>
+ x_ &= ~7;<br>
+ }<br>
+<br>
+ u64 GetTracePos() const {<br>
+ const int hs = GetHistorySize();<br>
+ // When hs == 0, the trace consists of 2 parts.<br></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">+ const u64 mask = (1ull << (kTracePartSizeBits + hs + 1)) - 1;<br>
+ return epoch() & mask;<br>
+ }<br>
+<br>
private:<br>
friend class Shadow;<br>
static const int kTidShift = 64 - kTidBits - 1;<br>
@@ -131,9 +153,14 @@<br>
// addr0 : 3<br>
class Shadow : public FastState {<br>
public:<br>
- explicit Shadow(u64 x) : FastState(x) { }<br>
+ explicit Shadow(u64 x)<br>
+ : FastState(x) {<br>
+ }<br>
<br>
- explicit Shadow(const FastState &s) : FastState(s.x_) { }<br>
+ explicit Shadow(const FastState &s)<br>
+ : FastState(s.x_) {<br>
+ ClearHistorySize();<br>
+ }<br>
<br>
void SetAddr0AndSizeLog(u64 addr0, unsigned kAccessSizeLog) {<br>
DCHECK_EQ(x_ & 31, 0);<br>
@@ -535,12 +562,13 @@<br>
<br>
void TraceSwitch(ThreadState *thr);<br>
uptr TraceTopPC(ThreadState *thr);<br>
+uptr TraceSize();<br>
<br>
extern "C" void __tsan_trace_switch();<br>
void ALWAYS_INLINE INLINE TraceAddEvent(ThreadState *thr, FastState fs,<br>
EventType typ, uptr addr) {<br>
StatInc(thr, StatEvents);<br>
- u64 epoch = fs.epoch();<br>
+ u64 epoch = fs.GetTracePos();<br></blockquote><div><br></div><div>Should this be named epoch then? </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
if (UNLIKELY((epoch % kTracePartSize) == 0)) {<br>
#ifndef TSAN_GO<br>
HACKY_CALL(__tsan_trace_switch);<br>
@@ -549,7 +577,7 @@<br>
#endif<br>
}<br>
Event *trace = (Event*)GetThreadTrace(fs.tid());<br>
- Event *evp = &trace[epoch % kTraceSize];<br>
+ Event *evp = &trace[epoch];<br>
Event ev = (u64)addr | ((u64)typ << 61);<br>
*evp = ev;<br>
}<br>
<br>
Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_report.cc<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_report.cc?rev=168786&r1=168785&r2=168786&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_report.cc?rev=168786&r1=168785&r2=168786&view=diff</a><br>
==============================================================================<br>
--- compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_report.cc (original)<br>
+++ compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_report.cc Wed Nov 28 06:19:50 2012<br>
@@ -263,11 +263,11 @@<br>
return;<br>
}<br>
Lock l(&trace->mtx);<br>
- const int partidx = (epoch / (kTraceSize / kTraceParts)) % kTraceParts;<br>
+ const int partidx = (epoch / (TraceSize() / kTraceParts)) % kTraceParts;<br>
TraceHeader* hdr = &trace->headers[partidx];<br>
if (epoch < hdr->epoch0)<br>
return;<br>
- const u64 eend = epoch % kTraceSize;<br>
+ const u64 eend = epoch % TraceSize();<br>
const u64 ebegin = eend / kTracePartSize * kTracePartSize;<br>
DPrintf("#%d: RestoreStack epoch=%zu ebegin=%zu eend=%zu partidx=%d\n",<br>
tid, (uptr)epoch, (uptr)ebegin, (uptr)eend, partidx);<br>
<br>
Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_thread.cc<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_thread.cc?rev=168786&r1=168785&r2=168786&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_thread.cc?rev=168786&r1=168785&r2=168786&view=diff</a><br>
==============================================================================<br>
--- compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_thread.cc (original)<br>
+++ compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_thread.cc Wed Nov 28 06:19:50 2012<br>
@@ -123,7 +123,7 @@<br>
void *mem = internal_alloc(MBlockThreadContex, sizeof(ThreadContext));<br>
tctx = new(mem) ThreadContext(tid);<br>
ctx->threads[tid] = tctx;<br>
- MapThreadTrace(GetThreadTrace(tid), kTraceSize * sizeof(Event));<br>
+ MapThreadTrace(GetThreadTrace(tid), TraceSize() * sizeof(Event));<br>
}<br>
CHECK_NE(tctx, 0);<br>
CHECK_GE(tid, 0);<br>
@@ -149,7 +149,6 @@<br>
thr->fast_synch_epoch = thr->fast_state.epoch();<br>
thr->clock.release(&tctx->sync);<br>
StatInc(thr, StatSyncRelease);<br>
-<br>
tctx->creation_stack.ObtainCurrent(thr, pc);<br>
}<br>
return tid;<br>
@@ -205,6 +204,7 @@<br>
thr->fast_synch_epoch = tctx->epoch0;<br>
thr->clock.set(tid, tctx->epoch0);<br>
thr->clock.acquire(&tctx->sync);<br>
+ thr->fast_state.SetHistorySize(flags()->history_size);<br>
StatInc(thr, StatSyncAcquire);<br>
DPrintf("#%d: ThreadStart epoch=%zu stk_addr=%zx stk_size=%zx "<br>
"tls_addr=%zx tls_size=%zx\n",<br>
<br>
Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_trace.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_trace.h?rev=168786&r1=168785&r2=168786&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_trace.h?rev=168786&r1=168785&r2=168786&view=diff</a><br>
==============================================================================<br>
--- compiler-rt/trunk/lib/tsan/rtl/tsan_trace.h (original)<br>
+++ compiler-rt/trunk/lib/tsan/rtl/tsan_trace.h Wed Nov 28 06:19:50 2012<br>
@@ -19,12 +19,9 @@<br>
<br>
namespace __tsan {<br>
<br>
-#ifndef TSAN_HISTORY_SIZE // in kibitraces<br>
-#define TSAN_HISTORY_SIZE 128<br>
-#endif<br>
-<br>
-const int kTracePartSize = 16 * 1024;<br>
-const int kTraceParts = TSAN_HISTORY_SIZE * 1024 / kTracePartSize;<br>
+const int kTracePartSizeBits = 14;<br>
+const int kTracePartSize = 1 << kTracePartSizeBits;<br>
+const int kTraceParts = 4 * 1024 * 1024 / kTracePartSize;<br>
const int kTraceSize = kTracePartSize * kTraceParts;<br>
<br>
// Must fit into 3 bits.<br>
<br>
Modified: compiler-rt/trunk/lib/tsan/tests/unit/tsan_shadow_test.cc<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/tests/unit/tsan_shadow_test.cc?rev=168786&r1=168785&r2=168786&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/tests/unit/tsan_shadow_test.cc?rev=168786&r1=168785&r2=168786&view=diff</a><br>
==============================================================================<br>
--- compiler-rt/trunk/lib/tsan/tests/unit/tsan_shadow_test.cc (original)<br>
+++ compiler-rt/trunk/lib/tsan/tests/unit/tsan_shadow_test.cc Wed Nov 28 06:19:50 2012<br>
@@ -22,6 +22,7 @@<br>
EXPECT_EQ(s.epoch(), (u64)22);<br>
EXPECT_EQ(s.GetIgnoreBit(), false);<br>
EXPECT_EQ(s.GetFreedAndReset(), false);<br>
+ EXPECT_EQ(s.GetHistorySize(), 0);<br>
EXPECT_EQ(s.addr0(), (u64)0);<br>
EXPECT_EQ(s.size(), (u64)1);<br>
EXPECT_EQ(s.is_write(), false);<br>
@@ -35,6 +36,14 @@<br>
EXPECT_EQ(s.GetIgnoreBit(), true);<br>
s.ClearIgnoreBit();<br>
EXPECT_EQ(s.GetIgnoreBit(), false);<br>
+<br>
+ for (int i = 0; i < 8; i++) {<br>
+ s.SetHistorySize(i);<br>
+ EXPECT_EQ(s.GetHistorySize(), i);<br>
+ }<br>
+ s.SetHistorySize(2);<br>
+ s.ClearHistorySize();<br>
+ EXPECT_EQ(s.GetHistorySize(), 0);<br>
}<br>
<br>
TEST(Shadow, Mapping) {<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>