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

[Alexander Potapenko]

Looks mostly good.


REPOSITORY
  rL LLVM

================
Comment at: lib/msan/CMakeLists.txt:18
@@ +17,3 @@
+  ${SANITIZER_COMMON_CFLAGS}
+  -Wno-unknown-pragmas
+)
----------------
Please add a comment listing the concrete pragmae you're suppressing.

================
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"},
----------------
Please add a comment about the restrictions that led to these particular memory layouts on each platform (like it's done for FreeBSD)

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

================
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));
----------------
Maybe also assert that "end - start" is sane (e.g. not less than a page).
I can imagine accidental zero-sized ranges.

================
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;
----------------
Can this loop be joined with the loop above?

http://reviews.llvm.org/D7146

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