[PATCH] [msan] Refactor memory layout specification and setup

[Evgeniy Stepanov]

================
Comment at: lib/msan/CMakeLists.txt:18
@@ +17,3 @@
+  ${SANITIZER_COMMON_CFLAGS}
+  -Wno-unknown-pragmas
+)
----------------
glider wrote:
> Please add a comment listing the concrete pragmae you're suppressing.
Now that I've added gcc pragmas, this is not needed.

================
Comment at: lib/msan/msan.h:72
@@ -89,3 +71,3 @@
 
-const uptr kBad2Beg  = kShadowBeg + kShadowSize;
-const uptr kBad2Size = kOriginsBeg - kBad2Beg;
+const MappingDesc kMemoryLayout[] = {
+    {0x000000000000ULL, 0x200000000000ULL, MappingDesc::INVALID, "invalid"},
----------------
glider wrote:
> Please add a comment about the restrictions that led to these particular memory layouts on each platform (like it's done for FreeBSD)
done for linux/x86_64

================
Comment at: lib/msan/msan.h:90
@@ +89,3 @@
+inline bool addr_is_type(uptr addr, MappingDesc::Type mapping_type) {
+#pragma unroll
+  for (unsigned i = 0; i < kMemoryLayoutSize; ++i)
----------------
glider wrote:
> Please add a comment describing the necessity of unrolling this loop.
> You may want to add something GCC-specific under an #ifdef (e.g. __attribute__((optimize("unroll-loops"))) or "pragma GCC unroll-loops")
> (see http://stackoverflow.com/questions/4071690/tell-gcc-to-specifically-unroll-a-loop)
done

================
Comment at: lib/msan/msan_linux.cc:80
@@ +79,3 @@
+    MappingDesc::Type type = kMemoryLayout[i].type;
+    CHECK_EQ(prev_end, start);
+    CHECK(addr_is_type(start, type));
----------------
glider wrote:
> Maybe also assert that "end - start" is sane (e.g. not less than a page).
> I can imagine accidental zero-sized ranges.
done

================
Comment at: lib/msan/msan_linux.cc:109
@@ -108,14 +108,3 @@
 
-  if (!CheckMemoryRangeAvailability(kShadowBeg, kShadowSize) ||
-      (init_origins &&
-        !CheckMemoryRangeAvailability(kOriginsBeg, kOriginsSize)) ||
-      !CheckMemoryRangeAvailability(kBad1Beg, kBad1Size) ||
-      !CheckMemoryRangeAvailability(kBad2Beg, kBad2Size) ||
-      !CheckMemoryRangeAvailability(kBad3Beg, kBad3Size)) {
-    return false;
-  }
-
-  if (!ProtectMemoryRange(kBad1Beg, kBad1Size) ||
-      !ProtectMemoryRange(kBad2Beg, kBad2Size) ||
-      !ProtectMemoryRange(kBad3Beg, kBad3Size)) {
-    return false;
+  for (unsigned i = 0; i < kMemoryLayoutSize; ++i) {
+    uptr start = kMemoryLayout[i].start;
----------------
glider wrote:
> Can this loop be joined with the loop above?
I'd prefer not to. They do different things, even though they iterate over the same array.
I've even moved the first loop to a separate function now.

http://reviews.llvm.org/D7146

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/