[PATCH] [msan] intercept dlopen and clear shadow for it

[Evgeniy Stepanov]

================
Comment at: lib/msan/msan_linux.cc:34
@@ +33,3 @@
+// llvm/Support/Compiler.h defines away our interface.
+#undef __msan_unpoison
+
----------------
Reid Kleckner wrote:
> Evgeniy Stepanov wrote:
> > This is fragile. Future additions to Compiler.h can silently disable parts of MSan.
> > Please move UnpoisonMappedDSO to a new .cc file, so that this include does not affect anything else.
> Yeah, sounds good.  Moved to msan_elf.cc.  It is kind of worrying that the ELF type stuff that I use here might at some point become more than just a header dependence.  Do you think it would be better to include llvm/Support/ELF.h and use Elf64/32_[EP]hdr directly?
Why not include <sys/elf.h> instead?

================
Comment at: lib/msan/msan_linux.cc:125
@@ +124,3 @@
+  // Compute the delta from the real base to get a relocation delta.
+  uptr delta = (uptr)base - preferred_base;
+  // Now we can figure out what the loader really mapped.
----------------
Reid Kleckner wrote:
> Evgeniy Stepanov wrote:
> > CHECK that base >= preferred_base.
> I actually intended for this to work if base >= preferred_base, so maybe I should make delta signed.
> 
> I haven't tested this case, though.  I think you need to use pre-linking or something to make it happen.
Yes, it can go either way, probably. Please make it signed.

================
Comment at: lib/msan/msan_linux.cc:134
@@ +133,3 @@
+      // mapping as defined.
+      seg_start = RoundDownTo(seg_start, GetPageSizeCached());
+      seg_end = RoundUpTo(seg_end, GetPageSizeCached());
----------------
Reid Kleckner wrote:
> Evgeniy Stepanov wrote:
> > Why is it needed? Could we just unpoison to the exact mapping limits?
> It shouldn't be needed, but I didn't like leaving stale shadow around.  The ragged edges are in fact well-defined (trailing bytes of .debug or .text before .data or trailing zeros past EOF), if not very useful.
Sounds good.

================
Comment at: lib/msandr/msandr.cc:556
@@ +555,3 @@
+
+  // We don't switch to the app context because we __msan_unpoison() doesn't
+  // need TLS segments.
----------------
s/we//

================
Comment at: lib/msandr/msandr.cc:649
@@ +648,3 @@
+        dr_printf("unpoisoning for dlopen: [%p-%p]\n", base, base + size);
+      // We don't switch to the app context because we __msan_unpoison() doesn't
+      // need TLS segments.
----------------
here, too


http://llvm-reviews.chandlerc.com/D509