[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 14:23:16 PDT 2015
On 11-09-2015 17:53, Alexey Samsonov wrote:
>
> On Fri, Sep 11, 2015 at 12:54 PM, Adhemerval Zanella <adhemerval.zanella at linaro.org <mailto:adhemerval.zanella at linaro.org>> wrote:
>
>
>
> 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> <mailto: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.
>
>
> Printf behavior does depend on the runtime flags. Also, I would really like to have all the compatibility checks
> etc. grouped together, and after we have the most basic parts of runtime initialized.
>
>
> 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?
>
>
> Yes please. At least some parts of sanitizer initialization.
Right, what about http://reviews.llvm.org/D12819 ?
>
>
>
> >
> >
> >
> >
> > 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.
>
>
> Sure, then just left the VMA checks before the mmap (which could fail), but move the flags initialization earlier.
>
>
>
> >
> >
> >
> > // 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.
>
>
> OK
>
>
>
> >
> >
> > +
> > } // 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> <mailto: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> <mailto:vonosmas at gmail.com <mailto:vonosmas at gmail.com>>
>
>
>
>
> --
> Alexey Samsonov
> vonosmas at gmail.com <mailto:vonosmas at gmail.com>
More information about the llvm-commits
mailing list