tomasz-kaminski-sonarsource created this revision.
Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, dkrupp,
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware,
xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
tomasz-kaminski-sonarsource added a reviewer: xazax.hun.
Herald added a subscriber: rnkovacs.
tomasz-kaminski-sonarsource edited the summary of this revision.
tomasz-kaminski-sonarsource updated this revision to Diff 453294.
tomasz-kaminski-sonarsource added a comment.
tomasz-kaminski-sonarsource published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Fixed warrning checks and added newline.
tomasz-kaminski-sonarsource added a comment.
This is now ready for review.
In case when the prvalue is returned from the function (kind is one
of `SimpleReturnedValueKind`, `CXX17ElidedCopyReturnedValueKind`),
then it construction happens in context of the caller.
We pass `BldrCtx` explicitly, as `currBldrCtx` will always refer to callee
context.
In the following example:
`
struct Result {int value; };
Result create() { return Result{10}; }
int accessValue(Result r) { return r.value; }
void test() {
for (int i = 0; i < 2; ++i)
accessValue(create());
}
In case when the returned object was constructed directly into the
argument to a function call `accessValue(create())`, this led to
inappropriate value of `blockCount` being used to locate parameter region,
and as a consequence resulting object (from `create()`) was constructed
into a different region, that was later read by inlined invocation of
outer function (`accessValue`).
This manifests itself only in case when calling block is visited more
than once (loop in above example), as otherwise there is no in `blockCount`
value between callee and caller context.
This happens only in case when copy elision is disabled (before C++17).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D132030
Files:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
clang/lib/StaticAnalyzer/Core/CallEvent.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
clang/test/Analysis/copy-elision.cpp
Index: clang/test/Analysis/copy-elision.cpp
===================================================================
--- clang/test/Analysis/copy-elision.cpp
+++ clang/test/Analysis/copy-elision.cpp
@@ -20,6 +20,7 @@
#endif
void clang_analyzer_eval(bool);
+void clang_analyzer_dump(int);
namespace variable_functional_cast_crash {
@@ -418,3 +419,31 @@
}
} // namespace address_vector_tests
+
+namespace arg_directly_from_return_in_loop {
+
+struct Result {
+ int value;
+};
+
+Result create() {
+ return Result{10};
+}
+
+int accessValue(Result r) {
+ return r.value;
+}
+
+void test() {
+ for (int i = 0; i < 3; ++i) {
+ int v = accessValue(create());
+ if (i == 0) {
+ clang_analyzer_dump(v); // expected-warning {{10 S32b}}
+ } else {
+ clang_analyzer_dump(v); // expected-warning {{10 S32b}}
+ // was {{reg_${{[0-9]+}}<int r.value> }} for C++11
+ }
+ }
+}
+
+} // namespace arg_directly_from_return_in_loop
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -111,9 +111,15 @@
return LValue;
}
+// In case when the prvalue is returned from the function (kind is one of
+// SimpleReturnedValueKind, CXX17ElidedCopyReturnedValueKind), then
+// it's materialization happens in context of the caller.
+// We pass BldrCtx explicitly, as currBldrCtx always refers to callee's context.
SVal ExprEngine::computeObjectUnderConstruction(
- const Expr *E, ProgramStateRef State, const LocationContext *LCtx,
- const ConstructionContext *CC, EvalCallOptions &CallOpts, unsigned Idx) {
+ const Expr *E, ProgramStateRef State, const NodeBuilderContext *BldrCtx,
+ const LocationContext *LCtx, const ConstructionContext *CC,
+ EvalCallOptions &CallOpts, unsigned Idx) {
+
SValBuilder &SVB = getSValBuilder();
MemRegionManager &MRMgr = SVB.getRegionManager();
ASTContext &ACtx = SVB.getContext();
@@ -209,8 +215,11 @@
CallerLCtx = CallerLCtx->getParent();
assert(!isa<BlockInvocationContext>(CallerLCtx));
}
+
+ NodeBuilderContext CallerBldrCtx(getCoreEngine(),
+ SFC->getCallSiteBlock(), CallerLCtx);
return computeObjectUnderConstruction(
- cast<Expr>(SFC->getCallSite()), State, CallerLCtx,
+ cast<Expr>(SFC->getCallSite()), State, &CallerBldrCtx, CallerLCtx,
RTC->getConstructionContext(), CallOpts);
} else {
// We are on the top frame of the analysis. We do not know where is the
@@ -250,7 +259,7 @@
EvalCallOptions PreElideCallOpts = CallOpts;
SVal V = computeObjectUnderConstruction(
- TCC->getConstructorAfterElision(), State, LCtx,
+ TCC->getConstructorAfterElision(), State, BldrCtx, LCtx,
TCC->getConstructionContextAfterElision(), CallOpts);
// FIXME: This definition of "copy elision has not failed" is unreliable.
@@ -318,7 +327,7 @@
CallEventManager &CEMgr = getStateManager().getCallEventManager();
auto getArgLoc = [&](CallEventRef<> Caller) -> Optional<SVal> {
const LocationContext *FutureSFC =
- Caller->getCalleeStackFrame(currBldrCtx->blockCount());
+ Caller->getCalleeStackFrame(BldrCtx->blockCount());
// Return early if we are unable to reliably foresee
// the future stack frame.
if (!FutureSFC)
@@ -337,7 +346,7 @@
// because this-argument is implemented as a normal argument in
// operator call expressions but not in operator declarations.
const TypedValueRegion *TVR = Caller->getParameterLocation(
- *Caller->getAdjustedParameterIndex(Idx), currBldrCtx->blockCount());
+ *Caller->getAdjustedParameterIndex(Idx), BldrCtx->blockCount());
if (!TVR)
return None;
Index: clang/lib/StaticAnalyzer/Core/CallEvent.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -485,9 +485,9 @@
EvalCallOptions CallOpts;
ExprEngine &Engine = getState()->getStateManager().getOwningEngine();
- SVal RetVal =
- Engine.computeObjectUnderConstruction(getOriginExpr(), getState(),
- getLocationContext(), CC, CallOpts);
+ SVal RetVal = Engine.computeObjectUnderConstruction(
+ getOriginExpr(), getState(),
+ &Engine.getBuilderContext(), getLocationContext(), CC, CallOpts);
return RetVal;
}
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -727,6 +727,7 @@
/// A multi-dimensional array is also a continuous memory location in a
/// row major order, so for arr[0][0] Idx is 0 and for arr[2][2] Idx is 8.
SVal computeObjectUnderConstruction(const Expr *E, ProgramStateRef State,
+ const NodeBuilderContext *BldrCtx,
const LocationContext *LCtx,
const ConstructionContext *CC,
EvalCallOptions &CallOpts,
@@ -749,7 +750,8 @@
const ConstructionContext *CC,
EvalCallOptions &CallOpts, unsigned Idx = 0) {
- SVal V = computeObjectUnderConstruction(E, State, LCtx, CC, CallOpts, Idx);
+ SVal V = computeObjectUnderConstruction(E, State, currBldrCtx, LCtx, CC,
+ CallOpts, Idx);
State = updateObjectsUnderConstruction(V, E, State, LCtx, CC, CallOpts);
return std::make_pair(State, V);
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
@@ -214,8 +214,12 @@
const CFGBlock *Block;
const LocationContext *LC;
+ NodeBuilderContext(const CoreEngine &E, const CFGBlock *B,
+ const LocationContext *L)
+ : Eng(E), Block(B), LC(L) { assert(B); }
+
NodeBuilderContext(const CoreEngine &E, const CFGBlock *B, ExplodedNode *N)
- : Eng(E), Block(B), LC(N->getLocationContext()) { assert(B); }
+ : NodeBuilderContext(E, B, N->getLocationContext()) { }
/// Return the CFGBlock associated with this builder.
const CFGBlock *getBlock() const { return Block; }
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
