[PATCH] D20751: [esan] Add sideline itimer support

[Derek Bruening via llvm-commits]

bruening marked 2 inline comments as done.

================
Comment at: lib/esan/esan_sideline_linux.cpp:49
@@ +48,3 @@
+      return;
+    Thread->sampleFunc(Thread->FuncArg);
+  } else
----------------
aizatsky wrote:
> I think you should add overrun detection in case your function is slow from day 1.
> You would get very unreliable results if you hit this, and it would be hard to tell why,
I would like to do that in a separate CL as it is not simple -- as we're using a real-time timer, a heavily loaded machine could result in an occasional sample still being processed when the next alarm arrives, which won't be a big deal.  We'd want to collect a count and report at the end the number of overlaps.  I.e., it wouldn't just be unblocking the signal, setting a flag inside, and a CHECK that the flag is not set on entry.

================
Comment at: lib/esan/esan_sideline_linux.cpp:73
@@ +72,3 @@
+  InternalScopedBuffer<char> StackMap(SigAltStackSize);
+  struct sigaltstack SigAltStack;
+  internal_memset(&SigAltStack, 0, sizeof(SigAltStack));
----------------
vitalybuka wrote:
> SigAltStack = {} and no need memset
The worry is that the compiler will call libc memset and hit the interceptor.  There are only 3 fields so that's unlikely here, but just setting ss_flags to 0 should be enough.

================
Comment at: lib/esan/esan_sideline_linux.cpp:88
@@ +87,3 @@
+
+  if (!Thread->adjustTimer(Thread->Freq)) {
+    Printf("FATAL: EfficiencySanitizer failed to set up itimer\n");
----------------
vitalybuka wrote:
> just CHECK(Thread->adjustTimer(Thread->Freq))  ?
Best not to place key actions inside a CHECK but I can do a CHECK on the result stored in a var.


http://reviews.llvm.org/D20751