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

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added inline comments.


================
Comment at: clang/test/CodeGen/hwasan-stack-safety-analysis-asm.c:4
+
+int main(int argc, char **argv) {
+  char buf[10];
----------------
this patch mostly change code under llvm/ so tests should be also there, as IR tests




================
Comment at: llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h:32
+                                  Triple TargetTriple = {});
   PreservedAnalyses run(Module &M, ModuleAnalysisManager &MAM);
   static bool isRequired() { return true; }
----------------
Why not from M.getTargetTriple() ?


================
Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:200
 
+bool shouldUsePageAliases(const Triple &TargetTriple) {
+  return ClUsePageAliases && TargetTriple.getArch() == Triple::x86_64;
----------------
Could you please extract these function in a separate patch?
LLVM likes small close to NFC patches.


================
Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:390
+  void getAnalysisUsage(AnalysisUsage &AU) const override {
+    if (shouldUseStackSafetyAnalysis(TargetTriple)) {
+      AU.addRequired<StackSafetyGlobalInfoWrapperPass>();
----------------
why we need to check TargetTriple for that?


================
Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:1275
 bool HWAddressSanitizer::isInterestingAlloca(const AllocaInst &AI) {
+  // clang-format off
   return (AI.getAllocatedType()->isSized() &&
----------------
Instead of // clang-format off
could you replace this with

```
# comment1
if (...)
  return false;

# comment2
if (...)
  return false;
...

```

in a separate patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105703