NoQ added a comment.
Looks great, thanks! Minor inline stuff.
================
Comment at: lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp:116
+ const StackSpaceRegion *S = cast<StackSpaceRegion>(R->getMemorySpace());
+ return S->getStackFrame() != C.getLocationContext()->getCurrentStackFrame();
+}
----------------
Because stack address escapes are not fatal errors, it is not necessarily true
that any other stack frame context here would be an ancestor of the current
context. It may be an escaped stack variable from a function that has exited
long time ago. You might want to check the `getParent()` chain.
Hmm, or rather rename the function to "isNotInCurrentFrame" because that's how
you seem to be using it.
================
Comment at: lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp:173
+ // To avoid false-positives (for now) we ignore all the blocks
+ // which have capatured a variable of the type "dispatch_semaphore_t".
+ if (isSemaphoreCaptured(*B.getDecl()))
----------------
Typo: cap~~a~~tured -> captured.
================
Comment at: lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp:177
+ for (const MemRegion *Region : getCapturedStackRegions(B, C)) {
+ if (isa<BlockDataRegion>(Region))
+ continue;
----------------
Hmm, why would such region even be in the list?
================
Comment at: lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp:184
+ BT_capturedstackasync = llvm::make_unique<BuiltinBug>(
+ this, "Address of stack-allocated memory");
+ SmallString<512> Buf;
----------------
I think bugtype should describe the bug, so that the user knew what sort of bug
it is by looking at the list. Maybe add "...is captured" here?
Repository:
rL LLVM
https://reviews.llvm.org/D39438
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
