[PATCH] D38214: [analyzer] Fix crash on modeling of pointer arithmetic
This revision was automatically updated to reflect the committed changes. Closed by commit rL314141: [analyzer] Fix crash on modeling of pointer arithmetic (authored by alexshap). Changed prior to commit: https://reviews.llvm.org/D38214?vs=116455&id=116590#toc Repository: rL LLVM https://reviews.llvm.org/D38214 Files: cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp cfe/trunk/test/Analysis/ptr-arith.cpp Index: cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp === --- cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp @@ -726,9 +726,11 @@ if (Optional rInt = rhs.getAs()) { // If one of the operands is a symbol and the other is a constant, // build an expression for use by the constraint manager. - if (SymbolRef lSym = lhs.getAsLocSymbol(true)) -return MakeSymIntVal(lSym, op, rInt->getValue(), resultTy); - + if (SymbolRef lSym = lhs.getAsLocSymbol(true)) { +if (BinaryOperator::isComparisonOp(op)) + return MakeSymIntVal(lSym, op, rInt->getValue(), resultTy); +return UnknownVal(); + } // Special case comparisons to NULL. // This must come after the test if the LHS is a symbol, which is used to // build constraints. The address of any non-symbolic region is guaranteed Index: cfe/trunk/test/Analysis/ptr-arith.cpp === --- cfe/trunk/test/Analysis/ptr-arith.cpp +++ cfe/trunk/test/Analysis/ptr-arith.cpp @@ -111,3 +111,9 @@ __UINTPTR_TYPE__ y = (__UINTPTR_TYPE__)p - 1; return y == x; } + +// Bug 34374 +bool integerAsPtrSubtractionNoCrash(char *p, __UINTPTR_TYPE__ m) { + auto n = p - reinterpret_cast((__UINTPTR_TYPE__)1); + return n == m; +} Index: cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp === --- cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp @@ -726,9 +726,11 @@ if (Optional rInt = rhs.getAs()) { // If one of the operands is a symbol and the other is a constant, // build an expression for use by the constraint manager. - if (SymbolRef lSym = lhs.getAsLocSymbol(true)) -return MakeSymIntVal(lSym, op, rInt->getValue(), resultTy); - + if (SymbolRef lSym = lhs.getAsLocSymbol(true)) { +if (BinaryOperator::isComparisonOp(op)) + return MakeSymIntVal(lSym, op, rInt->getValue(), resultTy); +return UnknownVal(); + } // Special case comparisons to NULL. // This must come after the test if the LHS is a symbol, which is used to // build constraints. The address of any non-symbolic region is guaranteed Index: cfe/trunk/test/Analysis/ptr-arith.cpp === --- cfe/trunk/test/Analysis/ptr-arith.cpp +++ cfe/trunk/test/Analysis/ptr-arith.cpp @@ -111,3 +111,9 @@ __UINTPTR_TYPE__ y = (__UINTPTR_TYPE__)p - 1; return y == x; } + +// Bug 34374 +bool integerAsPtrSubtractionNoCrash(char *p, __UINTPTR_TYPE__ m) { + auto n = p - reinterpret_cast((__UINTPTR_TYPE__)1); + return n == m; +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38214: [analyzer] Fix crash on modeling of pointer arithmetic
alexfh added a comment. Thanks for the fix! Repository: rL LLVM https://reviews.llvm.org/D38214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38214: [analyzer] Fix crash on modeling of pointer arithmetic
NoQ added a comment. Looks good! I guess the accurate thing to do would be to return LocAsInteger of type `intptr_t` of an ElementRegion with index `-1` from SymbolicRegion around `p`. But i'm totally fine with another `UnknownVal` placeholder until this becomes an actual problem. Repository: rL LLVM https://reviews.llvm.org/D38214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38214: [analyzer] Fix crash on modeling of pointer arithmetic
alexshap created this revision. Herald added a subscriber: xazax.hun. This patch attempts to fix analyzer's crash on the newly added test case (see also https://bugs.llvm.org/show_bug.cgi?id=34374). Pointer subtraction appears to be modeled incorrectly in the following example: char* p; long n = p - reinterpret_cast((unsigned long)1); In this case the analyzer (built without this patch) tries to create a symbolic value for the difference treating reinterpret_cast((unsigned long)1) as an integer, that is not correct. Test plan: make check-all Repository: rL LLVM https://reviews.llvm.org/D38214 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 @@ -111,3 +111,9 @@ __UINTPTR_TYPE__ y = (__UINTPTR_TYPE__)p - 1; return y == x; } + +// Bug 34374 +bool integerAsPtrSubtractionNoCrash(char *p, __UINTPTR_TYPE__ m) { + auto n = p - reinterpret_cast((__UINTPTR_TYPE__)1); + return n == m; +} Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp === --- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp @@ -726,9 +726,11 @@ if (Optional rInt = rhs.getAs()) { // If one of the operands is a symbol and the other is a constant, // build an expression for use by the constraint manager. - if (SymbolRef lSym = lhs.getAsLocSymbol(true)) -return MakeSymIntVal(lSym, op, rInt->getValue(), resultTy); - + if (SymbolRef lSym = lhs.getAsLocSymbol(true)) { +if (BinaryOperator::isComparisonOp(op)) + return MakeSymIntVal(lSym, op, rInt->getValue(), resultTy); +return UnknownVal(); + } // Special case comparisons to NULL. // This must come after the test if the LHS is a symbol, which is used to // build constraints. The address of any non-symbolic region is guaranteed Index: test/Analysis/ptr-arith.cpp === --- test/Analysis/ptr-arith.cpp +++ test/Analysis/ptr-arith.cpp @@ -111,3 +111,9 @@ __UINTPTR_TYPE__ y = (__UINTPTR_TYPE__)p - 1; return y == x; } + +// Bug 34374 +bool integerAsPtrSubtractionNoCrash(char *p, __UINTPTR_TYPE__ m) { + auto n = p - reinterpret_cast((__UINTPTR_TYPE__)1); + return n == m; +} Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp === --- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp @@ -726,9 +726,11 @@ if (Optional rInt = rhs.getAs()) { // If one of the operands is a symbol and the other is a constant, // build an expression for use by the constraint manager. - if (SymbolRef lSym = lhs.getAsLocSymbol(true)) -return MakeSymIntVal(lSym, op, rInt->getValue(), resultTy); - + if (SymbolRef lSym = lhs.getAsLocSymbol(true)) { +if (BinaryOperator::isComparisonOp(op)) + return MakeSymIntVal(lSym, op, rInt->getValue(), resultTy); +return UnknownVal(); + } // Special case comparisons to NULL. // This must come after the test if the LHS is a symbol, which is used to // build constraints. The address of any non-symbolic region is guaranteed ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits