[MSan][LLVM][MIPS] Shadow and Origin offsets for MIPS

[mohit.bhakkad]

Summary

Added offsets for MIPS in

lib/Transforms/Instrumentation/MemorySanitizer.cpp

[kcc]

lib/Transforms/Instrumentation/MemorySanitizer.cpp

128 ↗

(On Diff #15847)

I'd prefer not to have any more 32-bit here for now,
at least until we have 32-bit msan on x86

453 ↗

(On Diff #15847)

add an else statement with

report_fatal_error("unsupported triple");

[mohit.bhakkad]

Changes in this revision:

• Removed mips32 related changes.
• added fatal_error report for unsupported triplets.

[mohit.bhakkad]

Without this patch and http://reviews.llvm.org/D6147, mips64 in compiler-rt won't build.
I have removed all msan32 related changes, please let me know if any other changes are needed.

[eugenis]

LGTM w/ nit

lib/Transforms/Instrumentation/MemorySanitizer.cpp

123 ↗

(On Diff #16556)

Please change it to kX86_64ShadowMask, kX86ShadowMask, etc (the same for origin offset below).

125 ↗

(On Diff #16556)

No need for the "64" suffix, it already says "MIPS64".

[mohit.bhakkad]

Changes in this revision:

• Changed names of flags as suggested

[eugenis]

LGTM with 1 more comment

lib/Transforms/Instrumentation/MemorySanitizer.cpp

125 ↗

(On Diff #16724)

Please remove the extra underscore after MIPS64 and change the constant to 1ULL << (something) for consistency.

[mohit.bhakkad]

Changes in this revision:
-Removed the extra underscore after MIPS64 in names of constants.
-Changed the constants to 1ULL << (something) format.

[samsonov]

lib/Transforms/Instrumentation/MemorySanitizer.cpp

401 ↗

(On Diff #16756)

You can get rid of the PtrSize completely. Just make a single switch on M.getTrargetTriple().getArch() to choose between x86, x86_64 and MIPS.

[eugenis]

FYI, http://reviews.llvm.org/D6666

[kutuzov.viktor.84]

Mohit, I've committed D6666 so your patch may need some rework. Note that the patch committed does not include the improvements suggested for this patch like reporting unsupported triples. We probably should replace the references to the unsupported 32-bit memory map descriptors with null pointers and check them in doInitialization(). It's OK to remove the 32-bit descriptor for FreeBSD as it is not actually supported anyway.

[mohit.bhakkad]

Changes in this revision:

• Updated the patch in order to match with Viktor's commit.
• Some changes in names for distinction.
• Replaced PtrSize switch case with switch case for OS and Archs as suggested by Alexey.

[mohit.bhakkad]

Hi @eugenis, @samsonov

This patch is important one for msan to work on MIPS, please let me know if any suggestions/changes in this revision.

[eugenis]

Do you have commit access? (i.e. should I land this change?)

lib/Transforms/Instrumentation/MemorySanitizer.cpp

251 ↗

(On Diff #17668)

Let's call it "X86" instead of "INTEL".

[mohit.bhakkad]

In D6146#110712, @eugenis wrote:

Do you have commit access? (i.e. should I land this change?)

Yes, I have commit access, will commit it after doing suggested change.

[mohit.bhakkad]

Changes in this Revision:

• Changed "INTEL" to "X86".

llvm/trunk/lib/Transforms/Instrumentation/MemorySanitizer.cpp

 };
 
 struct PlatformMemoryMapParams {
   const MemoryMapParams *bits32;
   const MemoryMapParams *bits64;
 };
 
 // i386 Linux
-static const MemoryMapParams LinuxMemoryMapParams32 = {
+static const MemoryMapParams Linux_I386_MemoryMapParams = {
   0x000080000000,  // AndMask
   0,               // XorMask (not used)
   0,               // ShadowBase (not used)
   0x000040000000,  // OriginBase
 };
 
 // x86_64 Linux
-static const MemoryMapParams LinuxMemoryMapParams64 = {
+static const MemoryMapParams Linux_X86_64_MemoryMapParams = {
   0x400000000000,  // AndMask
   0,               // XorMask (not used)
   0,               // ShadowBase (not used)
   0x200000000000,  // OriginBase
 };
 
+// mips64 Linux
+static const MemoryMapParams Linux_MIPS64_MemoryMapParams = {
+  0x004000000000,  // AndMask
+  0,               // XorMask (not used)
+  0,               // ShadowBase (not used)
+  0x002000000000,  // OriginBase
+};
+
 // i386 FreeBSD
-static const MemoryMapParams FreeBSDMemoryMapParams32 = {
+static const MemoryMapParams FreeBSD_I386_MemoryMapParams = {
   0x000180000000,  // AndMask
   0x000040000000,  // XorMask
   0x000020000000,  // ShadowBase
   0x000700000000,  // OriginBase
 };
 
 // x86_64 FreeBSD
-static const MemoryMapParams FreeBSDMemoryMapParams64 = {
+static const MemoryMapParams FreeBSD_X86_64_MemoryMapParams = {
   0xc00000000000,  // AndMask
   0x200000000000,  // XorMask
   0x100000000000,  // ShadowBase
   0x380000000000,  // OriginBase
 };
 
-static const PlatformMemoryMapParams LinuxMemoryMapParams = {
-  &LinuxMemoryMapParams32,
-  &LinuxMemoryMapParams64,
+static const PlatformMemoryMapParams Linux_X86_MemoryMapParams = {
+  &Linux_I386_MemoryMapParams,
+  &Linux_X86_64_MemoryMapParams,
 };
 
-static const PlatformMemoryMapParams FreeBSDMemoryMapParams = {
-  &FreeBSDMemoryMapParams32,
-  &FreeBSDMemoryMapParams64,
+static const PlatformMemoryMapParams Linux_MIPS_MemoryMapParams = {
+  NULL,
+  &Linux_MIPS64_MemoryMapParams,
+};
+
+static const PlatformMemoryMapParams FreeBSD_X86_MemoryMapParams = {
+  &FreeBSD_I386_MemoryMapParams,
+  &FreeBSD_X86_64_MemoryMapParams,
 };
 
 /// \brief An instrumentation pass implementing detection of uninitialized
 /// reads.
 ///
 /// MemorySanitizer: instrument the code in module to find
 /// uninitialized reads.
 class MemorySanitizer : public FunctionPass {

 /// inserts a call to __msan_init to the module's constructor list.
 bool MemorySanitizer::doInitialization(Module &M) {
   DataLayoutPass *DLP = getAnalysisIfAvailable<DataLayoutPass>();
   if (!DLP)
     report_fatal_error("data layout missing");
   DL = &DLP->getDataLayout();
 
   Triple TargetTriple(M.getTargetTriple());
-  const PlatformMemoryMapParams *PlatformMapParams;
-  if (TargetTriple.getOS() == Triple::FreeBSD)
-    PlatformMapParams = &FreeBSDMemoryMapParams;
-  else
-    PlatformMapParams = &LinuxMemoryMapParams;
-
-  C = &(M.getContext());
-  unsigned PtrSize = DL->getPointerSizeInBits(/* AddressSpace */0);
-  switch (PtrSize) {
-    case 64:
-      MapParams = PlatformMapParams->bits64;
+  switch (TargetTriple.getOS()) {
+    case Triple::FreeBSD:
+      switch (TargetTriple.getArch()) {
+        case Triple::x86_64:
+          MapParams = FreeBSD_X86_MemoryMapParams.bits64;
           break;
-    case 32:
-      MapParams = PlatformMapParams->bits32;
+        case Triple::x86:
+          MapParams = FreeBSD_X86_MemoryMapParams.bits32;
           break;
         default:
-      report_fatal_error("unsupported pointer size");
+          report_fatal_error("unsupported architecture");
+      }
+      break;
+    case Triple::Linux:
+      switch (TargetTriple.getArch()) {
+        case Triple::x86_64:
+          MapParams = Linux_X86_MemoryMapParams.bits64;
+          break;
+        case Triple::x86:
+          MapParams = Linux_X86_MemoryMapParams.bits32;
           break;
+        case Triple::mips64:
+        case Triple::mips64el:
+          MapParams = Linux_MIPS_MemoryMapParams.bits64;
+          break;
+        default:
+          report_fatal_error("unsupported architecture");
+      }
+      break;
+    default:
+      report_fatal_error("unsupported operating system");
   }
 
+  C = &(M.getContext());
   IRBuilder<> IRB(*C);
   IntptrTy = IRB.getIntPtrTy(DL);
   OriginTy = IRB.getInt32Ty();
 
   ColdCallWeights = MDBuilder(*C).createBranchWeights(1, 1000);
   OriginStoreWeights = MDBuilder(*C).createBranchWeights(1, 1000);
 
   // Insert a call to __msan_init/__msan_track_origins into the module's CTORs.