Page MenuHomePhabricator

[WIP][clang][Fuchsia] Support HWASan for Fuchsia
Changes PlannedPublic

Authored by leonardchan on Nov 13 2020, 3:10 PM.

Details

Summary

This tracks all the compiler/llvm changes needed for building Fuchsia with the HWASan variant.

Diff Detail

Event Timeline

leonardchan created this revision.Nov 13 2020, 3:10 PM
Herald added subscribers: Restricted Project, mgorny. · View Herald TranscriptNov 13 2020, 3:10 PM
leonardchan requested review of this revision.Nov 13 2020, 3:10 PM
leonardchan planned changes to this revision.
pcc added a subscriber: pcc.Nov 13 2020, 4:11 PM

How does Zircon handle tagged addresses in syscalls? Are they handled equivalently to Linux's tagged address ABI?

Locally I'm able to build and run the bringup.arm64 config (that is launch the shell and run some commands).

Herald added a project: Restricted Project. · View Herald TranscriptJan 15 2021, 5:09 PM
leonardchan added a comment.EditedJan 15 2021, 5:13 PM
In D91466#2395297, @pcc wrote:

How does Zircon handle tagged addresses in syscalls? Are they handled equivalently to Linux's tagged address ABI?

Woops, accidentally let this slide. I'm guessing you saw this in the Fuchsia patch, but I believe we haven't made a final decision yet on how syscalls will handle tagged pointers.

phosek added inline comments.Feb 18 2021, 12:03 AM
clang/lib/Driver/ToolChains/Fuchsia.cpp
100–101 ↗(On Diff #317118)

Nit: can you move this above TSan so it's alphabetically sorted?

222 ↗(On Diff #317118)
222–229 ↗(On Diff #317118)

Can you move this above relative-vtables so we keep all sanitizers together?

225 ↗(On Diff #317118)
compiler-rt/lib/hwasan/hwasan_fuchsia.cpp
123–125

I don't think this applies to Fuchsia.

compiler-rt/lib/hwasan/hwasan_poisoning.cpp
22

It might be better to move this function to a per-platform file rather than conditionally compiling the entire body.

@leonardchan would it be possible to break up this patch into multiple changes:

  • Clang driver
  • Cache file
  • hwasan_new_delete.cpp
  • HWAddressSanitizer.cpp
  • sanitizer_platform_limits_fuchsia.h
  • sanitizer_symbolizer_markup.cpp
  • the rest

Rebase against submitted changes.

leonardchan planned changes to this revision.May 6 2021, 3:24 PM
leonardchan added inline comments.
compiler-rt/lib/hwasan/hwasan_dynamic_shadow.cpp
122

Add comment. Fuchsia never uses dynamic shadow, which is what this sets up in the Linux implementation.

From linux implemenation:

// Call the ifunc resolver for __hwasan_shadow and fill in its GOT entry. This
// needs to be done before other ifunc resolvers (which are handled by libc)
// because a resolver might read __hwasan_shadow.
compiler-rt/lib/hwasan/hwasan_fuchsia.cpp
47–52

Compare hwasan thread tracking implementation with asan + fuchsia implementation. HWASan uses different sanitizer machinery.

54

This can be replaced with a zircon-equivalent check that checks if TBI is enabled. This function is Linux-specific.

TODO: File a bug that tracks this once the TBI stuff is addressed.

61

See equivalent for other asan+lsan implementations. We use fuchsia sanitizer hooks instead of atexit. __sanitizer_exit_hook is what's used.

65–75

Don't define these. Move this logic into the hooks.

Ask why these are in the public header when we already have the thread hooks.

106–112

This is compiler-defined and should be abstracted out. Shouldn't have duplicate definitions.

106–131

This could probably be shared code with the Linux implementation. Move out of hwasan.

147–148

Ideally, the checks on these uninitialized globals should not be called.

Update to some feedback from first round of reviews. Minor refactoring of code that can be shared between the linux and fuchsia implementations. Still need to discuss refactoring the hwasan threads implementation.

leonardchan planned changes to this revision.Mon, May 17, 10:38 AM