[PATCH] D31289: [analyzer] Fix symbolication for unknown unary increment/decrement results.

2017-03-28 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL298927: [analyzer] Fix symbolication for unknown unary 
increment/decrement results. (authored by dergachev).

Changed prior to commit:
  https://reviews.llvm.org/D31289?vs=92812=93251#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D31289

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  cfe/trunk/test/Analysis/casts.c


Index: cfe/trunk/test/Analysis/casts.c
===
--- cfe/trunk/test/Analysis/casts.c
+++ cfe/trunk/test/Analysis/casts.c
@@ -118,3 +118,8 @@
   extern float globalFloat;
   clang_analyzer_eval(globalFloat); // expected-warning{{UNKNOWN}}
 }
+
+void locAsIntegerCasts(void *p) {
+  int x = (int) p;
+  clang_analyzer_eval(++x < 10); // no-crash // expected-warning{{UNKNOWN}}
+}
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -1054,7 +1054,7 @@
 // Conjure a new symbol if necessary to recover precision.
 if (Result.isUnknown()){
   DefinedOrUnknownSVal SymVal =
-svalBuilder.conjureSymbolVal(nullptr, Ex, LCtx,
+svalBuilder.conjureSymbolVal(nullptr, U, LCtx,
  currBldrCtx->blockCount());
   Result = SymVal;
 
Index: cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -362,6 +362,9 @@
  resultTy);
 case nonloc::ConcreteIntKind: {
   // Transform the integer into a location and compare.
+  // FIXME: This only makes sense for comparisons. If we want to, say,
+  // add 1 to a LocAsInteger, we'd better unpack the Loc and add to it,
+  // then pack it back into a LocAsInteger.
   llvm::APSInt i = rhs.castAs().getValue();
   BasicVals.getAPSIntType(Context.VoidPtrTy).apply(i);
   return evalBinOpLL(state, op, lhsL, makeLoc(i), resultTy);
@@ -942,6 +945,8 @@
 rhs = convertToArrayIndex(rhs).castAs();
 SVal index = UnknownVal();
 const MemRegion *superR = nullptr;
+// We need to know the type of the pointer in order to add an integer to 
it.
+// Depending on the type, different amount of bytes is added.
 QualType elementType;
 
 if (const ElementRegion *elemReg = dyn_cast(region)) {
@@ -955,6 +960,10 @@
   assert(op == BO_Add || op == BO_Sub);
   index = (op == BO_Add) ? rhs : evalMinus(rhs);
   superR = region;
+  // TODO: Is this actually reliable? Maybe improving our MemRegion
+  // hierarchy to provide typed regions for all non-void pointers would be
+  // better. For instance, we cannot extend this towards LocAsInteger
+  // operations, where result type of the expression is integer.
   if (resultTy->isAnyPointerType())
 elementType = resultTy->getPointeeType();
 }


Index: cfe/trunk/test/Analysis/casts.c
===
--- cfe/trunk/test/Analysis/casts.c
+++ cfe/trunk/test/Analysis/casts.c
@@ -118,3 +118,8 @@
   extern float globalFloat;
   clang_analyzer_eval(globalFloat); // expected-warning{{UNKNOWN}}
 }
+
+void locAsIntegerCasts(void *p) {
+  int x = (int) p;
+  clang_analyzer_eval(++x < 10); // no-crash // expected-warning{{UNKNOWN}}
+}
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -1054,7 +1054,7 @@
 // Conjure a new symbol if necessary to recover precision.
 if (Result.isUnknown()){
   DefinedOrUnknownSVal SymVal =
-svalBuilder.conjureSymbolVal(nullptr, Ex, LCtx,
+svalBuilder.conjureSymbolVal(nullptr, U, LCtx,
  currBldrCtx->blockCount());
   Result = SymVal;
 
Index: cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -362,6 +362,9 @@
  resultTy);
 case nonloc::ConcreteIntKind: {
   // Transform the integer into a location and compare.
+  // FIXME: This only makes sense for comparisons. If we want to, say,
+  // add 1 to a LocAsInteger, we'd better unpack the Loc and add to it,
+  // then pack it back into a LocAsInteger.
   llvm::APSInt i = rhs.castAs().getValue();
   

[PATCH] D31289: [analyzer] Fix symbolication for unknown unary increment/decrement results.

2017-03-23 Thread Devin Coughlin via Phabricator via cfe-commits
dcoughlin accepted this revision.
dcoughlin added a comment.
This revision is now accepted and ready to land.

Looks good to me.


https://reviews.llvm.org/D31289



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D31289: [analyzer] Fix symbolication for unknown unary increment/decrement results.

2017-03-23 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision.

If result of an unary increment or decrement is unknown, conjure a symbol to 
represent it based on the operator expression, not on the sub-expression.

In this particular test case, result of a LocAsInteger increment is unknown, 
and it gets symbolicated to an `int *`-type symbol, because sub-expression is 
an lvalue. This causes a crash later when we're trying to compare a Loc and a 
NonLoc.


https://reviews.llvm.org/D31289

Files:
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  test/Analysis/casts.c


Index: test/Analysis/casts.c
===
--- test/Analysis/casts.c
+++ test/Analysis/casts.c
@@ -118,3 +118,8 @@
   extern float globalFloat;
   clang_analyzer_eval(globalFloat); // expected-warning{{UNKNOWN}}
 }
+
+void locAsIntegerCasts(void *p) {
+  int x = (int) p;
+  clang_analyzer_eval(++x < 10); // no-crash // expected-warning{{UNKNOWN}}
+}
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -1054,7 +1054,7 @@
 // Conjure a new symbol if necessary to recover precision.
 if (Result.isUnknown()){
   DefinedOrUnknownSVal SymVal =
-svalBuilder.conjureSymbolVal(nullptr, Ex, LCtx,
+svalBuilder.conjureSymbolVal(nullptr, U, LCtx,
  currBldrCtx->blockCount());
   Result = SymVal;
 


Index: test/Analysis/casts.c
===
--- test/Analysis/casts.c
+++ test/Analysis/casts.c
@@ -118,3 +118,8 @@
   extern float globalFloat;
   clang_analyzer_eval(globalFloat); // expected-warning{{UNKNOWN}}
 }
+
+void locAsIntegerCasts(void *p) {
+  int x = (int) p;
+  clang_analyzer_eval(++x < 10); // no-crash // expected-warning{{UNKNOWN}}
+}
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -1054,7 +1054,7 @@
 // Conjure a new symbol if necessary to recover precision.
 if (Result.isUnknown()){
   DefinedOrUnknownSVal SymVal =
-svalBuilder.conjureSymbolVal(nullptr, Ex, LCtx,
+svalBuilder.conjureSymbolVal(nullptr, U, LCtx,
  currBldrCtx->blockCount());
   Result = SymVal;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits