kstoimenov marked 4 inline comments as done. kstoimenov added inline comments.
================ Comment at: clang/test/CodeGen/asan-stack-safety-analysis.c:1 +// REQUIRES: x86-registered-target + ---------------- fmayer wrote: > fmayer wrote: > > Should this file be in llvm/test/Instrumentation/ instead? Also consider > > porting some of the tests from HWASan > > (https://github.com/llvm/llvm-project/blob/main/llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll). > Uhm nevermind my first sentence, sorry about that. This is of course the > right location. I've removed this file. Also added a .ll test for the internal flag. ================ Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:812 bool runOnFunction(Function &F) override { + if (ClUseStackSafety) { + report_fatal_error("Stack safety analysis is not supported " ---------------- vitalybuka wrote: > functionality of the pass should be the same, so I don't see why this is the > fatal error. so just ignore it? > > btw, why don't you want to support it? I would like to support it, but I am not sure how to get an instance of StackSafetyGlobalAnalysis, because in the legacy pass manger I don't have access to AnalysisManager<Function> &AM. If you know how to make this work, please let me know. As far as the report_fatal_error my logic is that if someone sets ClUseStackSafety flag they expect it to work and if it silently doesn't it is a bad user experience. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112098/new/ https://reviews.llvm.org/D112098 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
