[MSan][Clang][MIPS] Enabled memory, dataflow and thread options for MIPS platform

[samsonov]

Does MSan/DFSan actually work and pass all the tests on mips32 and mips64? Does TSan work and pass all tests on mips64?

[mohit.bhakkad]

In D6147#5, @samsonov wrote:

Does MSan/DFSan actually work and pass all the tests on mips32 and mips64? Does TSan work and pass all tests on mips64?

I have submitted patches of MSan mips64[1] and mips32[2], for which majority of tests
are passing, and some failures are because of minor differences in printing, I am working on it.
@eugenis suggested me to submit patches for llvm and clang.

As we are also working on TSan and DFSan, I thought of enabling them in clang.

[ 1 ] http://reviews.llvm.org/D5906
[ 2 ] http://reviews.llvm.org/D5672

[eugenis]

I don't think 100% passing tests should be a requirement for this change to go in. On the opposite, driver support has value even if the tool barely works, and is a prerequisite for tests passing.

[samsonov]

IIRC we had problems when we enabled the feature with a few tests still failing, and immediately got complaints from users, whose LLVM build started to fail "check-all" command.
I agree that it's hard to use and test the tool w/o Driver support. But I'd like to see the order of landing patches that would keep the MIPS build green.

E.g. I'm OK with landing the change to the driver before enabling building/testing of a specific sanitizer runtime.

[kcc]

is dfsan expected to work in 32-bit?

[pcc]

I would be surprised if it did. Even if it were made to work, it would probably not be very useful because of its memory requirements.

[mohit.bhakkad]

Changes in this revision:
Removed DataFlow option for MIPS32

[kcc]

Please do not enable msan on MIPS32 before enabling it on x86-32, as I've explained in another thread.

I can not speak for DFSan.

[mohit.bhakkad]

Changes in this revision:

• removed memory for mips32
• added function for mips32

[kcc]

Looks ok. Alexey?

[pcc]

lib/Driver/SanitizerArgs.cpp

140

Does -fsanitize=function work on MIPS? I'd imagine you'd need an implementation of getUBSanFunctionSignature for MIPS.

[mohit.bhakkad]

lib/Driver/SanitizerArgs.cpp

140

@Sagar is going to submit a patch which implements this

[samsonov]

Do you plan to setup the public buildbot that would run sanitizer test suite on MIPS machine?

[dsanders]

Buildbots are on my todo list and I intend to start pushing this along again once we're past the 3.5.1 release. One thing I'm wondering about for the sanitizer buildbot is how important is the amount of RAM? The systems I have available are 2GB as standard. I might be able to do a bit better than that but I've been warned about the dragons down that path so I'd prefer not to take that route unless it's really important.

[kcc]

*san uses more memory, so RAM is indeed important and you need to be careful not to overwhelm the bot.
Our Android bot has (afaict) 2Gb, so it should work for you too.

[samsonov]

I'm fine with the change, although I'd rather roll it gradually. First land the part for MSan/DFSan (which are already supported on MIPS), then TSan and -fsanitize=function when they passes the review.

[samsonov]

Let me know if you need some part of the patch committed

[mohit.bhakkad]

In D6147#28, @samsonov wrote:

Let me know if you need some part of the patch committed

Yes, Memory and DataFlow options should be there to build compiler-rt on mips. Between I just got "commit after approval" rights, I can commit this partial change if you agree.
One more thing, http://reviews.llvm.org/D6146 is also equally important for MSan, please let me know if any changes are required in it.

[samsonov]

Alright, feel free to submit the MSan/DFSan piece once they are functional (i.e. when http://reviews.llvm.org/D6146 is landed).

[mohit.bhakkad]

Enabled memory, dataflow in rL226790

lib/Driver/SanitizerArgs.cpp

 /// this group enables.
 static unsigned expandGroups(unsigned Kinds);
 
 static unsigned getToolchainUnsupportedKinds(const ToolChain &TC) {
   bool IsFreeBSD = TC.getTriple().getOS() == llvm::Triple::FreeBSD;
   bool IsLinux = TC.getTriple().getOS() == llvm::Triple::Linux;
   bool IsX86 = TC.getTriple().getArch() == llvm::Triple::x86;
   bool IsX86_64 = TC.getTriple().getArch() == llvm::Triple::x86_64;
+  bool IsMIPS32 = TC.getTriple().getArch() == llvm::Triple::mips ||
+                  TC.getTriple().getArch() == llvm::Triple::mipsel;
+  bool IsMIPS64 = TC.getTriple().getArch() == llvm::Triple::mips64 ||
+                  TC.getTriple().getArch() == llvm::Triple::mips64el;
 
   unsigned Unsupported = 0;
-  if (!(IsLinux && IsX86_64)) {
-    Unsupported |= Memory | DataFlow;
+  if (!(IsLinux && (IsX86_64 || IsMIPS32 || IsMIPS64))) {
+    Unsupported |= Memory;
   }
-  if (!((IsLinux || IsFreeBSD) && IsX86_64)) {
+  if (!(IsLinux && (IsX86_64 || IsMIPS64))) {
+    Unsupported |= DataFlow;
+  }
+  if (!((IsLinux || IsFreeBSD) && (IsX86_64 || IsMIPS64))) {
     Unsupported |= Thread;
   }
-  if (!(IsLinux && (IsX86 || IsX86_64))) {
+  if (!(IsLinux && (IsX86 || IsX86_64 || IsMIPS64))) {
     Unsupported |= Function;
   }
   return Unsupported;
 }
 
 bool SanitizerArgs::needsUbsanRt() const {
   return !UbsanTrapOnError && hasOneOf(Sanitizers, NeedsUbsanRt);
 }