[PATCH] D38214: [analyzer] Fix crash on modeling of pointer arithmetic

[Alexander Shaposhnikov via Phabricator via cfe-commits]

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=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

[Alexander Kornienko via Phabricator via cfe-commits]

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

[Artem Dergachev via Phabricator via cfe-commits]

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

[Alexander Shaposhnikov via Phabricator via cfe-commits]

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