This is an archive of the discontinued LLVM Phabricator instance.

[sanitizer] Trivial portion of the port to Myriad RTEMS
ClosedPublic

Authored by waltl on May 4 2018, 1:44 PM.

Details

Summary

This commit contains the trivial portion of the port of ASan to
Myriad RTEMS.

  • Whitelist platform in sanitizer_platform.h, ubsan_platform.h
  • Turn off general interception
  • Use memset for FastPoisonShadow
  • Define interception wrappers
  • Set errno symbol correctly
  • Enable ASAN_LOW_MEMORY
  • Disable slow unwinding
  • Use fuchsia offline symbolizer
  • Disable common code for: InitializeShadowMemory, CreateMainThread, AsanThread::ThreadStart, StartReportDeadlySignal, MaybeReportNonExecRegion.

Diff Detail

Event Timeline

waltl created this revision.May 4 2018, 1:44 PM
krytarowski added inline comments.May 4 2018, 1:53 PM
compiler-rt/lib/asan/asan_interceptors_memintrinsics.cc
15 ↗(On Diff #145272)

rationale?

compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h
104 ↗(On Diff #145272)

I think it will look better if we will split OSes here and document what linker, what versions supports what.

kcc added inline comments.May 4 2018, 2:27 PM
compiler-rt/lib/asan/asan_interceptors_memintrinsics.cc
15 ↗(On Diff #145272)

please don't.
this file should not include any system headers.

lebedev.ri added inline comments.
compiler-rt/lib/asan/asan_interceptors_memintrinsics.cc
15 ↗(On Diff #145272)

Was passing of -nostdinc / -nostdlib / -nostdlibinc / -nostdinc++ / -nostdlib++ compiler options when building compiler-rt / xray been considered?
I wonder if that would help enforce these rules.

waltl updated this revision to Diff 145851.May 8 2018, 8:03 PM

Remove asan_interceptors_memintrinsics.cc change (with the
controversial header). Will propose something else in a separate
patch.

waltl marked 3 inline comments as done.May 8 2018, 8:05 PM
waltl added inline comments.
compiler-rt/lib/asan/asan_interceptors_memintrinsics.cc
15 ↗(On Diff #145272)

I reverted it. Will propose something else in a separate patch.

waltl edited the summary of this revision. (Show Details)May 8 2018, 8:06 PM
waltl updated this revision to Diff 146000.May 9 2018, 2:02 PM

Disable preinit array -- we don't need it.

waltl edited the summary of this revision. (Show Details)May 9 2018, 2:02 PM
waltl marked an inline comment as done.
waltl added inline comments.
compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h
104 ↗(On Diff #145272)

I reverted the change -- turns out we don't need it.

alekseyshl added inline comments.May 15 2018, 2:46 PM
compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
32

How SANITIZER_FUCHSIA and SANITIZER_RTEMS are related to "Windows is not POSIX!" message? Both are not Windows indeed.

compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cc
19–20

The comment should mention RTEMS too.

waltl marked an inline comment as done.May 16 2018, 8:24 AM
waltl added inline comments.
compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
32

The check was from r309536 back when a port was either Windows or
POSIX, predating Fuchsia/RTEMS. Even back then the error message was
a little cryptic as it would fire when a port is neither.

I can rewrite this to be slightly more clear, but I wonder whether we need
this check at all? Vitaly can you weigh in?

alekseyshl added inline comments.May 16 2018, 12:39 PM
compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
32

It feels like now it should be

#if SI_WINDOWS && SI_POSIX
# error "Windows is not POSIX!"
#endif

to keep the original check around.

waltl updated this revision to Diff 147220.May 16 2018, 5:50 PM

Address CR comments

waltl marked 4 inline comments as done.May 16 2018, 5:54 PM
waltl added inline comments.
compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cc
19–20

I updated the comment to reflect the changes in r332157.

waltl updated this revision to Diff 147296.May 17 2018, 4:56 AM
waltl marked an inline comment as done.

Update #endif markers

alekseyshl accepted this revision.May 17 2018, 4:09 PM
This revision is now accepted and ready to land.May 17 2018, 4:09 PM
This revision was automatically updated to reflect the committed changes.
Herald added a subscriber: Restricted Project. · View Herald TranscriptMay 17 2018, 5:47 PM