alexshap created this revision. Herald added a subscriber: xazax.hun. This diff attempts to address a crash (triggered assert) on the newly-added test case. The assert being discussed is inside SValBuilder::evalBinOp
if (Optional<Loc> RV = rhs.getAs<Loc>()) { // Support pointer arithmetic where the addend is on the left // and the pointer on the right. assert(op == BO_Add); but the root cause seems to be in a different place (if I'm not missing smth). The method simplifySVal appears to be doing the wrong thing for SVal "(char*)E - (char*)B". Call stack: evalIntegralCast -> evalBinOpNN -> simplifySVal -> ... -> simplifySVal -> Simplifier::VisitSymSymExpr -> ... Inside Simplifier::VisitSymSymExpr the call Visit(S->getLHS()) returns a NonLoc (where S->getLHS() represents char* E) while the call Visit(S->getRHS()) returns a Loc. For Visit(S->getRHS()) this happens because in VisitSymbolData(const SymbolData *S) the "if" condition if (const llvm::APSInt *I = SVB.getKnownValue(State, nonloc::SymbolVal(S))) is satisfied and Loc is correctly chosen. For Visit(S->getLHS()) nonloc::SymbolVal(S) is used instead. Next those values will passed to SValBuilder::evalBinOp and the code path if (Optional<Loc> RV = rhs.getAs<Loc>()) { // Support pointer arithmetic where the addend is on the left // and the pointer on the right. assert(op == BO_Add); // Commute the operands. return evalBinOpLN(state, op, *RV, lhs.castAs<NonLoc>(), type); } is executed instead of if (Optional<Loc> LV = lhs.getAs<Loc>()) { if (Optional<Loc> RV = rhs.getAs<Loc>()) return evalBinOpLL(state, op, *LV, *RV, type); return evalBinOpLN(state, op, *LV, rhs.castAs<NonLoc>(), type); } Test plan: make check-all (green) Repository: rL LLVM https://reviews.llvm.org/D36564 Files: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp test/Analysis/ptr-arith.cpp Index: test/Analysis/ptr-arith.cpp =================================================================== --- test/Analysis/ptr-arith.cpp +++ test/Analysis/ptr-arith.cpp @@ -98,3 +98,10 @@ int a[5][5]; *(*(a+1)+2) = 2; } + +unsigned ptrSubtractionNoCrash(char *Begin, char *End) { + auto N = End - Begin; + if (Begin) + return 0; + return N; +} Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp =================================================================== --- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp @@ -1016,7 +1016,8 @@ SVB.getKnownValue(State, nonloc::SymbolVal(S))) return Loc::isLocType(S->getType()) ? (SVal)SVB.makeIntLocVal(*I) : (SVal)SVB.makeIntVal(*I); - return nonloc::SymbolVal(S); + return Loc::isLocType(S->getType()) ? (SVal)SVB.makeLoc(S) + : nonloc::SymbolVal(S); } // TODO: Support SymbolCast. Support IntSymExpr when/if we actually
Index: test/Analysis/ptr-arith.cpp =================================================================== --- test/Analysis/ptr-arith.cpp +++ test/Analysis/ptr-arith.cpp @@ -98,3 +98,10 @@ int a[5][5]; *(*(a+1)+2) = 2; } + +unsigned ptrSubtractionNoCrash(char *Begin, char *End) { + auto N = End - Begin; + if (Begin) + return 0; + return N; +} Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp =================================================================== --- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp @@ -1016,7 +1016,8 @@ SVB.getKnownValue(State, nonloc::SymbolVal(S))) return Loc::isLocType(S->getType()) ? (SVal)SVB.makeIntLocVal(*I) : (SVal)SVB.makeIntVal(*I); - return nonloc::SymbolVal(S); + return Loc::isLocType(S->getType()) ? (SVal)SVB.makeLoc(S) + : nonloc::SymbolVal(S); } // TODO: Support SymbolCast. Support IntSymExpr when/if we actually
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits