Sorry i just noticed this email. I will take a look next week.
On Sun, Jan 30, 2022, 2:50 AM Simon Pilgrim via Phabricator <
revi...@reviews.llvm.org> wrote:
> RKSimon added inline comments.
>
>
>
> Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1534
>}
RKSimon added inline comments.
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1534
} else if (StoreInst *SI = dyn_cast(I)) {
-if (!ClInstrumentWrites || ignoreAccess(SI->getPointerOperand()))
+if (!ClInstrumentWrites || ignoreAccess(LI, SI->getPoi
RKSimon added inline comments.
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1534
} else if (StoreInst *SI = dyn_cast(I)) {
-if (!ClInstrumentWrites || ignoreAccess(SI->getPointerOperand()))
+if (!ClInstrumentWrites || ignoreAccess(LI, SI->getPoi
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
kstoimenov marked an inline comment as done.
Closed by commit rG3f1aca58df8f: [ASan] Added stack safety support in address
sanitizer. (authored by kstoimenov).
Reposit
kstoimenov updated this revision to Diff 384902.
kstoimenov added a comment.
Moved AddressSanitizer back to the loop.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112098/new/
https://reviews.llvm.org/D112098
Files:
llvm/lib/Transforms/Instrumen
vitalybuka accepted this revision.
vitalybuka added inline comments.
This revision is now accepted and ready to land.
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1328
+ ClUseStackSafety ? &MAM.getResult(M) :
nullptr;
+ AddressSanitizer FunctionSani
kstoimenov updated this revision to Diff 384877.
kstoimenov marked 2 inline comments as done.
kstoimenov added a comment.
Fixed tests.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112098/new/
https://reviews.llvm.org/D112098
Files:
llvm/lib/Tra
vitalybuka accepted this revision.
vitalybuka added inline comments.
This revision is now accepted and ready to land.
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1511
+ if (ClUseStackSafety && SSGI != nullptr && findAllocaForValue(Ptr) &&
+ SSGI->
kstoimenov updated this revision to Diff 384587.
kstoimenov added a comment.
Restored StackSafetyAnalysis.h.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112098/new/
https://reviews.llvm.org/D112098
Files:
llvm/lib/Transforms/Instrumentation/Ad
kstoimenov updated this revision to Diff 384583.
kstoimenov added a comment.
After removing function pass.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112098/new/
https://reviews.llvm.org/D112098
Files:
llvm/include/llvm/Analysis/StackSafetyAn
kstoimenov added inline comments.
Comment at: llvm/include/llvm/Analysis/StackSafetyAnalysis.h:93
+ bool invalidate(Module &, const PreservedAnalyses &,
+ ModuleAnalysisManager::Invalidator &) {
+return false;
eugenis wrote:
> vitalybuka wro
eugenis added inline comments.
Comment at: llvm/include/llvm/Analysis/StackSafetyAnalysis.h:93
+ bool invalidate(Module &, const PreservedAnalyses &,
+ ModuleAnalysisManager::Invalidator &) {
+return false;
vitalybuka wrote:
> vitalybuka wro
vitalybuka added subscribers: morehouse, pcc, eugenis.
vitalybuka added inline comments.
Comment at: llvm/include/llvm/Analysis/StackSafetyAnalysis.h:93
+ bool invalidate(Module &, const PreservedAnalyses &,
+ ModuleAnalysisManager::Invalidator &) {
+return
kstoimenov updated this revision to Diff 382486.
kstoimenov added a comment.
s/I/LI/.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112098/new/
https://reviews.llvm.org/D112098
Files:
llvm/include/llvm/Analysis/StackSafetyAnalysis.h
llvm/lib/T
vitalybuka requested changes to this revision.
vitalybuka added inline comments.
This revision now requires changes to proceed.
Comment at: llvm/include/llvm/Analysis/StackSafetyAnalysis.h:93
+ bool invalidate(Module &, const PreservedAnalyses &,
+ ModuleAnalysi
kstoimenov marked 2 inline comments as done.
kstoimenov added a comment.
PTAL.
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1265
GlobalsMetadata ASanGlobalsMetadataAnalysis::run(Module &M,
ModuleAnalysis
vitalybuka accepted this revision.
vitalybuka added a comment.
This revision is now accepted and ready to land.
Can you please fix in a separate patch llvm::runPassPipeline: it runs
ModulePass after the function pass for -passes=asan-pipeline
Comment at: llvm/lib/Transforms/In
kstoimenov updated this revision to Diff 382483.
kstoimenov added a comment.
Removed {} after if.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112098/new/
https://reviews.llvm.org/D112098
Files:
llvm/include/llvm/Analysis/StackSafetyAnalysis.h
kstoimenov updated this revision to Diff 382482.
kstoimenov added a comment.
Added a comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112098/new/
https://reviews.llvm.org/D112098
Files:
llvm/include/llvm/Analysis/StackSafetyAnalysis.h
ll
kstoimenov updated this revision to Diff 382481.
kstoimenov marked 2 inline comments as done.
kstoimenov added a comment.
Fixed test.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112098/new/
https://reviews.llvm.org/D112098
Files:
llvm/include/
vitalybuka added inline comments.
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:813
+const StackSafetyGlobalInfo *SSI = nullptr;
+if (ClUseStackSafety) {
+ SSI = &getAnalysis().getResult();
usually we don't use {} for one liner
kstoimenov added a comment.
This should be good for re-review now. PTAL.
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-commit
kstoimenov added inline comments.
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 "
fmayer wrote:
kstoimenov updated this revision to Diff 381391.
kstoimenov added a comment.
Added support for legacy pass manager and a test for it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112098/new/
https://reviews.llvm.org/D112098
Files:
llvm/include/
fmayer added inline comments.
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 "
kstoimenov wrote:
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/
fmayer added a comment.
I think you also want to use `SSI->isSafe(AllocaInst*)` in
`isInterestingAlloca` to prevent use-after-scope instrumentation if all
accesses are safe.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112098/new/
https://review
kstoimenov updated this revision to Diff 381365.
kstoimenov added a comment.
Removed the top level flag and addressed comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112098/new/
https://reviews.llvm.org/D112098
Files:
llvm/include/llvm/A
fmayer added inline comments.
Comment at: clang/test/CodeGen/asan-stack-safety-analysis.c:1
+// REQUIRES: x86-registered-target
+
fmayer wrote:
> Should this file be in llvm/test/Instrumentation/ instead? Also consider
> porting some of the tests from HWASan
>
fmayer added inline comments.
Comment at: clang/test/CodeGen/asan-stack-safety-analysis.c:1
+// REQUIRES: x86-registered-target
+
Should this file be in llvm/test/Instrumentation/ instead? Also consider
porting some of the tests from HWASan
(https://github.com/
vitalybuka added inline comments.
Comment at: clang/include/clang/Basic/CodeGenOptions.def:227
CODEGENOPT(SanitizeAddressUseOdrIndicator, 1, 0) ///< Enable ODR indicator
globals
+CODEGENOPT(SanitizeAddressUseStackSafety, 1, 0) ///< Use stack safety result.
CODEGENOPT(SanitizeM
fmayer added inline comments.
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1499-1502
+ if (ClUseStackSafety && findAllocaForValue(Ptr)) {
+if (SSI && SSI->stackAccessIsSafe(*Inst)) {
+ return true;
+}
Doesn't need nested if.
kstoimenov updated this revision to Diff 380829.
kstoimenov added a comment.
Fixed the test to pass.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112098/new/
https://reviews.llvm.org/D112098
Files:
clang/include/clang/Basic/CodeGenOptions.def
kstoimenov created this revision.
Herald added subscribers: ormris, dexonsmith, dang, hiraditya.
kstoimenov requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: llvm-commits, cfe-commits, sstefan1.
Herald added projects: clang, LLVM.
Added and implemente
34 matches
Mail list logo
