This is an archive of the discontinued LLVM Phabricator instance.

[HWASan] Add aliasing flag and enable HWASan to use it.
ClosedPublic

Authored by morehouse on May 11 2021, 4:59 PM.

Details

Summary

-fsanitize-hwaddress-experimental-aliasing is intended to distinguish
aliasing mode from LAM mode on x86_64. check-hwasan is configured
to use aliasing mode while check-hwasan-lam is configured to use LAM
mode.

The current patch doesn't actually do anything differently in the two
modes. A subsequent patch will actually build the separate runtimes
and use them in each mode.

Currently LAM mode tests must be run in an emulator that
has LAM support. To ensure LAM mode isn't broken by future patches, I
will next set up a QEMU buildbot to run the HWASan tests in LAM.

Diff Detail

Event Timeline

morehouse created this revision.May 11 2021, 4:59 PM
morehouse requested review of this revision.May 11 2021, 4:59 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMay 11 2021, 4:59 PM
Herald added subscribers: Restricted Project, cfe-commits. · View Herald Transcript
vitalybuka added inline comments.May 11 2021, 7:13 PM
clang/include/clang/Basic/Sanitizers.def
55–59 ↗(On Diff #344598)

if it's experimental, why not just "-fsanitize=hwaddress -mllvm -havasan-lam=1" ?

compiler-rt/test/hwasan/TestCases/Linux/vfork.c
7

What does here XFAIL mean, do not test in x86_64 ?

morehouse added inline comments.May 12 2021, 8:29 AM
clang/include/clang/Basic/Sanitizers.def
55–59 ↗(On Diff #344598)

Well, -mllvm indicates a flag for LLVM, but we need to change the Clang behavior to link with the LAM-enabled HWASan runtime. It seems to me that we should use a flag directed to Clang for this.

Maybe it's possible to parse the -mllvm flag before the point where we need to choose a runtime (I'm not sure), but it seems simpler to do it this way.

compiler-rt/test/hwasan/TestCases/Linux/vfork.c
7

It means the test will run in x86_64 and is expected to fail.

Under LAM the test actually passes, since manually tagging the upper bits in the stack pointers succeeds. So we need to distinguish between x86 LAM mode and x86 aliasing mode, which we do with the new pointer-tagging feature.

vitalybuka added inline comments.May 12 2021, 3:09 PM
clang/include/clang/Basic/Sanitizers.def
55–59 ↗(On Diff #344598)

I see, I expected you convince you to keep same runtime lib, but I see your reasoning on the another patch.

What do you thing about still keeping it hwasan and add hwasan internal flag like we added for asan
e.g. -fsanitize-hwasan-use-lam=1. Later when -mlam is available we will drop this flag.

eugenis added inline comments.May 12 2021, 3:39 PM
clang/include/clang/Basic/Sanitizers.def
55–59 ↗(On Diff #344598)

I agree, this is not a new "sanitizer", just a different kind of hwasan.

Can we simply add "-mlam" right now, with the single purpose of selecting the new hwasan mode? It can grow more functionality later.

morehouse marked 4 inline comments as done.
  • Use -mlam option instead of -fsanitize=lam.
eugenis added inline comments.May 13 2021, 12:02 PM
clang/include/clang/Driver/Options.td
4180

Hmm this might be a good argument to go with something else for now.
What would -mlam actually affect, besides hwasan? Significant bits computation?

If we see LAM as future production mode for HWASan, and aliasing - more of a temporary, proof-of-concept thing, then how about -fexperimental-sanitize-hwaddress-aliasing flag?

morehouse updated this revision to Diff 345284.May 13 2021, 2:14 PM
  • Remove diffbase.
  • Use -fexperimental-sanitize-hwaddress-aliasing instead of -mlam.
clang/include/clang/Driver/Options.td
4180

Hmm this might be a good argument to go with something else for now.
What would -mlam actually affect, besides hwasan? Significant bits computation?

Not sure. I could imagine at least defining a preprocessor guard for conditional compilation (similar to -mavx).

If we see LAM as future production mode for HWASan, and aliasing - more of a temporary, proof-of-concept thing, then how about -fexperimental-sanitize-hwaddress-aliasing flag?

Sounds good, done.

morehouse retitled this revision from [HWASan] Add -fsanitize=lam flag and enable HWASan to use it. to [HWASan] Add aliasing flag and enable HWASan to use it..May 13 2021, 2:19 PM
morehouse edited the summary of this revision. (Show Details)
morehouse edited the summary of this revision. (Show Details)May 13 2021, 2:23 PM
vitalybuka added inline comments.May 13 2021, 2:42 PM
clang/include/clang/Driver/Options.td
1551

Please switch to fsanitize-hwaddress-experimental-aliasing for consistency with other "fsanitize-<sanitizer>" flags

morehouse updated this revision to Diff 345295.May 13 2021, 2:48 PM
morehouse marked an inline comment as done.
  • Rename flag to -fsanitize-hwaddress-experimental-aliasing.
vitalybuka accepted this revision.May 13 2021, 4:09 PM

LGTM

compiler-rt/test/hwasan/lit.site.cfg.py.in
10

ENABLE_ALIASES -> HWASAB_ENABLE_ALIASES for consistency?

This revision is now accepted and ready to land.May 13 2021, 4:09 PM
morehouse updated this revision to Diff 345317.May 13 2021, 5:23 PM
morehouse marked an inline comment as done.
  • s/ENABLE_ALIASES/HWASAN_ENABLE_ALIASES
morehouse edited the summary of this revision. (Show Details)May 13 2021, 5:24 PM
This revision was automatically updated to reflect the committed changes.