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

Derek Bruening via llvm-commits llvm-commits at lists.llvm.org
Wed May 25 14:08:37 PDT 2016

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).


More information about the llvm-commits mailing list