[PATCH] D105201: [hwasan] Detect use after scope within function.

[Florian Mayer via Phabricator via llvm-commits]

fmayer added inline comments.


================
Comment at: compiler-rt/test/hwasan/TestCases/stack-uas.c:10
+
+// RUN: %clang_hwasan -fexperimental-new-pass-manager -mllvm -hwasan-use-after-scope -g %s -o %t && not %run %t 2>&1 | FileCheck %s
+// RUN: %clang_hwasan -fno-experimental-new-pass-manager -mllvm -hwasan-use-after-scope -g %s -o %t && not %run %t 2>&1 | FileCheck %s
----------------
eugenis wrote:
> fmayer wrote:
> > vitalybuka wrote:
> > > vitalybuka wrote:
> > > > It looks all these test ported version of asan
> > > > it would be nice if in a separate patch you commit them as exact copy (with XFAIL: *)
> > > > 
> > > > so in D105201 we can see only change you need to make for HWASAN
> > > Actually compiler-rt should not be in this patch. Ideally two additional patches
> > > 1. D105201 only llvm/
> > > 2. I guess we  will add proper clang fronted flag to avoid "-mllvm -hwasan-use-after-scope" ? similar -fsanitize-address-use-after-scope. Doing this before 4 will save one patch to replace them "-mllvm -hwasan-use-after-scope" -> -fsanitize-hwaddress-use-after-scope
> > > 3. Clone compiler-rt tests with XFAIL:*
> > > 4. Update and enable -hwasan-use-after-scope
> > > 
> > > D105201 changes llvm/lib/Transforms/Instrumentation/, so we need tests in llvm/test/Instrumentation/, similar to AddressSanitizer/lifetime.ll
> > > 
> > > if you move AArch64StackTagging.cpp into a separate refactoring NFC patch, it does not need new tests
> > >  
> > Ad 2. I will let eugenis comment on that, but to me it seems a lot of the HWAsan feature flags are passed via -mllvm without any intention to make them first-class clang options. Especially given we want to make this the default soon (see the other comment thread between eugenis and me).
> Let's look at performance and code size impact first, but I'd like that to just be on all the time, and the -mllvm option can be used to opt out if something goes wrong.
> 
> Note that memtag does not have a frontend flag for use-after-scope, either.
So is it okay to leave as is? I'll add an LLVM IR level test as well to this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105201