[compiler-rt] r247413 - [compiler-rt] [sanitizers] Add VMA size check at runtime

Adhemerval Zanella via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 11 12:54:44 PDT 2015



On 11-09-2015 15:20, Alexey Samsonov wrote:
> I don't think this is a good change as-is. Please follow-up with the code review fixing the issues pointed below.
> 
> 
> On Fri, Sep 11, 2015 at 6:55 AM, Adhemerval Zanella via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
> 
>     Author: azanella
>     Date: Fri Sep 11 08:55:00 2015
>     New Revision: 247413
> 
>     URL: http://llvm.org/viewvc/llvm-project?rev=247413&view=rev
>     Log:
>     [compiler-rt] [sanitizers] Add VMA size check at runtime
> 
>     This patch adds a runtime check for asan, dfsan, msan, and tsan for
>     architectures that support multiple VMA size (like aarch64).  Currently
>     the check only prints a warning indicating which is the VMA built and
>     expected against the one detected at runtime.
> 
> 
>     Modified:
>         compiler-rt/trunk/lib/asan/asan_rtl.cc
>         compiler-rt/trunk/lib/dfsan/dfsan.cc
>         compiler-rt/trunk/lib/msan/msan.cc
>         compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h
>         compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.cc
>         compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc
>         compiler-rt/trunk/lib/tsan/rtl/tsan_rtl.cc
> 
>     Modified: compiler-rt/trunk/lib/asan/asan_rtl.cc
>     URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_rtl.cc?rev=247413&r1=247412&r2=247413&view=diff
>     ==============================================================================
>     --- compiler-rt/trunk/lib/asan/asan_rtl.cc (original)
>     +++ compiler-rt/trunk/lib/asan/asan_rtl.cc Fri Sep 11 08:55:00 2015
>     @@ -585,6 +585,7 @@ void NOINLINE __asan_set_death_callback(
>      // Initialize as requested from instrumented application code.
>      // We use this call as a trigger to wake up ASan from deactivated state.
>      void __asan_init() {
>     +  CheckVMASize();
> 
> 
> Hm? I feel that this is far too early - the runtime is not properly initialized at this point yet. I believe you have to find a different place
> for CheckVMASize() call. It should at least be after the runtime flags initialization in AsanInitInternal() - for example where we call
> 
>   AsanCheckIncompatibleRT();
>   AsanCheckDynamicRTPrereqs();

Right, but currently the only facility it requires is the warning using Printf, which 
in my understanding it not dependent of the sanitizer initialization. And different
that he AsanCheckXXX it is not really dependent of runtime information rather than 
the kernel VMA.

So do we really require to move it after sanitizer initialization to print the
warning message?

> 
> 
>  
> 
>        AsanActivate();
>        AsanInitInternal();
>      }
> 
>     Modified: compiler-rt/trunk/lib/dfsan/dfsan.cc
>     URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/dfsan/dfsan.cc?rev=247413&r1=247412&r2=247413&view=diff
>     ==============================================================================
>     --- compiler-rt/trunk/lib/dfsan/dfsan.cc (original)
>     +++ compiler-rt/trunk/lib/dfsan/dfsan.cc Fri Sep 11 08:55:00 2015
>     @@ -399,6 +399,8 @@ static void dfsan_fini() {
>      }
> 
>      static void dfsan_init(int argc, char **argv, char **envp) {
>     +  CheckVMASize();
>     +
>        MmapFixedNoReserve(kShadowAddr, kUnusedAddr - kShadowAddr);
> 
> 
> 
> Same here. Please parse the flags first. If you want to prevent calling Mmap() without checking VMA first, move
> the InitializeFlags() call up (this would be the correct change).

The idea is not to prevent the mmap neither to bail out the binary, but rather
inform the use that different VMA is assumed in sanitizer and the one kernel
is providing.

>  
> 
> 
>        // Protect the region of memory we don't use, to preserve the one-to-one
> 
>     Modified: compiler-rt/trunk/lib/msan/msan.cc
>     URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/msan/msan.cc?rev=247413&r1=247412&r2=247413&view=diff
>     ==============================================================================
>     --- compiler-rt/trunk/lib/msan/msan.cc (original)
>     +++ compiler-rt/trunk/lib/msan/msan.cc Fri Sep 11 08:55:00 2015
>     @@ -375,6 +375,8 @@ void __msan_init() {
>        msan_init_is_running = 1;
>        SanitizerToolName = "MemorySanitizer";
> 
>     +  CheckVMASize();
>     +
> 
> 
> Same here.
>  
> 
>        InitTlsSize();
> 
>        CacheBinaryName();
> 
>     Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h
>     URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h?rev=247413&r1=247412&r2=247413&view=diff
>     ==============================================================================
>     --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h (original)
>     +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h Fri Sep 11 08:55:00 2015
>     @@ -97,6 +97,8 @@ void DecreaseTotalMmap(uptr size);
>      uptr GetRSS();
>      void NoHugePagesInRegion(uptr addr, uptr length);
>      void DontDumpShadowMemory(uptr addr, uptr length);
>     +// Check if the built VMA size matches the runtime one.
>     +void CheckVMASize();
> 
> 
> 
> 
>      // InternalScopedBuffer can be used instead of large stack arrays to
>      // keep frame size low.
> 
>     Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.cc
>     URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.cc?rev=247413&r1=247412&r2=247413&view=diff
>     ==============================================================================
>     --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.cc (original)
>     +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.cc Fri Sep 11 08:55:00 2015
>     @@ -324,6 +324,22 @@ SignalContext SignalContext::Create(void
>        return SignalContext(context, addr, pc, sp, bp);
>      }
> 
>     +// This function check is the built VMA matches the runtime one for
>     +// architectures with multiple VMA size.
>     +void CheckVMASize() {
>     +#ifdef __aarch64__
>     +  static const unsigned kBuiltVMA = SANITIZER_AARCH64_VMA;
>     +  unsigned maxRuntimeVMA =
>     +    (MostSignificantSetBitIndex(GET_CURRENT_FRAME()) + 1);
>     +  if (kBuiltVMA != maxRuntimeVMA) {
>     +    Printf("WARNING: %s runtime VMA is not the one built for.\n",
>     +      SanitizerToolName);
>     +    Printf("\tBuilt VMA:   %u bits\n", kBuiltVMA);
>     +    Printf("\tRuntime VMA: %u bits\n", maxRuntimeVMA);
>     +  }
>     +#endif
>     +}
> 
> 
> Can the program proceed with invalid VMA? Or it's better to just kill it in this case?

Usually the program will fail to mmap either the shadow/origin/etc, print the maps
and bail out.  So I will let to sanitizer error handling to exit if it not able
to run properly.

>  
> 
>     +
>      }  // namespace __sanitizer
> 
>      #endif  // SANITIZER_POSIX
> 
>     Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc
>     URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc?rev=247413&r1=247412&r2=247413&view=diff
>     ==============================================================================
>     --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc (original)
>     +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc Fri Sep 11 08:55:00 2015
>     @@ -755,6 +755,10 @@ uptr ReadLongProcessName(/*out*/char *bu
>        return ReadBinaryName(buf, buf_len);
>      }
> 
>     +void CheckVMASize() {
>     +  // Do nothing.
>     +}
>     +
>      }  // namespace __sanitizer
> 
>      #endif  // _WIN32
> 
>     Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_rtl.cc
>     URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_rtl.cc?rev=247413&r1=247412&r2=247413&view=diff
>     ==============================================================================
>     --- compiler-rt/trunk/lib/tsan/rtl/tsan_rtl.cc (original)
>     +++ compiler-rt/trunk/lib/tsan/rtl/tsan_rtl.cc Fri Sep 11 08:55:00 2015
>     @@ -312,6 +312,9 @@ void Initialize(ThreadState *thr) {
>        if (is_initialized)
>          return;
>        is_initialized = true;
>     +
>     +  CheckVMASize();
> 
> 
> 
> Same here.
>  
> 
>     +
>        // We are not ready to handle interceptors yet.
>        ScopedIgnoreInterceptors ignore;
>        SanitizerToolName = "ThreadSanitizer";
> 
> 
>     _______________________________________________
>     llvm-commits mailing list
>     llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>     http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 
> 
> 
> -- 
> Alexey Samsonov
> vonosmas at gmail.com <mailto:vonosmas at gmail.com>


More information about the llvm-commits mailing list