pgousseau updated the summary for this revision.
pgousseau updated this revision to Diff 43120.
pgousseau added a comment.
Following Gabor and Anna's advice:
- Instead of modifying assumeSymNE and assumeSymEQ, this patch adds a new
method 'SValBuilder::evalIntegralCast'.
The current workaround for truncations not being modelled is that the
evaluation of integer to integer casts are simply bypassed and so the original
symbol is used as the new casted symbol (cf
SimpleSValBuilder::evalCastFromNonLoc).
This lead to the issue described above, as the RangeConstraintManager
associates ranges with symbols.
The new evalIntegralCast method added by this patch wont bypass the cast if it
finds the range of the symbol to be greater than the maximum value of the
target type.
This patch also fixes a bug in 'RangeSet::pin' causing single value ranges to
not be considered conventionally ordered.
The patch has been tested with openssl-1.0.0d-src and bullet-2.82-r2704 with no
regressions observed.
Please let me know if this an acceptable change?
Regards,
Pierre Gousseau
SN Systems - Sony Computer Entertainment
http://reviews.llvm.org/D12901
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
lib/StaticAnalyzer/Core/ExprEngineC.cpp
lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
lib/StaticAnalyzer/Core/SValBuilder.cpp
test/Analysis/range_casts.c
Index: test/Analysis/range_casts.c
===================================================================
--- /dev/null
+++ test/Analysis/range_casts.c
@@ -0,0 +1,156 @@
+// This test checks that intersecting ranges does not cause 'system is over constrained' assertions in the case of eg: 32 bits unsigned integers getting their range from 64 bits signed integers.
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-store=region -verify %s
+
+void clang_analyzer_warnIfReached();
+
+void f1(long foo)
+{
+ unsigned index = -1;
+ if (index < foo) index = foo;
+ if (index + 1 == 0) // because of foo range, index is in range [0; UINT_MAX]
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+ else
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f2(unsigned long foo)
+{
+ int index = -1;
+ if (index < foo) index = foo; // index equals ULONG_MAX
+ if (index + 1 == 0)
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+ else
+ clang_analyzer_warnIfReached(); // no-warning
+}
+
+void f3(unsigned long foo)
+{
+ unsigned index = -1;
+ if (index < foo) index = foo;
+ if (index + 1 == 0)
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+ else
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f4(long foo)
+{
+ int index = -1;
+ if (index < foo) index = foo;
+ if (index + 1 == 0)
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+ else
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f5(long foo)
+{
+ unsigned index = -1;
+ if (index < foo) index = foo;
+ if (index == -1)
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+ else
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f6(long foo)
+{
+ unsigned index = -1;
+ if (index < foo) index = foo;
+ if (index == -1)
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+ else
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f7(long foo)
+{
+ unsigned index = -1;
+ if (index < foo) index = foo;
+ if (index - 1 == 0) // Was not reached prior fix.
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+ else
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f8(long foo)
+{
+ unsigned index = -1;
+ if (index < foo) index = foo;
+ if (index + 1L == 0L)
+ clang_analyzer_warnIfReached(); // no-warning
+ else
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f9(long foo)
+{
+ unsigned index = -1;
+ if (index < foo) index = foo;
+ if (index - 1L == 0L) // Was not reached prior fix.
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+ else
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f10(long foo)
+{
+ unsigned index = -1;
+ if (index < foo) index = foo;
+ if (index + 1 == 0L)
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+ else
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f11(long foo)
+{
+ unsigned index = -1;
+ if (index < foo) index = foo;
+ if (index + 1UL == 0L)
+ clang_analyzer_warnIfReached(); // no-warning
+ else
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f12(long foo)
+{
+ unsigned index = -1;
+ if (index < foo) index = foo;
+ if (index - 1UL == 0L) // Was not reached prior fix.
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+ else
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f13(int foo)
+{
+ unsigned short index = -1;
+ if (index < foo) index = foo;
+ if (index + 1 == 0)
+ clang_analyzer_warnIfReached(); // no-warning
+ else
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f14(long foo)
+{
+ unsigned index = -1;
+ if (index < foo) index = foo;
+ long bar = foo;
+ if (index + 1 == 0)
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+ else
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f15(long foo)
+{
+ unsigned index = -1;
+ if (index < foo) index = foo;
+ unsigned int tmp = index + 1;
+ if (tmp == 0)
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+ else
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
Index: lib/StaticAnalyzer/Core/SValBuilder.cpp
===================================================================
--- lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -423,6 +423,45 @@
return true;
}
+// Handles casts of type CK_IntegralCast.
+// At the moment, this function will redirect to evalCast, except when the range
+// of the original value is known to be greater than the max of the target type.
+SVal SValBuilder::evalIntegralCast(ProgramStateRef state, SVal val,
+ QualType castTy, QualType originalTy) {
+
+ // No truncations if target type is big enough.
+ if (getContext().getTypeSize(castTy) >= getContext().getTypeSize(originalTy))
+ return evalCast(val, castTy, originalTy);
+
+ const SymExpr *se = val.getAsSymbolicExpression();
+ if (!se) // Let evalCast handle non symbolic expressions.
+ return evalCast(val, castTy, originalTy);
+
+ // Find the maximum value of the target type.
+ APSIntType ToType(getContext().getTypeSize(castTy),
+ castTy->isUnsignedIntegerType());
+ llvm::APSInt ToTypeMax = ToType.getMaxValue();
+ NonLoc ToTypeMaxVal =
+ makeIntVal(ToTypeMax.isUnsigned() ? ToTypeMax.getZExtValue()
+ : ToTypeMax.getSExtValue(),
+ castTy)
+ .castAs<NonLoc>();
+ // Check the range of the symbol being casted against the maximum value of the
+ // target type.
+ NonLoc FromVal = val.castAs<NonLoc>();
+ QualType CmpTy = getConditionType();
+ NonLoc CompVal =
+ evalBinOpNN(state, BO_LT, FromVal, ToTypeMaxVal, CmpTy).castAs<NonLoc>();
+ ProgramStateRef IsNotTruncated, IsTruncated;
+ std::tie(IsNotTruncated, IsTruncated) = state->assume(CompVal);
+ if (!IsNotTruncated && IsTruncated) {
+ // Symbol is truncated so we evaluate it as a cast.
+ NonLoc CastVal = makeNonLoc(se, originalTy, castTy);
+ return CastVal;
+ }
+ return evalCast(val, castTy, originalTy);
+}
+
// FIXME: should rewrite according to the cast kind.
SVal SValBuilder::evalCast(SVal val, QualType castTy, QualType originalTy) {
castTy = Context.getCanonicalType(castTy);
Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===================================================================
--- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -171,7 +171,7 @@
case APSIntType::RTR_Below:
// The entire range is outside the symbol's set of possible values.
// If this is a conventionally-ordered range, the state is infeasible.
- if (Lower < Upper)
+ if (Lower <= Upper)
return false;
// However, if the range wraps around, it spans all possible values.
@@ -222,7 +222,7 @@
case APSIntType::RTR_Above:
// The entire range is outside the symbol's set of possible values.
// If this is a conventionally-ordered range, the state is infeasible.
- if (Lower < Upper)
+ if (Lower <= Upper)
return false;
// However, if the range wraps around, it spans all possible values.
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -316,7 +316,6 @@
case CK_ArrayToPointerDecay:
case CK_BitCast:
case CK_AddressSpaceConversion:
- case CK_IntegralCast:
case CK_NullToPointer:
case CK_IntegralToPointer:
case CK_PointerToIntegral:
@@ -349,6 +348,14 @@
Bldr.generateNode(CastE, Pred, state);
continue;
}
+ case CK_IntegralCast: {
+ // Delegate to SValBuilder to process.
+ SVal V = state->getSVal(Ex, LCtx);
+ V = svalBuilder.evalIntegralCast(state, V, T, ExTy);
+ state = state->BindExpr(CastE, LCtx, V);
+ Bldr.generateNode(CastE, Pred, state);
+ continue;
+ }
case CK_DerivedToBase:
case CK_UncheckedDerivedToBase: {
// For DerivedToBase cast, delegate to the store manager.
Index: include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -83,7 +83,11 @@
}
SVal evalCast(SVal val, QualType castTy, QualType originalType);
-
+
+ // Handles casts of type CK_IntegralCast.
+ SVal evalIntegralCast(ProgramStateRef state, SVal val, QualType castTy,
+ QualType originalType);
+
virtual SVal evalMinus(NonLoc val) = 0;
virtual SVal evalComplement(NonLoc val) = 0;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
