[PATCH] Add a truncation warning for values under a bitwise or
Sam Panzer
espanz at gmail.com
Thu Feb 21 12:47:04 PST 2013
Suppressed duplicate warning on right-hand side of <<= and >>= (narrowing in addition to the shift being out of range)
http://llvm-reviews.chandlerc.com/D405
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D405?vs=1069&id=1070#toc
Files:
lib/Sema/SemaChecking.cpp
test/Sema/constant-conversion.c
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -4778,6 +4778,53 @@
}
}
+// Determine if E is an expression prone to implicit narrowing bugs, containing
+// a constant. If so, Value is set to the value of that constant.
+static bool OperandMightImplicitlyNarrow(Sema &S, llvm::APSInt &Value,
+ Expr *E) {
+ BinaryOperator *Bop = dyn_cast<BinaryOperator>(E);
+ if (!Bop)
+ return false;
+
+ switch (Bop->getOpcode()) {
+ case BO_Or:
+ case BO_OrAssign:
+ case BO_Add:
+ case BO_AddAssign:
+ case BO_Sub:
+ case BO_SubAssign:
+ case BO_Mul:
+ case BO_MulAssign:
+ case BO_Xor:
+ case BO_XorAssign:
+ return Bop->getLHS()->EvaluateAsInt(Value, S.Context,
+ Expr::SE_AllowSideEffects) ||
+ Bop->getRHS()->EvaluateAsInt(Value, S.Context,
+ Expr::SE_AllowSideEffects);
+
+ // We can ignore the left side of the comma operator, since the value is
+ // explicitly ignored anyway.
+ case BO_Comma:
+ return Bop->getRHS()->EvaluateAsInt(Value, S.Context,
+ Expr::SE_AllowSideEffects);
+ case BO_Shl:
+ return Bop->getLHS()->EvaluateAsInt(Value, S.Context,
+ Expr::SE_AllowSideEffects);
+ default:
+ break;
+ }
+ return false;
+}
+
+// Returns true when Value's value would change when narrowed to TargetRange.
+static bool TruncationChangesValue(const llvm::APSInt &Value,
+ const IntRange &TargetRange) {
+ bool isSigned = !TargetRange.NonNegative;
+ llvm::APSInt UnsignedValue(Value);
+ return (!isSigned && UnsignedValue.getActiveBits() > TargetRange.Width) ||
+ (isSigned && Value.getMinSignedBits() > TargetRange.Width);
+}
+
void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
SourceLocation CC, bool *ICContext = 0) {
if (E->isTypeDependent() || E->isValueDependent()) return;
@@ -4962,9 +5009,17 @@
if (SourceRange.Width > TargetRange.Width) {
// If the source is a constant, use a default-on diagnostic.
+ // Also use a default-on diagnostic if the source is involved in a
+ // narrowing-prone operation with a constant.
// TODO: this should happen for bitfield stores, too.
llvm::APSInt Value(32);
- if (E->isIntegerConstantExpr(Value, S.Context)) {
+ if (E->isIntegerConstantExpr(Value, S.Context) ||
+ OperandMightImplicitlyNarrow(S, Value, E)) {
+ // In case the number involved happens to be a wide type but have a
+ // small value nonetheless, don't issue a warning.
+ if (!TruncationChangesValue(Value, TargetRange))
+ return;
+
if (S.SourceMgr.isInSystemMacro(CC))
return;
@@ -5114,6 +5169,25 @@
if (E->getType() != T)
CheckImplicitConversion(S, E, T, CC);
+ // For CompoundAssignmentOperators, check the conversion from the computed
+ // LHS type to the type of the assignee.
+ if (CompoundAssignOperator *CAO = dyn_cast<CompoundAssignOperator>(E)) {
+ QualType LHST = CAO->getComputationLHSType();
+ // This CheckImplicitConversion would trigger a warning in addition to the
+ // more serious problem of using NULLs in arithmetic.
+ // Let the call to CheckImplicitConversion on the child expressions at the
+ // bottom of this function handle them by filtering those out here.
+ // Similarly, disable the extra check for shift assignments, since any
+ // narrowing would be less serious than a too-large shift count.
+ if (LHST != T &&
+ T->isIntegralType(S.Context) && LHST->isIntegralType(S.Context) &&
+ !CAO->isShiftAssignOp() &&
+ CAO->getRHS()->isNullPointerConstant(S.Context,
+ Expr::NPC_ValueDependentIsNotNull)
+ != Expr::NPCK_GNUNull)
+ CheckImplicitConversion(S, CAO->getRHS(), T, CC);
+ }
+
// Now continue drilling into this expression.
// Skip past explicit casts.
@@ -5127,8 +5201,9 @@
if (BO->isComparisonOp())
return AnalyzeComparison(S, BO);
- // And with simple assignments.
- if (BO->getOpcode() == BO_Assign)
+ // And with simple assignments. We also do a special check for bitwise or
+ // assignments to catch overflow in bitfields.
+ if (BO->getOpcode() == BO_Assign || BO->getOpcode() == BO_OrAssign)
return AnalyzeAssignment(S, BO);
}
Index: test/Sema/constant-conversion.c
===================================================================
--- test/Sema/constant-conversion.c
+++ test/Sema/constant-conversion.c
@@ -66,17 +66,49 @@
struct {
unsigned int twoBits1:2;
unsigned int twoBits2:2;
- unsigned int reserved:28;
+ unsigned int twoBits3:2;
+ unsigned int reserved:26;
} f;
f.twoBits1 = ~1; // expected-warning {{implicit truncation from 'int' to bitfield changes value from -2 to 2}}
f.twoBits2 = ~2; // expected-warning {{implicit truncation from 'int' to bitfield changes value from -3 to 1}}
f.twoBits1 &= ~1; // no-warning
f.twoBits2 &= ~2; // no-warning
+ f.twoBits3 |= 4; // expected-warning {{implicit truncation from 'int' to bitfield changes value from 4 to 0}}
+ f.twoBits3 |= 1; // no-warning
}
void test8() {
enum E { A, B, C };
struct { enum E x : 1; } f;
f.x = C; // expected-warning {{implicit truncation from 'int' to bitfield changes value from 2 to 0}}
}
+
+int func(int);
+
+void test9() {
+ unsigned char x = 0;
+ unsigned char y = 0;
+ x = y | 0x1ff; // expected-warning {{implicit conversion from 'int' to 'unsigned char' changes value from 511 to 255}}
+ x = y | 0xff; // no-warning
+ x = 0x1ff | y; // expected-warning {{implicit conversion from 'int' to 'unsigned char' changes value from 511 to 255}}
+ x = 0xff | y; // no-warning
+ x = (y | 0x1ff); // expected-warning {{implicit conversion from 'int' to 'unsigned char' changes value from 511 to 255}}
+ x = (y | 0xff); // no-warning
+ x = 0x1ff + y; // expected-warning {{implicit conversion from 'int' to 'unsigned char' changes value from 511 to 255}}
+ x = 0xff + y; // no-warning
+ x += 0x1ff; // expected-warning {{implicit conversion from 'int' to 'unsigned char' changes value from 511 to 255}}
+ x = 0x1ff - y; // expected-warning {{implicit conversion from 'int' to 'unsigned char' changes value from 511 to 255}}
+ x = 0xff - y; // no-warning
+ x -= 0x1ff; // expected-warning {{implicit conversion from 'int' to 'unsigned char' changes value from 511 to 255}}
+ x = y * 0x1ff; // expected-warning {{implicit conversion from 'int' to 'unsigned char' changes value from 511 to 255}}
+ x = y * 0xff; // no-warning
+ x *= 0x1ff; // expected-warning {{implicit conversion from 'int' to 'unsigned char' changes value from 511 to 255}}
+ x = y ^ 0x1ff; // expected-warning {{implicit conversion from 'int' to 'unsigned char' changes value from 511 to 255}}
+ x = y ^ 0xff; // no-warning
+ x ^= 0x1ff; // expected-warning {{implicit conversion from 'int' to 'unsigned char' changes value from 511 to 255}}
+ x = (func(1), 0x1ff); // expected-warning {{implicit conversion from 'int' to 'unsigned char' changes value from 511 to 255}}
+ x = (func(1), 0xff); // no-warning
+ x = 0xff << y; // no-warning
+ x = 0x1ff << y; // expected-warning {{implicit conversion from 'int' to 'unsigned char' changes value from 511 to 255}}
+}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D405.6.patch
Type: text/x-patch
Size: 7548 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130221/57ec4aa3/attachment.bin>
More information about the cfe-commits
mailing list