[PATCH] D20578: [esan|wset] Iterate all memory to compute the total working set

[Derek Bruening via llvm-commits]

bruening added inline comments.

================
Comment at: lib/esan/working_set.cpp:85
@@ +84,3 @@
+  // Get word aligned start.
+  ShadowStart = RoundDownTo(ShadowStart, 4);
+  for (u32 *Ptr = (u32 *)ShadowStart; Ptr < (u32 *)ShadowEnd; ++Ptr) {
----------------
filcab wrote:
> RoundDown? We'll be looking at some shadow bytes before `ShadowStart`. Won't that be a problem?
No, this routine may be used for handling smaller regions in the future (e.g., just the heap) where it is convenient to not require precision or alignment from the caller.

================
Comment at: lib/esan/working_set.cpp:129
@@ +128,3 @@
+static u32 getSizeForPrinting(u32 NumOfCachelines, const char *&Unit) {
+  // We need a constant to avoid software divide support:
+  static const u32 line_size = 64;
----------------
filcab wrote:
> We don't really care, since we only do this at finalize time (and we're printing stuff anyway), no?
There are linker errors for missing symbols like __cxa_guard_acquire which would require adding libraries these sanitizer libs try to avoid.

================
Comment at: lib/esan/working_set_posix.cpp:69
@@ -68,5 +68,3 @@
   if (SigNum == SIGSEGV) {
-
-    // TODO: Add shadow memory fault detection and handling.
-
-    if (AppSigAct.sigaction) {
+    // We rely on si_addr being filled in (thus we do not support old kernels).
+    siginfo_t *SigInfo = (siginfo_t *)Info;
----------------
filcab wrote:
> aizatsky wrote:
> > Should this be working_set_linux.cpp then?
> or at least the _posix variant.
POSIX specifies that si_addr be filled in for SIGSEGV.  It's older Linux that fails to do so (we're talking really old: 10+ years IIRC).


http://reviews.llvm.org/D20578