[PATCH] D112098: [ASan] Added stack safety support in address sanitizer.

[Kirill Stoimenov via Phabricator via llvm-commits]

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