vitalybuka added a comment.
In D105703#2883666 <https://reviews.llvm.org/D105703#2883666>, @eugenis wrote:
> Btw Vitaly, will this work with LTO out of the box? I think we used to add
> pre-LTO StackSafety pass explicitly for memtag only, but it looks like that
> code is gone.
What do you mean gone? Can you show me the patch?
We probably needs something to add analysis for HWASAN as well.
================
Comment at: llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h:32
+ Triple TargetTriple = {});
PreservedAnalyses run(Module &M, ModuleAnalysisManager &MAM);
static bool isRequired() { return true; }
----------------
fmayer wrote:
> vitalybuka wrote:
> > fmayer wrote:
> > > vitalybuka wrote:
> > > > fmayer wrote:
> > > > > vitalybuka wrote:
> > > > > > Why not from M.getTargetTriple() ?
> > > > > Mostly for consistency with the legacy pass. Either way is fine for
> > > > > me though, what do you prefer?
> > > > I don't know if will cause any issues, but usually most passes get
> > > > triple from the module.
> > > > I prefer we stay consistent with the rest of the code if possible.
> > > >
> > > I'll leave it as is, for consistency within this file, as we need to do
> > > it this way for the new pass manager.
> > That's what I don't like, passing Triple from BackendUtil.cpp
> > I've updated the patch. You can download it with "arc patch D105703"
> OK, no strong feelings but now we addRequire the pass even if we don't need
> it, so there isn't much point turning it off anymore, no?
In case of legacy pass? Correct. I don't think we need to over optimize this
deprecated case which is going to be removed at some point anyway.
Let's check DisableOptimization as we have it here anyway, but avoid forwarding
Triple.
For new PM it's irrelevant as it's does not declare requirements in advance.
================
Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:427
+ bool DisableOptimization) {
assert(!CompileKernel || Recover);
+ return new HWAddressSanitizerLegacyPass(CompileKernel, Recover,
----------------
@eugenis unrelated to the patch, but why do we this args if then we
assert(!CompileKernel || Recover);
================
Comment at:
llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll:5
+; ModuleID =
'/usr/local/google/home/fmayer/llvm-project/clang/test/CodeGen/hwasan-stack-safety-analysis.c'
+source_filename =
"/usr/local/google/home/fmayer/llvm-project/clang/test/CodeGen/hwasan-stack-safety-analysis.c"
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
----------------
you don't need to hardcode local path here
================
Comment at:
llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll:10
+; Function Attrs: mustprogress nofree nounwind uwtable willreturn
+define i32 @test_load(i32* %a) sanitize_hwaddress {
+entry:
----------------
would be nice to have also function which have __hwasan_generate_tag in either
case to make sure
-hwasan-use-stack-safety=1 does not disable tagging completely.
================
Comment at:
llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll:27
+
+attributes #0 = { mustprogress nofree nounwind uwtable willreturn
"frame-pointer"="non-leaf" "min-legal-vector-width"="0"
"no-trapping-math"="true" "stack-protector-buffer-size"="8"
"target-cpu"="generic" "target-features"="+neon" }
+attributes #1 = { argmemonly mustprogress nofree nosync nounwind willreturn }
----------------
usually we remove irrelevant attributes #* and module flags !* to make test
simpler
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105703/new/
https://reviews.llvm.org/D105703
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
