[MSan] [MIPS] Adding support for MIPS64

[mohit.bhakkad]

Summary

Most of the changes are related to mips64 arch.

[kcc]

lib/msan/msan.cc

510

eugenis@, maybe we should simply drop this code?

lib/msan/msan_linux.cc

38

Why do you use

#if defined(__mips64__) && (SANITIZER_WORDSIZE == 64)

here and just

#if defined(__mips64__)

elsewhere?

Does mips64 imply SANITIZER_WORDSIZE == 64?
If so, just use mips64

lib/sanitizer_common/sanitizer_allocator.h

475

Did you already verify that this actually works?
For a 40-bit address space you may need to use SanitizerAllocator32 instead.

[mohit.bhakkad]

This revision resolves mips64 flag related issue

[mohit.bhakkad]

lib/msan/msan_linux.cc

38

https://code.google.com/p/android/issues/detail?id=75904 suggests that __mips64 is prevalent than __mips64__

lib/sanitizer_common/sanitizer_allocator.h

475

Yes, I have verified using both SanitizerAllocator32 and SanitizerAllocator64, which are giving same results. As SizeClassAllocator64 is more efficient, so we are using that.

[kcc]

lib/sanitizer_common/sanitizer_allocator.h

475

How exactly did you verify it?
Did you run any large programs that consume gigs of RAM?
I still doubt this will work.

[mohit.bhakkad]

How exactly did you verify it?
Did you run any large programs that consume gigs of RAM?
I still doubt this will work.

MIPS is generally used in embedded, which may not require gigs of RAM.
Till now, I have just ran the testcases on mips rpe-100 board, and majority of it are passing.
For real world testing, I have tried MSan on nano editor, which is also working fine.
I will be testing it with some larger application, and will update you with the results.
Any suggestions?

[samsonov]

I'm fine with adjusting constants for MIPS you're doing here, but will let Kostya sign this off. I hope you will be able to setup the buildbot, otherwise we can frequently break you.

lib/sanitizer_common/sanitizer_allocator.h

473

s/its/it's

[mohit.bhakkad]

@kcc @samsonov :Thanks for your comments.

To make MSan work on MIPS, there are 2 more patches:

• LLVM code: kShadowMask and kOriginOffset are to be defined for MIPS:

lib/Transforms/Instrumentation/MemorySanitizer.cpp#L123

• Clang code: to enable memory option for mips.

lib/Driver/SanitizerArgs.cpp#L272

I just wanted to ask if I should submit those patches for review, or wait till this one is merged.

[eugenis]

I'm OK with this change, but please wait for a comment from Kostya.

I just wanted to ask if I should submit those patches for review, or wait till this one is merged.

Better send them early.

lib/msan/msan.h

35

Please change this to #elif defined(x86_64) and an #error if neither is defined.
The same in msan_allocator.cc and other similar cases.

[mohit.bhakkad]

lib/msan/msan.h

35

I will put x86_64 in #elif, but an extra line for #error will be redundant, as control will never come here, as "memory" option is not enabled in clang driver for those architectures.

[mohit.bhakkad]

Changes in this revision:

1. Used SanitizerAllocator32 instead of SanitizerAllocator64 for mips64 as its address space is just 40-bit.
2. Changed increment for PC in origin tracking w.r.t. mips64.
3. Put x86_64 in #elif instead of just #else.

[samsonov]

lib/msan/msan_report.cc

56 ↗

(On Diff #16151)

Evgeniy, do we know the reason why this adjustment is needed? Looks like we'd have to use the inverse of StackTrace::GetPreviousInstructionPc() here.

lib/sanitizer_common/sanitizer_posix.cc

92

Hm? This looks similar to the previous #elif

[eugenis]

lib/msan/msan_report.cc

56 ↗

(On Diff #16151)

No. I'd like to know why :)

[mohit.bhakkad]

lib/sanitizer_common/sanitizer_posix.cc

92

Sorry for my mistake, I didn't realized that it was added in some previous patch.
Any other comments/changes on this revision?

[kcc]

lib/msan/msan.cc

510–512

This code will get deleted with http://reviews.llvm.org/D6268, please wait for it.

lib/msan/msan_report.cc

58 ↗

(On Diff #16151)

I'd prefer not to have #ifdef here.
At the very least, move it into a separate function near the StackTrace::GetPreviousInstructionPc
but I would like to understand why we need this adjustment here at all.

[mohit.bhakkad]

lib/msan/msan_report.cc

58 ↗

(On Diff #16151)

I don't know the reason for this adjustment.
I will submit a new revision with a function StackTrace::GetNextInstructionPc in
sanitizer_stacktrace.cc, once http://reviews.llvm.org/D6268 gets commited.

[mohit.bhakkad]

Changes in this revision:
Added function StackTrace::GetNextInstructionPc in sanitizer_stacktrace.cc

[kcc]

Looks good.
eugenis, please have one more look and commit if you are ok with this.

[mohit.bhakkad]

Thanks @kcc @eugenis @samsonov

Just a gentle reminder, that this patch is dependent on http://reviews.llvm.org/D6146 and http://reviews.llvm.org/D6147
I have one more query, is there is any plan to accept msan32?, if yes I will update whatever changes needed in http://reviews.llvm.org/D5672

[samsonov]

LGTM

[kcc]

I have one more query, is there is any plan to accept msan32?, if yes I will update whatever changes needed in http://reviews.llvm.org/D5672

msan on 32 bit will have very limited applicability due to limited address space.
Remember, the application will have only < 1Gb of RAM available to it.
Supporting msan on 32-bit is an additional maintenance cost with little to not benefit, at least where I see it.
Do you want msan on 32-bit because there are MIPS customers who really need it and can't use 64-bit,
or just for completeness?

[samsonov]

Landed in r222388.

[earthdok]

Now some lucky soul has to update all the docs. :)

[earthdok]

Hold on, there hasn't been an LGTM from eugenis@. Why did you land this?

[samsonov]

In D5906#14, @eugenis wrote:

I'm OK with this change, but please wait for a comment from Kostya.

^^ here ^^
eugenis@, did I land it too early?

We should update the docs after the change to LLVM instrumentation pass and to Clang driver are in.

[mohit.bhakkad]

In D5906#36, @kcc wrote:

msan on 32 bit will have very limited applicability due to limited address space.
Remember, the application will have only < 1Gb of RAM available to it.
Supporting msan on 32-bit is an additional maintenance cost with little to not benefit, at least where I see it.
Do you want msan on 32-bit because there are MIPS customers who really need it and can't use 64-bit,
or just for completeness?

We are aware of the limitation of MSan 32bit however there are existing customers of MIPS32 which we think might be potential users of MSAN 32bit.

[eugenis]

In D5906#42, @samsonov wrote:

In D5906#14, @eugenis wrote:

I'm OK with this change, but please wait for a comment from Kostya.

^^ here ^^
eugenis@, did I land it too early?

No, it's fine. LGTM.

[kcc]

We are aware of the limitation of MSan 32bit however there are existing customers of MIPS32 which we think might be potential users of MSAN 32bit.

Ok, let's try.
We can not afford to have MIIPS-32 and not have x86-32 because for most developers there is
no way to test on MIIPS-32.
I would ask you to enable MSan on x86-32, make sure that all tests pass,
that check-msan runs the 32-bit tests on x86-64 machines,
that the bots (lab.llvm.org:8011/builders/sanitizer-x86_64-linux and lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/)
run these tests too.
Then we can enable msan on other 32-bit architecture(s).

cmake/config-ix.cmake

 filter_available_targets(ASAN_SUPPORTED_ARCH
   x86_64 i386 i686 powerpc64 arm mips mipsel)
 filter_available_targets(DFSAN_SUPPORTED_ARCH x86_64)
 filter_available_targets(LSAN_SUPPORTED_ARCH x86_64)
 # LSan common files should be available on all architectures supported
 # by other sanitizers (even if they build into dummy object files).
 filter_available_targets(LSAN_COMMON_SUPPORTED_ARCH
   ${SANITIZER_COMMON_SUPPORTED_ARCH})
-filter_available_targets(MSAN_SUPPORTED_ARCH x86_64)
+filter_available_targets(MSAN_SUPPORTED_ARCH x86_64 mips64)
 filter_available_targets(PROFILE_SUPPORTED_ARCH x86_64 i386 i686 arm mips mips64 mipsel mips64el aarch64)
 filter_available_targets(TSAN_SUPPORTED_ARCH x86_64)
 filter_available_targets(UBSAN_SUPPORTED_ARCH x86_64 i386 i686 arm aarch64 mips mipsel)
 
 if(ANDROID)
   set(OS_NAME "Android")
 else()
   set(OS_NAME "${CMAKE_SYSTEM_NAME}")

lib/msan/CMakeLists.txt

 append_list_if(COMPILER_RT_HAS_FPIE_FLAG -fPIE MSAN_RTL_CFLAGS)
 # Prevent clang from generating libc calls.
 append_list_if(COMPILER_RT_HAS_FFREESTANDING_FLAG -ffreestanding MSAN_RTL_CFLAGS)
 
 set(MSAN_RUNTIME_LIBRARIES)
 
 # Static runtime library.
 add_custom_target(msan)
-set(arch "x86_64")
-if(CAN_TARGET_${arch})
+foreach(arch ${MSAN_SUPPORTED_ARCH})
   add_compiler_rt_runtime(clang_rt.msan-${arch} ${arch} STATIC
     SOURCES ${MSAN_RTL_SOURCES}
             $<TARGET_OBJECTS:RTInterception.${arch}>
             $<TARGET_OBJECTS:RTSanitizerCommon.${arch}>
             $<TARGET_OBJECTS:RTSanitizerCommonLibc.${arch}>
     CFLAGS ${MSAN_RTL_CFLAGS})
   add_dependencies(msan clang_rt.msan-${arch})
   list(APPEND MSAN_RUNTIME_LIBRARIES clang_rt.msan-${arch})
   if(UNIX)
     add_sanitizer_rt_symbols(clang_rt.msan-${arch} msan.syms.extra)
     add_dependencies(msan clang_rt.msan-${arch}-symbols)
   endif()
-endif()
+endforeach()
 
 add_compiler_rt_resource_file(msan_blacklist msan_blacklist.txt)
 add_dependencies(msan msan_blacklist)
 add_dependencies(compiler-rt msan)
 
 if(COMPILER_RT_INCLUDE_TESTS)
   add_subdirectory(tests)
 endif()

lib/msan/msan.h

 #include "sanitizer_common/sanitizer_stacktrace.h"
 #include "msan_interface_internal.h"
 #include "msan_flags.h"
 
 #ifndef MSAN_REPLACE_OPERATORS_NEW_AND_DELETE
 # define MSAN_REPLACE_OPERATORS_NEW_AND_DELETE 1
 #endif
 
+#if defined(__mips64__) && (SANITIZER_WORDSIZE == 64)
+#define MEM_TO_SHADOW(mem)       (((uptr)mem) & ~0x4000000000ULL)
+#define SHADOW_TO_ORIGIN(shadow) (((uptr)shadow) + 0x2000000000ULL)
+#define MEM_TO_ORIGIN(mem)       (SHADOW_TO_ORIGIN(MEM_TO_SHADOW(mem)))
+#define MEM_IS_APP(mem)          ((uptr)mem >= 0xe000000000ULL)
+#define MEM_IS_SHADOW(mem) \
+  ((uptr)mem >= 0xa000000000ULL && (uptr)mem <= 0xc000000000ULL)
+#else
 #define MEM_TO_SHADOW(mem)       (((uptr)mem) & ~0x400000000000ULL)
 #define SHADOW_TO_ORIGIN(shadow) (((uptr)shadow) + 0x200000000000ULL)
 #define MEM_TO_ORIGIN(mem)       (SHADOW_TO_ORIGIN(MEM_TO_SHADOW(mem)))
 #define MEM_IS_APP(mem)          ((uptr)mem >= 0x600000000000ULL)
 #define MEM_IS_SHADOW(mem) \
   ((uptr)mem >= 0x200000000000ULL && (uptr)mem <= 0x400000000000ULL)
+#endif
 
 // These constants must be kept in sync with the ones in MemorySanitizer.cc.
 const int kMsanParamTlsSize = 800;
 const int kMsanRetvalTlsSize = 800;
 
 namespace __msan {
 extern int msan_inited;
 extern bool msan_init_is_running;

lib/msan/msan.cc

int  __msan_has_dynamic_component() {
   return msan_running_under_dr;
 }
 
 NOINLINE
 void __msan_clear_on_return() {
   __msan_param_tls[0] = 0;
 }
 
 static void* get_tls_base() {
+  // as this is only relevant for architecture supporting msandr
+#ifdef __x86_64__
   u64 p;
   asm("mov %%fs:0, %0"
       : "=r"(p) ::);
   return (void*)p;
+#else
+  return 0;
+#endif /* __x86_64__ */
 }
 
 int __msan_get_retval_tls_offset() {
   // volatile here is needed to avoid UB, because the compiler thinks that we
   // are doing address arithmetics on unrelated pointers, and takes some
   // shortcuts
   volatile sptr retval_tls_p = (sptr)&__msan_retval_tls;
   volatile sptr tls_base_p = (sptr)get_tls_base();

lib/msan/msan_allocator.cc

void OnUnmap(uptr p, uptr size) const {
     // We are about to unmap a chunk of user memory.
     // Mark the corresponding shadow memory as not needed.
     FlushUnneededShadowMemory(MEM_TO_SHADOW(p), size);
     if (__msan_get_track_origins())
       FlushUnneededShadowMemory(MEM_TO_ORIGIN(p), size);
   }
 };
 
+#if defined(__mips64__) && (SANITIZER_WORDSIZE == 64)
+static const uptr kAllocatorSpace = 0xf00000000ULL;
+static const uptr kAllocatorSize   = 0xf00000000;
+static const uptr kMaxAllowedMallocSize = 1UL << 37;
+#else
 static const uptr kAllocatorSpace = 0x600000000000ULL;
 static const uptr kAllocatorSize   = 0x80000000000;  // 8T.
-static const uptr kMetadataSize  = sizeof(Metadata);
 static const uptr kMaxAllowedMallocSize = 8UL << 30;
+#endif
+static const uptr kMetadataSize  = sizeof(Metadata);
 
 typedef SizeClassAllocator64<kAllocatorSpace, kAllocatorSize, kMetadataSize,
                              DefaultSizeClassMap,
                              MsanMapUnmapCallback> PrimaryAllocator;
 typedef SizeClassAllocatorLocalCache<PrimaryAllocator> AllocatorCache;
 typedef LargeMmapAllocator<MsanMapUnmapCallback> SecondaryAllocator;
 typedef CombinedAllocator<PrimaryAllocator, AllocatorCache,
                           SecondaryAllocator> Allocator;

lib/msan/msan_linux.cc

 #include <sys/time.h>
 #include <sys/resource.h>
 
 #include "sanitizer_common/sanitizer_common.h"
 #include "sanitizer_common/sanitizer_procmaps.h"
 
 namespace __msan {
 
+#if defined(__mips64__) && (SANITIZER_WORDSIZE == 64)
+static const uptr kMemBeg     = 0xe000000000;
+static const uptr kMemEnd     = 0xffffffffff;
+#else
 static const uptr kMemBeg     = 0x600000000000;
 static const uptr kMemEnd     = 0x7fffffffffff;
+#endif
+
 static const uptr kShadowBeg  = MEM_TO_SHADOW(kMemBeg);
 static const uptr kShadowEnd  = MEM_TO_SHADOW(kMemEnd);
 static const uptr kBad1Beg    = 0;
 static const uptr kBad1End    = kShadowBeg - 1;
 static const uptr kBad2Beg    = kShadowEnd + 1;
 static const uptr kBad2End    = kMemBeg - 1;
 static const uptr kOriginsBeg = kBad2Beg;
 static const uptr kOriginsEnd = kBad2End;

lib/sanitizer_common/sanitizer_allocator.h

public:
   typedef SizeClassMap SizeClassMapT;
   static const uptr kNumClasses = SizeClassMap::kNumClasses;
   static const uptr kNumClassesRounded = SizeClassMap::kNumClassesRounded;
 
  private:
   static const uptr kRegionSize = kSpaceSize / kNumClassesRounded;
   static const uptr kSpaceEnd = kSpaceBeg + kSpaceSize;
   COMPILER_CHECK(kSpaceBeg % kSpaceSize == 0);
+#if defined(__mips64__)
+  // For mips64 virtual address space is just 40 bits, so its not possible
+  // to have kRegionSize >= 2^32
+  COMPILER_CHECK((kRegionSize) >= (1ULL << (40 / 2)));
+#else
   // kRegionSize must be >= 2^32.
   COMPILER_CHECK((kRegionSize) >= (1ULL << (SANITIZER_WORDSIZE / 2)));
+#endif
   // Populate the free list with at most this number of bytes at once
   // or with one element if its size is greater.
   static const uptr kPopulateSize = 1 << 14;
   // Call mmap for user memory with at least this size.
   static const uptr kUserMapSize = 1 << 16;
   // Call mmap for metadata memory with at least this size.
   static const uptr kMetaMapSize = 1 << 16;
 

lib/sanitizer_common/sanitizer_platform.h

 #if defined(__ANDROID__)
 # define SANITIZER_ANDROID 1
 #else
 # define SANITIZER_ANDROID 0
 #endif
 
 #define SANITIZER_POSIX (SANITIZER_FREEBSD || SANITIZER_LINUX || SANITIZER_MAC)
 
-#if __LP64__ || defined(_WIN64)
+#if __LP64__ || defined(_WIN64) || defined(__mips64__)
 #  define SANITIZER_WORDSIZE 64
 #else
 #  define SANITIZER_WORDSIZE 32
 #endif
 
 #if SANITIZER_WORDSIZE == 64
 # define FIRST_32_SECOND_64(a, b) (b)
 #else

 #endif
 
 // The range of addresses which can be returned my mmap.
 // FIXME: this value should be different on different platforms,
 // e.g. on AArch64 it is most likely (1ULL << 39). Larger values will still work
 // but will consume more memory for TwoLevelByteMap.
 #if defined(__aarch64__)
 # define SANITIZER_MMAP_RANGE_SIZE FIRST_32_SECOND_64(1ULL << 32, 1ULL << 39)
+#elseif defined(__mips64__)
+# define SANITIZER_MMAP_RANGE_SIZE FIRST_32_SECOND_64(1ULL << 32, 1ULL << 40)
 #else
 # define SANITIZER_MMAP_RANGE_SIZE FIRST_32_SECOND_64(1ULL << 32, 1ULL << 47)
 #endif
 
 // The AArch64 linux port uses the canonical syscall set as mandated by
 // the upstream linux community for all new ports. Other ports may still
 // use legacy syscalls.
 #ifndef SANITIZER_USES_CANONICAL_LINUX_SYSCALLS
 # if defined(__aarch64__) && SANITIZER_LINUX
 # define SANITIZER_USES_CANONICAL_LINUX_SYSCALLS 1
 # else
 # define SANITIZER_USES_CANONICAL_LINUX_SYSCALLS 0
 # endif
 #endif
 
 #endif // SANITIZER_PLATFORM_H

lib/sanitizer_common/sanitizer_posix.cc

# if defined(__powerpc64__) && defined(__BIG_ENDIAN__)
   // one of 0x00000fffffffffffUL and 0x00003fffffffffffUL.
   // Note that with 'ulimit -s unlimited' the stack is moved away from the top
   // of the address space, so simply checking the stack address is not enough.
   return (1ULL << 44) - 1;  // 0x00000fffffffffffUL
 # elif defined(__powerpc64__) && defined(__LITTLE_ENDIAN__)
   return (1ULL << 46) - 1;  // 0x00003fffffffffffUL
 # elif defined(__aarch64__)
   return (1ULL << 39) - 1;
+# elif defined(__mips64__)
+  return (1ULL << 40) - 1;  // 0x000000ffffffffffUL;
 # else
   return (1ULL << 47) - 1;  // 0x00007fffffffffffUL;
 # endif
 #else  // SANITIZER_WORDSIZE == 32
   uptr res = (1ULL << 32) - 1;  // 0xffffffff;
   if (!common_flags()->full_address_space)
     res -= GetKernelAreaSize();
   CHECK_LT(reinterpret_cast<uptr>(&res), res);