diff --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp --- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -1410,6 +1410,83 @@ } } +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(R); + + if (!TVR) + return nullptr; + + const auto ITy = ILE->getType().getCanonicalType(); + + // Push each sub-region onto the stack. + std::stack TVRStack; + while (isa(TVR) || isa(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(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(Init)) + return nullptr; + + ILE = cast(Init); + auto NumInits = ILE->getNumInits(); + + if (const auto *FR = dyn_cast(TVR)) { + const auto *FD = FR->getDecl(); + + if (FD->getFieldIndex() >= NumInits) + return nullptr; + + Init = ILE->getInit(FD->getFieldIndex()); + } else if (const auto *ER = dyn_cast(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 @@ StoreSite = Succ; - // If this is an assignment expression, we can track the value - // being assigned. - if (Optional P = Succ->getLocationAs()) - if (const BinaryOperator *BO = P->getStmtAs()) + if (Optional P = Succ->getLocationAs()) { + // If this is an assignment expression, we can track the value + // being assigned. + if (const BinaryOperator *BO = P->getStmtAs()) { 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()) { + const auto *Decl = DS->getSingleDecl(); + if (isa(Decl)) { + const auto *VD = cast(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(VD->getInit())) + InitE = tryExtractInitializerFromList(ILE, R); + } + } else if (const auto *CE = P->getStmtAs()) { + + const auto State = Succ->getState(); + + if (isTrivialCopyOrMoveCtor(CE) && isa(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 SRStack; + const SubRegion *SR = cast(R); + while (isa(SR) || isa(SR)) { + SRStack.push(SR); + SR = cast(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(TopR)) { + OriginField = State->getLValue(FR->getDecl(), OriginField); + } else if (const auto *ER = dyn_cast(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()) { + // 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 @@ 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 different 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(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(E); @@ -2406,13 +2582,6 @@ 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 --- /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@-1{{'x' initialized to a null pointer value}} + + S s(x, y); + // expected-note@-1{{Passing null pointer value via 1st parameter 'a'}} + // expected-note@-2{{Calling constructor for 'S'}} + // expected-note@-3{{Returning from constructor for 'S'}} + // expected-note@-4{{'s' initialized here}} + S s2 = s; // expected-note{{Null pointer value stored to 's2.p1'}} + // expected-note@-1{{'s2' initialized here}} + S s3 = s2; // expected-note{{Null pointer value stored to 's3.p1'}} + // expected-note@-1{{'s3' initialized here}} + S s4 = std::move(s3); // expected-note{{Null pointer value stored to 's4.p1'}} + // expected-note@-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@-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@-1{{'s' initialized here}} + S s2 = s; // expected-note{{Null pointer value stored to 's2.p1'}} + // expected-note@-1{{'s2' initialized here}} + S s3 = s2; // expected-note{{Null pointer value stored to 's3.p1'}} + // expected-note@-1{{'s3' initialized here}} + S s4 = std::move(s3); // expected-note{{Null pointer value stored to 's4.p1'}} + // expected-note@-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@-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@-1{{'s' initialized here}} + S s2 = s; // expected-note{{Null pointer value stored to 's2.p1'}} + // expected-note@-1{{'s2' initialized here}} + S s3 = s2; // expected-note{{Null pointer value stored to 's3.p1'}} + // expected-note@-1{{'s3' initialized here}} + S s4 = std::move(s3); // expected-note{{Null pointer value stored to 's4.p1'}} + // expected-note@-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@-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@-1{{'x' initialized to a null pointer value}} + + S s{x, y}; + // expected-note@-1{{Passing null pointer value via 1st parameter 'a'}} + // expected-note@-2{{Calling constructor for 'S'}} + // expected-note@-3{{Returning from constructor for 'S'}} + // expected-note@-4{{'s' initialized here}} + S s2 = s; // expected-note{{Null pointer value stored to 's2.p1'}} + // expected-note@-1{{'s2' initialized here}} + S s3 = s2; // expected-note{{Null pointer value stored to 's3.p1'}} + // expected-note@-1{{'s3' initialized here}} + S s4 = std::move(s3); // expected-note{{Null pointer value stored to 's4.p1'}} + // expected-note@-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@-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@-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@-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@-1{{'y' initialized to a null pointer value}} + + S s{x, y}; + //expected-note@-1{{'s.p2' initialized to a null pointer value}} + //expected-note@-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@-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@-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@-1{{Passing null pointer value via 2nd parameter}} + // expected-note@-2{{Calling constructor for 'S'}} + // expected-note@-3{{Returning from constructor for 'S'}} + + int i = *s.s2.s3.s4.s5.y; // expected-warning{{Dereference of null pointer}} + // expected-note@-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@-1{{'y' initialized to a null pointer value}} + + // Test for nested single element initializer list here. + S a{{{{{{{{y}}}}}}}}; + // expected-note@-1{{'a.y.e' initialized to a null pointer value}} + // expected-note@-2{{'a' initialized here}} + // expected-warning@-3{{too many braces around scalar initializer}} + // expected-warning@-4{{too many braces around scalar initializer}} + // expected-warning@-5{{too many braces around scalar initializer}} + // expected-warning@-6{{too many braces around scalar initializer}} + // expected-warning@-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@-1{{Dereference of null pointer}} + // expected-note@-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@-1{{'y' initialized to a null pointer value}} + + S a{{x, y}}; + // expected-note@-1{{Initializing to a null pointer value}} + // expected-note@-2{{'a' initialized here}} + + S b = a; // expected-note{{Storing null pointer value}} + + int i = *b.n.arr[1]; + // expected-warning@-1{{Dereference of null pointer}} + // expected-note@-2{{Dereference of null pointer}} + (void) i; +} + +} // namespace NestedElementRegionTrack