[clang] a11e51e - [analyzer] Track trivial copy/move constructors and initializer lists in the BugReporter
via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 5 08:06:37 PDT 2022
Author: isuckatcs
Date: 2022-09-05T17:06:27+02:00
New Revision: a11e51e91f73e413eb044bc0b2f2e205735618d7
URL: https://github.com/llvm/llvm-project/commit/a11e51e91f73e413eb044bc0b2f2e205735618d7
DIFF: https://github.com/llvm/llvm-project/commit/a11e51e91f73e413eb044bc0b2f2e205735618d7.diff
LOG: [analyzer] Track trivial copy/move constructors and initializer lists in the BugReporter
If an object has a trivial copy/move constructor, it's not inlined
on invocation but a trivial copy is performed instead. This patch
handles trivial copies in the bug reporter by matching the field
regions of the 2 objects involved in the copy/move construction,
and tracking the appropriate region further. This patch also
introduces some support for tracking values in initializer lists.
Differential Revision: https://reviews.llvm.org/D131262
Added:
clang/test/Analysis/ctor-bug-path.cpp
Modified:
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index 3a90c37a36dab..336376b78ce80 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1410,6 +1410,83 @@ static void showBRDefaultDiagnostics(llvm::raw_svector_ostream &OS,
}
}
+static bool isTrivialCopyOrMoveCtor(const CXXConstructExpr *CE) {
+ if (!CE)
+ return false;
+
+ const auto *CtorDecl = CE->getConstructor();
+
+ return CtorDecl->isCopyOrMoveConstructor() && CtorDecl->isTrivial();
+}
+
+static const Expr *tryExtractInitializerFromList(const InitListExpr *ILE,
+ const MemRegion *R) {
+
+ const auto *TVR = dyn_cast_or_null<TypedValueRegion>(R);
+
+ if (!TVR)
+ return nullptr;
+
+ const auto ITy = ILE->getType().getCanonicalType();
+
+ // Push each sub-region onto the stack.
+ std::stack<const TypedValueRegion *> TVRStack;
+ while (isa<FieldRegion>(TVR) || isa<ElementRegion>(TVR)) {
+ // We found a region that matches the type of the init list,
+ // so we assume this is the outer-most region. This can happen
+ // if the initializer list is inside a class. If our assumption
+ // is wrong, we return a nullptr in the end.
+ if (ITy == TVR->getValueType().getCanonicalType())
+ break;
+
+ TVRStack.push(TVR);
+ TVR = cast<TypedValueRegion>(TVR->getSuperRegion());
+ }
+
+ // If the type of the outer most region doesn't match the type
+ // of the ILE, we can't match the ILE and the region.
+ if (ITy != TVR->getValueType().getCanonicalType())
+ return nullptr;
+
+ const Expr *Init = ILE;
+ while (!TVRStack.empty()) {
+ TVR = TVRStack.top();
+ TVRStack.pop();
+
+ // We hit something that's not an init list before
+ // running out of regions, so we most likely failed.
+ if (!isa<InitListExpr>(Init))
+ return nullptr;
+
+ ILE = cast<InitListExpr>(Init);
+ auto NumInits = ILE->getNumInits();
+
+ if (const auto *FR = dyn_cast<FieldRegion>(TVR)) {
+ const auto *FD = FR->getDecl();
+
+ if (FD->getFieldIndex() >= NumInits)
+ return nullptr;
+
+ Init = ILE->getInit(FD->getFieldIndex());
+ } else if (const auto *ER = dyn_cast<ElementRegion>(TVR)) {
+ const auto Ind = ER->getIndex();
+
+ // If index is symbolic, we can't figure out which expression
+ // belongs to the region.
+ if (!Ind.isConstant())
+ return nullptr;
+
+ const auto IndVal = Ind.getAsInteger()->getLimitedValue();
+ if (IndVal >= NumInits)
+ return nullptr;
+
+ Init = ILE->getInit(IndVal);
+ }
+ }
+
+ return Init;
+}
+
PathDiagnosticPieceRef StoreSiteFinder::VisitNode(const ExplodedNode *Succ,
BugReporterContext &BRC,
PathSensitiveBugReport &BR) {
@@ -1456,12 +1533,88 @@ PathDiagnosticPieceRef StoreSiteFinder::VisitNode(const ExplodedNode *Succ,
StoreSite = Succ;
- // If this is an assignment expression, we can track the value
- // being assigned.
- if (Optional<PostStmt> P = Succ->getLocationAs<PostStmt>())
- if (const BinaryOperator *BO = P->getStmtAs<BinaryOperator>())
+ if (Optional<PostStmt> P = Succ->getLocationAs<PostStmt>()) {
+ // If this is an assignment expression, we can track the value
+ // being assigned.
+ if (const BinaryOperator *BO = P->getStmtAs<BinaryOperator>()) {
if (BO->isAssignmentOp())
InitE = BO->getRHS();
+ }
+ // If we have a declaration like 'S s{1,2}' that needs special
+ // handling, we handle it here.
+ else if (const auto *DS = P->getStmtAs<DeclStmt>()) {
+ const auto *Decl = DS->getSingleDecl();
+ if (isa<VarDecl>(Decl)) {
+ const auto *VD = cast<VarDecl>(Decl);
+
+ // FIXME: Here we only track the inner most region, so we lose
+ // information, but it's still better than a crash or no information
+ // at all.
+ //
+ // E.g.: The region we have is 's.s2.s3.s4.y' and we only track 'y',
+ // and throw away the rest.
+ if (const auto *ILE = dyn_cast<InitListExpr>(VD->getInit()))
+ InitE = tryExtractInitializerFromList(ILE, R);
+ }
+ } else if (const auto *CE = P->getStmtAs<CXXConstructExpr>()) {
+
+ const auto State = Succ->getState();
+
+ if (isTrivialCopyOrMoveCtor(CE) && isa<SubRegion>(R)) {
+ // Migrate the field regions from the current object to
+ // the parent object. If we track 'a.y.e' and encounter
+ // 'S a = b' then we need to track 'b.y.e'.
+
+ // Push the regions to a stack, from last to first, so
+ // considering the example above the stack will look like
+ // (bottom) 'e' -> 'y' (top).
+
+ std::stack<const SubRegion *> SRStack;
+ const SubRegion *SR = cast<SubRegion>(R);
+ while (isa<FieldRegion>(SR) || isa<ElementRegion>(SR)) {
+ SRStack.push(SR);
+ SR = cast<SubRegion>(SR->getSuperRegion());
+ }
+
+ // Get the region for the object we copied/moved from.
+ const auto *OriginEx = CE->getArg(0);
+ const auto OriginVal =
+ State->getSVal(OriginEx, Succ->getLocationContext());
+
+ // Pop the stored field regions and apply them to the origin
+ // object in the same order we had them on the copy.
+ // OriginField will evolve like 'b' -> 'b.y' -> 'b.y.e'.
+ SVal OriginField = OriginVal;
+ while (!SRStack.empty()) {
+ const auto *TopR = SRStack.top();
+ SRStack.pop();
+
+ if (const auto *FR = dyn_cast<FieldRegion>(TopR)) {
+ OriginField = State->getLValue(FR->getDecl(), OriginField);
+ } else if (const auto *ER = dyn_cast<ElementRegion>(TopR)) {
+ OriginField = State->getLValue(ER->getElementType(),
+ ER->getIndex(), OriginField);
+ } else {
+ // FIXME: handle other region type
+ }
+ }
+
+ // Track 'b.y.e'.
+ getParentTracker().track(V, OriginField.getAsRegion(), Options);
+ InitE = OriginEx;
+ }
+ }
+ // This branch can occur in cases like `Ctor() : field{ x, y } {}'.
+ else if (const auto *ILE = P->getStmtAs<InitListExpr>()) {
+ // FIXME: Here we only track the top level region, so we lose
+ // information, but it's still better than a crash or no information
+ // at all.
+ //
+ // E.g.: The region we have is 's.s2.s3.s4.y' and we only track 'y', and
+ // throw away the rest.
+ InitE = tryExtractInitializerFromList(ILE, R);
+ }
+ }
// If this is a call entry, the variable should be a parameter.
// FIXME: Handle CXXThisRegion as well. (This is not a priority because
@@ -2395,6 +2548,29 @@ class PRValueHandler final : public ExpressionHandler {
if (!RVNode)
return {};
+ Tracker::Result CombinedResult;
+ Tracker &Parent = getParentTracker();
+
+ const auto track = [&CombinedResult, &Parent, ExprNode,
+ Opts](const Expr *Inner) {
+ CombinedResult.combineWith(Parent.track(Inner, ExprNode, Opts));
+ };
+
+ // FIXME: Initializer lists can appear in many
diff erent contexts
+ // and most of them needs a special handling. For now let's handle
+ // what we can. If the initializer list only has 1 element, we track
+ // that.
+ // This snippet even handles nesting, e.g.: int *x{{{{{y}}}}};
+ if (const auto *ILE = dyn_cast<InitListExpr>(E)) {
+ if (ILE->getNumInits() == 1) {
+ track(ILE->getInit(0));
+
+ return CombinedResult;
+ }
+
+ return {};
+ }
+
ProgramStateRef RVState = RVNode->getState();
SVal V = RVState->getSValAsScalarOrLoc(E, RVNode->getLocationContext());
const auto *BO = dyn_cast<BinaryOperator>(E);
@@ -2406,13 +2582,6 @@ class PRValueHandler final : public ExpressionHandler {
SVal LHSV = RVState->getSVal(BO->getLHS(), RVNode->getLocationContext());
// Track both LHS and RHS of a multiplication.
- Tracker::Result CombinedResult;
- Tracker &Parent = getParentTracker();
-
- const auto track = [&CombinedResult, &Parent, ExprNode, Opts](Expr *Inner) {
- CombinedResult.combineWith(Parent.track(Inner, ExprNode, Opts));
- };
-
if (BO->getOpcode() == BO_Mul) {
if (LHSV.isZeroConstant())
track(BO->getLHS());
diff --git a/clang/test/Analysis/ctor-bug-path.cpp b/clang/test/Analysis/ctor-bug-path.cpp
new file mode 100644
index 0000000000000..107e5b0c3389c
--- /dev/null
+++ b/clang/test/Analysis/ctor-bug-path.cpp
@@ -0,0 +1,271 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-output=text -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-output=text -std=c++17 -verify %s
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+namespace copyMoveTrackCtor {
+struct S {
+ int *p1, *p2;
+ S(int *a, int *b) : p1(a), p2(b) {} // expected-note{{Null pointer value stored to 's.p1'}}
+};
+
+void CtorDirect() {
+ int *x = nullptr, *y = nullptr;
+ // expected-note at -1{{'x' initialized to a null pointer value}}
+
+ S s(x, y);
+ // expected-note at -1{{Passing null pointer value via 1st parameter 'a'}}
+ // expected-note at -2{{Calling constructor for 'S'}}
+ // expected-note at -3{{Returning from constructor for 'S'}}
+ // expected-note at -4{{'s' initialized here}}
+ S s2 = s; // expected-note{{Null pointer value stored to 's2.p1'}}
+ // expected-note at -1{{'s2' initialized here}}
+ S s3 = s2; // expected-note{{Null pointer value stored to 's3.p1'}}
+ // expected-note at -1{{'s3' initialized here}}
+ S s4 = std::move(s3); // expected-note{{Null pointer value stored to 's4.p1'}}
+ // expected-note at -1{{'s4' initialized here}}
+ S s5 = s4; // expected-note{{Null pointer value stored to 's5.p1'}}
+
+ int i = *s5.p1; // expected-warning{{Dereference of null pointer}}
+ // expected-note at -1{{Dereference of null pointer (loaded from field 'p1')}}
+
+ (void) i;
+}
+} // namespace copyMoveTrackCtor
+
+namespace copyMoveTrackInitList {
+struct S {
+ int *p1, *p2;
+};
+
+void InitListDirect() {
+ int *x = nullptr, *y = nullptr; //expected-note{{'x' initialized to a null pointer value}}
+
+ S s{x, y}; //expected-note{{'s.p1' initialized to a null pointer value}}
+ //expected-note at -1{{'s' initialized here}}
+ S s2 = s; // expected-note{{Null pointer value stored to 's2.p1'}}
+ // expected-note at -1{{'s2' initialized here}}
+ S s3 = s2; // expected-note{{Null pointer value stored to 's3.p1'}}
+ // expected-note at -1{{'s3' initialized here}}
+ S s4 = std::move(s3); // expected-note{{Null pointer value stored to 's4.p1'}}
+ // expected-note at -1{{'s4' initialized here}}
+ S s5 = s4; // expected-note{{Null pointer value stored to 's5.p1'}}
+
+ int i = *s5.p1; // expected-warning{{Dereference of null pointer}}
+ // expected-note at -1{{Dereference of null pointer (loaded from field 'p1')}}
+
+ (void) i;
+}
+
+void InitListAssign() {
+ int *x = nullptr, *y = nullptr; //expected-note{{'x' initialized to a null pointer value}}
+
+ S s = {x, y}; //expected-note{{'s.p1' initialized to a null pointer value}}
+ //expected-note at -1{{'s' initialized here}}
+ S s2 = s; // expected-note{{Null pointer value stored to 's2.p1'}}
+ // expected-note at -1{{'s2' initialized here}}
+ S s3 = s2; // expected-note{{Null pointer value stored to 's3.p1'}}
+ // expected-note at -1{{'s3' initialized here}}
+ S s4 = std::move(s3); // expected-note{{Null pointer value stored to 's4.p1'}}
+ // expected-note at -1{{'s4' initialized here}}
+ S s5 = s4; // expected-note{{Null pointer value stored to 's5.p1'}}
+
+ int i = *s5.p1; // expected-warning{{Dereference of null pointer}}
+ // expected-note at -1{{Dereference of null pointer (loaded from field 'p1')}}
+
+ (void) i;
+}
+
+} // namespace copyMoveTrackInitList
+
+namespace copyMoveTrackCtorMemberInitList {
+struct S {
+ int *p1, *p2;
+ S(int *a, int *b) : p1{a}, p2{b} {} // expected-note{{Null pointer value stored to 's.p1'}}
+};
+
+void CtorDirect() {
+ int *x = nullptr, *y = nullptr;
+ // expected-note at -1{{'x' initialized to a null pointer value}}
+
+ S s{x, y};
+ // expected-note at -1{{Passing null pointer value via 1st parameter 'a'}}
+ // expected-note at -2{{Calling constructor for 'S'}}
+ // expected-note at -3{{Returning from constructor for 'S'}}
+ // expected-note at -4{{'s' initialized here}}
+ S s2 = s; // expected-note{{Null pointer value stored to 's2.p1'}}
+ // expected-note at -1{{'s2' initialized here}}
+ S s3 = s2; // expected-note{{Null pointer value stored to 's3.p1'}}
+ // expected-note at -1{{'s3' initialized here}}
+ S s4 = std::move(s3); // expected-note{{Null pointer value stored to 's4.p1'}}
+ // expected-note at -1{{'s4' initialized here}}
+ S s5 = s4; // expected-note{{Null pointer value stored to 's5.p1'}}
+
+ int i = *s5.p1; // expected-warning{{Dereference of null pointer}}
+ // expected-note at -1{{Dereference of null pointer (loaded from field 'p1')}}
+
+ (void) i;
+}
+} // namespace copyMoveTrackCtorMemberInitList
+
+namespace directInitList {
+struct S {
+ int *p1, *p2;
+};
+
+void InitListDirect() {
+ int *x = nullptr, *y = nullptr; //expected-note{{'y' initialized to a null pointer value}}
+
+ S s{x, y}; //expected-note{{'s.p2' initialized to a null pointer value}}
+
+ int i = *s.p2; // expected-warning{{Dereference of null pointer}}
+ // expected-note at -1{{Dereference of null pointer}}
+ (void) i;
+}
+} // namespace directInitList
+
+namespace directNestedInitList {
+struct S2 {
+ int *p1, *p2;
+};
+
+struct S {
+ S2 s;
+};
+
+void InitListNestedDirect() {
+ int *x = nullptr, *y = nullptr; //expected-note{{'y' initialized to a null pointer value}}
+
+ //FIXME: Put more information to the notes.
+ S s{x, y}; //expected-note{{'s.s.p2' initialized to a null pointer value}}
+
+ int i = *s.s.p2; // expected-warning{{Dereference of null pointer}}
+ // expected-note at -1{{Dereference of null pointer}}
+ (void) i;
+}
+} // namespace directNestedInitList
+
+#if __cplusplus >= 201703L
+
+namespace structuredBinding {
+struct S {
+ int *p1, *p2;
+};
+
+void StructuredBinding() {
+ int *x = nullptr, *y = nullptr;
+ //expected-note at -1{{'y' initialized to a null pointer value}}
+
+ S s{x, y};
+ //expected-note at -1{{'s.p2' initialized to a null pointer value}}
+ //expected-note at -2{{'s' initialized here}}
+
+ auto [a, b] = s; //expected-note{{Null pointer value stored to '.p2'}}
+
+ int i = *b; // expected-warning{{Dereference of null pointer}}
+ // expected-note at -1{{Dereference of null pointer}}
+ (void) i;
+}
+} // namespace structuredBinding
+
+#endif
+
+namespace nestedCtorInitializer {
+ struct S5{
+ int *x, *y;
+ };
+
+ struct S4 {
+ S5 s5;
+ };
+
+ struct S3 {
+ S4 s4;
+ };
+
+ struct S2 {
+ S3 s3;
+ };
+
+ struct S {
+ S2 s2;
+
+ //FIXME: Put more information to the notes.
+ S(int *x, int *y) : s2{x, y} {};
+ // expected-note at -1{{Null pointer value stored to 's.s2.s3.s4.s5.y'}}
+ };
+
+ void nestedCtorInit(){
+ int *x = nullptr, *y = nullptr; // expected-note{{'y' initialized to a null pointer value}}
+
+ S s{x,y};
+ // expected-note at -1{{Passing null pointer value via 2nd parameter}}
+ // expected-note at -2{{Calling constructor for 'S'}}
+ // expected-note at -3{{Returning from constructor for 'S'}}
+
+ int i = *s.s2.s3.s4.s5.y; // expected-warning{{Dereference of null pointer}}
+ // expected-note at -1{{Dereference of null pointer}}
+ (void) i;
+ }
+} // namespace nestedCtorInitializer
+
+namespace NestedRegionTrack {
+struct N {
+ int *e;
+};
+
+struct S {
+ N y;
+};
+
+void NestedRegionTrack() {
+ int *x = nullptr, *y = nullptr;
+ // expected-note at -1{{'y' initialized to a null pointer value}}
+
+ // Test for nested single element initializer list here.
+ S a{{{{{{{{y}}}}}}}};
+ // expected-note at -1{{'a.y.e' initialized to a null pointer value}}
+ // expected-note at -2{{'a' initialized here}}
+ // expected-warning at -3{{too many braces around scalar initializer}}
+ // expected-warning at -4{{too many braces around scalar initializer}}
+ // expected-warning at -5{{too many braces around scalar initializer}}
+ // expected-warning at -6{{too many braces around scalar initializer}}
+ // expected-warning at -7{{too many braces around scalar initializer}}
+
+ S b = a; // expected-note{{Null pointer value stored to 'b.y.e'}}
+
+ int i = *b.y.e;
+ // expected-warning at -1{{Dereference of null pointer}}
+ // expected-note at -2{{Dereference of null pointer}}
+ (void) i;
+ (void) x;
+}
+
+} // namespace NestedRegionTrack
+
+namespace NestedElementRegionTrack {
+struct N {
+ int *arr[2];
+};
+
+struct S {
+ N n;
+};
+
+void NestedElementRegionTrack() {
+ int *x = nullptr, *y = nullptr;
+ // expected-note at -1{{'y' initialized to a null pointer value}}
+
+ S a{{x, y}};
+ // expected-note at -1{{Initializing to a null pointer value}}
+ // expected-note at -2{{'a' initialized here}}
+
+ S b = a; // expected-note{{Storing null pointer value}}
+
+ int i = *b.n.arr[1];
+ // expected-warning at -1{{Dereference of null pointer}}
+ // expected-note at -2{{Dereference of null pointer}}
+ (void) i;
+}
+
+} // namespace NestedElementRegionTrack
More information about the cfe-commits
mailing list