[Asan] Disable inline instrumentation for MMU-less targets

[kosarev]

Summary

This patch starts a series of changes dedicated to adding support for targets that do not have hardware memory managers.

D30583 contains the overview patch for this work.

[dvyukov]

You can use -asan-instrumentation-with-call-threshold=0 for this without any changes to llvm.

[filcab]

If the only thing you'll want to do with kUseSoftwareMemoryManager is to always emit calls, then you should use what @dvyukov mentioned: -asan-instrumentation-with-call-threshold=0
If, eventually, you'll want this flag to mean additional things, then let's add a *run-time* option for it, when it's needed.

CMakeLists.txt

432

Why is this a compile-time option for llvm?

[kosarev]

Indeed there's no need to introduce a special clang build. Thanks for pointing out.

Updated.

[filcab]

Is this something orthogonal to the target triple? Can you know if it's an MMU-less target from the triple?
I'd prefer to switch on the target triple if possible (some component of the triple that would allow us to assume it's an MMU-less target).

CMakeLists.txt

329

That option's name + what it does isn't what the string says.

lib/asan/tests/CMakeLists.txt

345 ↗

(On Diff #93150)

# The software memory manager requires out-of-line instrumentation.
if(NOT SANITIZER_USE_SOFTWARE_MEMORY_MANAGER)
  add_asan_tests_for_arch_and_kind(${arch} "-inline")
endif()
add_asan_tests_for_arch_and_kind(${arch} "-with-calls"
    -mllvm -asan-instrumentation-with-call-threshold=0)

And no need to have the argument passing above.

At least for now this seems enough.

test/asan/lit.cfg

86 ↗

(On Diff #93150)

Check out how we handle COMPILER_RT_DEBUG, and use pythonize_bool with SANITIZER_USE_SOFTWARE_MEMORY_MANAGER.

[kosarev]

lib/asan/tests/CMakeLists.txt

345 ↗

(On Diff #93150)

This way building the "-with-calls" tests will fail due to duplicate "-mllvm -asan-instrumentation-with-call-threshold=0" option. With SANITIZER_USE_SOFTWARE_MEMORY_MANAGER turned on, the option is part of the command string by default.

[kosarev]

Fixed. Thanks for reviewing.

[kosarev]

In D31395#712098, @filcab wrote:

Is this something orthogonal to the target triple? Can you know if it's an MMU-less target from the triple?
I'd prefer to switch on the target triple if possible (some component of the triple that would allow us to assume it's an MMU-less target).

It seems to be orthogonal to the architecture and vendor fields and we can probably deduce presence of MMU from the OS and environment fields of the triple. For development and testing purposes, however, we still would need a cmake-level option that makes it possible to build and run Asan tests with the software memory manager enabled on Linux and other platforms that do have MMU.

[filcab]

I don't like adding a new configuration option, but the argument about testing the no-MMU code paths in an MMU environment makes sense (*assuming* those code paths make sense in that environment).
Do you plan on setting up a bot for this configuration? (Right after this patch it's not absolutely needed, but I'm guessing it will soon, when the other patches get here)

lib/asan/tests/CMakeLists.txt

345 ↗

(On Diff #93150)

Not if you remove the chunk above adding that flag. That way you only need to change here.

test/asan/lit.cfg

87 ↗

(On Diff #93243)

No need to use getattr if you're always setting it in lit.common.configured.in. Just if config. use_software_memory_manager: should do it, no?

[kosarev]

Updated once again.

[kosarev]

In D31395#712216, @filcab wrote:

Do you plan on setting up a bot for this configuration? (Right after this patch it's not absolutely needed, but I'm guessing it will soon, when the other patches get here)

Yes, we are planning establishing a buildbot at some point later when there is more of the no-MMU specific code to maintain.

[filcab]

I have no problems with this patch (assuming the rest of the patches will follow), but double-check that @kcc is ok with it (I haven't been paying too much attention to the discussion thread, sorry).

[kosarev]

Thanks Filipe.

Dmitry, do you think we can start with this?

[kosarev]

Ping.

[dvyukov]

I was on vacation/ooo, sorry for delays.

LGTM (assuming the new option is for testing on linux)

[dvyukov]

Ivan, do you have commit access? Or I need to land it?

[kosarev]

Yes, I can commit it, thanks. I just thought that we probably should postpone committing it until we know we can do the rest in a form suitable for putting to mainline.

[kosarev]

Dmitry, with your permission I'm going to update and commit this patch as it seems to be useful regardless of which way we support sanitizers on MMU-less platforms.

One more thing: now that D31592 is landed, would you mind if we do the same for Tsan?

Thanks.

[dvyukov]

Dmitry, with your permission I'm going to update and commit this patch as it seems to be useful regardless of which way we support sanitizers on MMU-less platforms.

If you plan to continue working on this, I think it's fine.
It's not end-user-visible, so we are not committing to anything (no pun intended).

[dvyukov]

One more thing: now that D31592 is landed, would you mind if we do the same for Tsan?

In general getting rid of the reverse mapping is positive.
However, there is a potential problem with tsan. The memory accesses handling functions are super performance sensitive, and we tried very hard to get number of memory accesses in general and spill in particular to absolute minimum (you can see that insane lib/tsan/check_analyze.sh).
Now, there is a reason to pass shadow address to ReportRace -- this way we convert address to shadow in the very beginning of the function and then it's not needed anymore (if we need original address in ReportRace we instead convert shadow back), this frees 1 register for the function. If you pass address to ReportRace, it means that it needs to be preserved alive for the whole duration of the function, which can cause additional spills.
So if you can make it without affecting fast path performance, then I am all for it. Otherwise, my plan was to get rid of the reverse mapping only in the nommu mode under ifdefs.

CMakeLists.txt

set(LLVM_ALL_TARGETS
   XCore
   )
 
 # List of targets with JIT support:
 set(LLVM_TARGETS_WITH_JIT X86 PowerPC AArch64 ARM Mips SystemZ)
 
 set(LLVM_TARGETS_TO_BUILD "all"
     CACHE STRING "Semicolon-separated list of targets to build, or \"all\".")
 
 set(LLVM_EXPERIMENTAL_TARGETS_TO_BUILD ""
   CACHE STRING "Semicolon-separated list of experimental targets to build.")
 
 option(BUILD_SHARED_LIBS
   "Build all libraries as shared libraries instead of static" OFF)
 
 option(LLVM_ENABLE_BACKTRACES "Enable embedding backtraces on crash." ON)
 if(LLVM_ENABLE_BACKTRACES)
if( LLVM_USE_OPROFILE )
   if( NOT CMAKE_SYSTEM_NAME MATCHES "Linux" )
     message(FATAL_ERROR "OProfile support is available on Linux only.")
   endif( NOT CMAKE_SYSTEM_NAME MATCHES "Linux" )
 endif( LLVM_USE_OPROFILE )
 
 set(LLVM_USE_SANITIZER "" CACHE STRING
   "Define the sanitizer used to build binaries and tests.")
 
+option(LLVM_SANITIZERS_TO_USE_SOFTWARE_MEMORY_MANAGER
+  "Experimental: Do not rely on availability of mmap() in sanitizers." OFF)
+if(LLVM_SANITIZERS_TO_USE_SOFTWARE_MEMORY_MANAGER)
+  add_definitions(-DLLVM_SANITIZERS_TO_USE_SOFTWARE_MEMORY_MANAGER)
+endif()
+
 option(LLVM_USE_SPLIT_DWARF
   "Use -gsplit-dwarf when compiling llvm." OFF)
 
 option(LLVM_POLLY_LINK_INTO_TOOLS "Statically link Polly into tools (if available)" ON)
 option(LLVM_POLLY_BUILD "Build LLVM with Polly" ON)
 
 if (EXISTS ${LLVM_MAIN_SRC_DIR}/tools/polly/CMakeLists.txt)
   set(POLLY_IN_TREE TRUE)

lib/Transforms/Instrumentation/AddressSanitizer.cpp

 static const char *const kAsanAllocaPoison = "__asan_alloca_poison";
 static const char *const kAsanAllocasUnpoison = "__asan_allocas_unpoison";
 
 // Accesses sizes are powers of two: 1, 2, 4, 8, 16.
 static const size_t kNumberOfAccessSizes = 5;
 
 static const unsigned kAllocaRzSize = 32;
 
+// With the software memory manager enabled we need the RTL to handle all
+// shadow memory accesses.
+#ifdef LLVM_SANITIZERS_TO_USE_SOFTWARE_MEMORY_MANAGER
+static const bool kUseSoftwareMemoryManager = true;
+#else
+static const bool kUseSoftwareMemoryManager = false;
+#endif
+
 // Command-line flags.
 static cl::opt<bool> ClEnableKasan(
     "asan-kernel", cl::desc("Enable KernelAddressSanitizer instrumentation"),
     cl::Hidden, cl::init(false));
 static cl::opt<bool> ClRecover(
     "asan-recover",
     cl::desc("Enable recovery mode (continue-after-error)."),
     cl::Hidden, cl::init(false));
for (auto &Inst : BB) {
       }
       ToInstrument.push_back(&Inst);
       NumInsnsPerBB++;
       if (NumInsnsPerBB >= ClMaxInsnsToInstrumentPerBB) break;
     }
   }
 
   bool UseCalls =
-      CompileKernel ||
+      CompileKernel || kUseSoftwareMemoryManager ||
       (ClInstrumentationWithCallsThreshold >= 0 &&
        ToInstrument.size() > (unsigned)ClInstrumentationWithCallsThreshold);
   const DataLayout &DL = F.getParent()->getDataLayout();
   ObjectSizeOpts ObjSizeOpts;
   ObjSizeOpts.RoundToAlign = true;
   ObjectSizeOffsetVisitor ObjSizeVis(DL, TLI, F.getContext(), ObjSizeOpts);
 
   // Instrument.