rnkovacs updated this revision to Diff 129448.
rnkovacs added a comment.
I extended the warning message to include more information. What do you think?
https://reviews.llvm.org/D41816
Files:
lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
lib/StaticAnalyzer/Core/BasicValueFactory.cpp
test/Analysis/bitwise-ops.c
Index: test/Analysis/bitwise-ops.c
===================================================================
--- test/Analysis/bitwise-ops.c
+++ test/Analysis/bitwise-ops.c
@@ -51,3 +51,9 @@
}
return 0;
}
+
+int testUnrepresentableLeftShift(int a) {
+ if (a == 8)
+ return a << 30; // expected-warning{{The result of the left shift is
undefined due to shifting '8' by '30', which is unrepresentable in the unsigned
version of the return type 'int'}}
+ return 0;
+}
Index: lib/StaticAnalyzer/Core/BasicValueFactory.cpp
===================================================================
--- lib/StaticAnalyzer/Core/BasicValueFactory.cpp
+++ lib/StaticAnalyzer/Core/BasicValueFactory.cpp
@@ -224,7 +224,6 @@
// FIXME: This logic should probably go higher up, where we can
// test these conditions symbolically.
- // FIXME: Expand these checks to include all undefined behavior.
if (V1.isSigned() && V1.isNegative())
return nullptr;
@@ -236,16 +235,17 @@
if (Amt >= V1.getBitWidth())
return nullptr;
+ if (V1.isSigned() && Amt > V1.countLeadingZeros())
+ return nullptr;
+
return &getValue( V1.operator<<( (unsigned) Amt ));
}
case BO_Shr: {
// FIXME: This logic should probably go higher up, where we can
// test these conditions symbolically.
- // FIXME: Expand these checks to include all undefined behavior.
-
if (V2.isSigned() && V2.isNegative())
return nullptr;
Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
@@ -141,6 +141,19 @@
C.isNegative(B->getLHS())) {
OS << "The result of the left shift is undefined because the left "
"operand is negative";
+ } else if (B->getOpcode() == BinaryOperatorKind::BO_Shl) {
+ SValBuilder &SB = C.getSValBuilder();
+ const llvm::APSInt *LHS =
+ SB.getKnownValue(state, C.getSVal(B->getLHS()));
+ const llvm::APSInt *RHS =
+ SB.getKnownValue(state, C.getSVal(B->getRHS()));
+ if ((unsigned) RHS->getZExtValue() > LHS->countLeadingZeros()) {
+ OS << "The result of the left shift is undefined due to shifting \'"
+ << LHS->getSExtValue() << "\' by \'" << RHS->getZExtValue()
+ << "\', which is unrepresentable in the unsigned version of "
+ << "the return type \'" << B->getLHS()->getType().getAsString()
+ << "\'";
+ }
} else {
OS << "The result of the '"
<< BinaryOperator::getOpcodeStr(B->getOpcode())
Index: test/Analysis/bitwise-ops.c
===================================================================
--- test/Analysis/bitwise-ops.c
+++ test/Analysis/bitwise-ops.c
@@ -51,3 +51,9 @@
}
return 0;
}
+
+int testUnrepresentableLeftShift(int a) {
+ if (a == 8)
+ return a << 30; // expected-warning{{The result of the left shift is undefined due to shifting '8' by '30', which is unrepresentable in the unsigned version of the return type 'int'}}
+ return 0;
+}
Index: lib/StaticAnalyzer/Core/BasicValueFactory.cpp
===================================================================
--- lib/StaticAnalyzer/Core/BasicValueFactory.cpp
+++ lib/StaticAnalyzer/Core/BasicValueFactory.cpp
@@ -224,7 +224,6 @@
// FIXME: This logic should probably go higher up, where we can
// test these conditions symbolically.
- // FIXME: Expand these checks to include all undefined behavior.
if (V1.isSigned() && V1.isNegative())
return nullptr;
@@ -236,16 +235,17 @@
if (Amt >= V1.getBitWidth())
return nullptr;
+ if (V1.isSigned() && Amt > V1.countLeadingZeros())
+ return nullptr;
+
return &getValue( V1.operator<<( (unsigned) Amt ));
}
case BO_Shr: {
// FIXME: This logic should probably go higher up, where we can
// test these conditions symbolically.
- // FIXME: Expand these checks to include all undefined behavior.
-
if (V2.isSigned() && V2.isNegative())
return nullptr;
Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
@@ -141,6 +141,19 @@
C.isNegative(B->getLHS())) {
OS << "The result of the left shift is undefined because the left "
"operand is negative";
+ } else if (B->getOpcode() == BinaryOperatorKind::BO_Shl) {
+ SValBuilder &SB = C.getSValBuilder();
+ const llvm::APSInt *LHS =
+ SB.getKnownValue(state, C.getSVal(B->getLHS()));
+ const llvm::APSInt *RHS =
+ SB.getKnownValue(state, C.getSVal(B->getRHS()));
+ if ((unsigned) RHS->getZExtValue() > LHS->countLeadingZeros()) {
+ OS << "The result of the left shift is undefined due to shifting \'"
+ << LHS->getSExtValue() << "\' by \'" << RHS->getZExtValue()
+ << "\', which is unrepresentable in the unsigned version of "
+ << "the return type \'" << B->getLHS()->getType().getAsString()
+ << "\'";
+ }
} else {
OS << "The result of the '"
<< BinaryOperator::getOpcodeStr(B->getOpcode())
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
