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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
