[PATCH] D105703: [hwasan] Use stack safety analysis.

Florian Mayer via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 21 15:32:15 PDT 2021


fmayer added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:410
+    Dummy.setArch(Triple::aarch64);
+    if (shouldUseStackSafetyAnalysis(Dummy, DisableOptimization))
+      AU.addRequired<StackSafetyGlobalInfoWrapperPass>();
----------------
fmayer wrote:
> vitalybuka wrote:
> > Why not just:
> > ```
> > if (!DisableOptimization || ClUseStackSafety)
> >   AU.addRequired<StackSafetyGlobalInfoWrapperPass>();
> > ```
> Because `ClUseStackSafety` defaults to true, so this will be quite useless except if the user explicitly sets it to false. So then you end up duplicating the logic in `shouldUseStackSafetyAnalysis`, which is why I went for the Dummy (thinking about it, we don't actually need to setArch on it).
How about this (mightUseStackSafetyAnalysis)? This seems like a nice tradeoff without duplication of the logic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105703



More information about the cfe-commits mailing list