Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -1410,6 +1410,16 @@ } } +static bool isTrivialCopyOrMoveCtor(const CXXConstructExpr *CE) { + if (!CE) + return false; + + const auto *CtorDecl = CE->getConstructor(); + + return (CtorDecl->isCopyConstructor() || CtorDecl->isMoveConstructor()) && + CtorDecl->isTrivial(); +} + PathDiagnosticPieceRef StoreSiteFinder::VisitNode(const ExplodedNode *Succ, BugReporterContext &BRC, PathSensitiveBugReport &BR) { @@ -1439,6 +1449,84 @@ } } + // if (Optional P = Succ->getLocationAs()) { + // // In this case we handle if the initializer list is a part of a + // // constructor. + // // + // // E.g.: 'Ctor : field{ x, y } {}'. + // if(isa(P->getStmt()) && isa(R)) { + // // Try to match the ILE with the region. + // // For the region we can only travel outwards and for + // // ILEs we can only travel inwards. + // // + // // E.g.: + // // The region 's.field.field2.y' can be travelled as + // // 's.field.field2.y' -> 's.field.field2' -> 's.field' + // // + // // The ILE would look like 'ILE.ILE.Decl', this can be + // // travelled as 'ILE.ILE.Decl' -> 'ILE.ILE' -> 'ILE' + // // + // // The idea here is to push each sub-region to a stack, + // // and after each region has been pushed, start popping them, + // // and move towards the sub-expression whose index corresponds + // // to the index of the region. + // const auto *ILE = cast(P->getStmt()); + // const auto *FR = cast(R); + // const auto State = Succ->getState(); + + // // This method tries to match the region agains the ILE but might fail, + // so we need to double check. auto matchRegion = [](const InitListExpr + // *I, const FieldRegion *F) -> const Expr * { + // auto impl = [](const InitListExpr *I, const FieldRegion *F, auto + // &matcher_ref) mutable -> const Expr *{ + // if(!isa(F->getSuperRegion())) + // return I; + + // const auto *FD = F->getDecl(); + + // I = dyn_cast_or_null(matcher_ref(I, + // cast(F->getSuperRegion()), matcher_ref)); + + // if(!I || FD->getFieldIndex() >= I->getNumInits()) + // return nullptr; + + // return I->getInit(FD->getFieldIndex()); + // }; + + // const auto *E = impl(I, F, impl); + + // if(E && E->getType() == F->getDecl()->getType()) + // return E; + + // return nullptr; + // }; + + // // If the layout of the region matches the layout of + // // the initializer list, we move one region up, and + // // track launch a new tracker. + // if(const auto *E = matchRegion(ILE, FR)) + // { + // const MemRegion *ParentReg = FR; + // if(const auto *SFR = dyn_cast(FR->getSuperRegion())) + // ParentReg = SFR->getSuperRegion(); + + // const auto ParentVal = State->getLValue(FR->getDecl(), + // loc::MemRegionVal(ParentReg)); + // getParentTracker().track(V,ParentVal.getAsRegion(),Options); + + // // This is only used for generating more accurate notes. + // if(State->getSVal(R) != V) + // R = FR->getSuperRegion(); + + // InitE = E; + // StoreSite = Succ; + // } + // else + // return nullptr; + + // } + // } + // Otherwise, see if this is the store site: // (1) Succ has this binding and Pred does not, i.e. this is // where the binding first occurred. @@ -1456,12 +1544,76 @@ 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(R) && isa(Decl)) { + const auto *VD = cast(Decl); + const auto *FR = cast(R); + const auto *FD = FR->getDecl(); + + // If the initializer is an initializer list, we know that + // the field index matches the index of the argument in the + // initializer list. + if (const auto *ILE = dyn_cast(VD->getInit())) + InitE = ILE->getInit(FD->getFieldIndex()); + } + } else if (const auto *CE = P->getStmtAs()) { + + const auto State = Succ->getState(); + + if (isTrivialCopyOrMoveCtor(CE) && isa(R)) { + const auto *FR = cast(R); + const auto *FD = FR->getDecl(); + + // Move the current field region to the object the we copied/moved + // from. If we track 'a.y' and encounter 'S a = b', then get the + // region 'b.y' and track it. + + const auto *OriginEx = CE->getArg(0); + const auto OriginVal = + State->getSVal(OriginEx, Succ->getLocationContext()); + const auto OriginField = State->getLValue(FD, OriginVal); + + getParentTracker().track(V, OriginField.getAsRegion(), Options); + InitE = OriginEx; + } + } + // In this case we handle if the initializer list is a part of a + // constructor. E.g.: 'Ctor : field{ x, y } {}'. + else if (const auto *ILE = P->getStmtAs()) { + const auto State = Succ->getState(); + + // If we dereference a nested object, like 's.s2.s3.y', + // we bubble the last region up one object, as it happend + // with trivial copy/move ctors. + if (isa(R)) { + const auto *FR = cast(R); + const auto *FD = FR->getDecl(); + + const MemRegion *ParentReg = FR; + const auto *ParentDecl = FD; + if (const auto *SFR = dyn_cast(FR->getSuperRegion())) { + ParentReg = SFR->getSuperRegion(); + ParentDecl = SFR->getDecl(); + } + + const auto FieldVal = + State->getLValue(FD, loc::MemRegionVal(ParentReg)); + getParentTracker().track(V, FieldVal.getAsRegion(), Options); + + InitE = ILE->getInit(ParentDecl->getFieldIndex()); + } + } + } // If this is a call entry, the variable should be a parameter. // FIXME: Handle CXXThisRegion as well. (This is not a priority because @@ -2418,12 +2570,12 @@ track(BO->getLHS()); if (RHSV.isZeroConstant()) track(BO->getRHS()); - } else { // Track only the LHS of a division or a modulo. - if (LHSV.isZeroConstant()) - track(BO->getLHS()); - } + } else { // Track only the LHS of a division or a modulo. + if (LHSV.isZeroConstant()) + track(BO->getLHS()); + } - return CombinedResult; + return CombinedResult; } }; Index: clang/test/Analysis/ctor-bug-path.cpp =================================================================== --- /dev/null +++ clang/test/Analysis/ctor-bug-path.cpp @@ -0,0 +1,137 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-output=text -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 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 nestedCtorInitializer { + struct S5{ + int *x, *y; + }; + + struct S4 { + S5 s5; + }; + + struct S3 { + S4 s4; + }; + + struct S2 { + S3 s3; + }; + + struct S { + S2 s2; + + S(int *x, int *y) : s2{x, y} {}; + xpected-note@-1{{Null pointer value stored to 's.s2'}} + xpected-note@-2{{Null pointer value stored to 's.s2.s3'}} + xpected-note@-3{{Null pointer value stored to 's.s2.s3.s4'}} + xpected-note@-4{{Null pointer value stored to 's.s2.s3.s4.s5.y'}} + }; + + void nestedCtorInit(){ + int *x = nullptr, *y = nullptr; xpected-note{{'y' initialized to a null pointer value}} + + S s{x,y}; + xpected-note@-1{{Passing null pointer value via 2nd parameter}} + xpected-note@-2{{Calling constructor for 'S'}} + xpected-note@-3{{Returning from constructor for 'S'}} + + int i = *s.s2.s3.s4.s5.y; xpected-warning{{Dereference of null pointer}} + xpected-note@-1{{Dereference of null pointer}} + (void) i; + } +} // namespace nestedCtorInitializer +*/