This is an archive of the discontinued LLVM Phabricator instance.

[HWASan] Port HWASan to Linux x86-64 (clang)
ClosedPublic

Authored by alekseyshl on Mar 21 2018, 10:30 AM.

Diff Detail

Event Timeline

alekseyshl created this revision.Mar 21 2018, 10:30 AM
eugenis added inline comments.Mar 21 2018, 4:06 PM
lib/Driver/SanitizerArgs.cpp
33

I did not realize hwasan was not mentioned in RequiresPIE before.
Could you add a test for it?

test/Driver/asan.c
12

Don't replace existing tests!
Add new ones.

alekseyshl added inline comments.Mar 22 2018, 10:03 AM
test/Driver/asan.c
12

I am not replacing them, I am making it consistent with all other sanitizers, which also support many more platforms than just x86_64-linux-gnu or *-unknown-linux, but have tests for that one platform only.

  • Add tests to verify that HWASan requires "-pie".
alekseyshl marked an inline comment as done.Mar 22 2018, 10:27 AM
eugenis added inline comments.Mar 22 2018, 1:48 PM
test/Driver/asan.c
12

I don't think this is a good argument for changing existing tests. It does not add any new coverage, and in this particular example it is not even consistent with surrounding RUN lines - those use i386 triple. It simply adds noise to code history.

If you want to test that hwasan supports x86_64 triple - add a new test case.

  • Revert triple change in asan.cc test
alekseyshl added inline comments.Mar 22 2018, 2:36 PM
test/Driver/asan.c
12

Ok, this file, I'm ambivalent about, it is not consistent already, but the other ones I do feel quite strongly about, they are become more consistent and consistency is good.

eugenis accepted this revision.Mar 22 2018, 5:01 PM

OK, sure, If you feel so strongly about this.

This revision is now accepted and ready to land.Mar 22 2018, 5:01 PM
This revision was automatically updated to reflect the committed changes.
Enna1 added a subscriber: Enna1.Mar 22 2023, 1:11 AM

Hi, I'm curious about why hwasan requires PIE. Is there any documents mentioned this? Thanks!