[PATCH] D103841: [llvm][hwasan] Decouple use of the TLS global for getting the shadow base and using the frame record feature

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:1050
+
+  if (!WithFrameRecord && ShadowBase)
     return;
----------------
leonardchan wrote:
> eugenis wrote:
> > I think this will break Linux kernel build.
> > They use -fsanitize=kernel-hwaddress with one of
> > ```
> > -hwasan-instrument-with-calls=1
> > ```
> > or
> > ```
> > -hwasan-mapping-offset=
> > ```
> > The assumption here is that InTls == 0 means there is no thread slot, and therefore no frame recording.
> > 
> Perhaps we could set the default value of `-hwasan-record-stack-history` to false so users who don't expect frame recording don't suddenly see TLS being used? It would mean people who do use frame recording won't see it anymore and will need to manually add `-hwasan-record-stack-history=1`, but maybe that won't break existing users.
These are internal compiler flags, they should not have users, ideally. The right thing should happen based on -fsanitize flag and the target triple, and the right thing in this case is to enable frame recording on supported platforms.

I'd rather put this check under InTls, or add something like ShadowMapping::WithFrameRecord and make the decision in ShadowMapping::init.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103841/new/

https://reviews.llvm.org/D103841