diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index df327e7e5097..d1e127b22bd6 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -2340,6 +2340,14 @@ def warn_division_by_zero : Warning<"division by zero is undefined">; def warn_remainder_by_zero : Warning<"remainder by zero is undefined">; def warn_shift_negative : Warning<"shift count is negative">; def warn_shift_gt_typewidth : Warning<"shift count >= width of type">; +def warn_shift_result_gt_typewidth : Warning< + "shift result (%0) requires %1 bits to represent, but the promoted type of " + "the shift expression is %2 with only %3 bits">, + InGroup>; +def warn_shift_result_overrides_sign_bit : Warning< + "shift result (%0) overrides the sign bit of the promoted type of the shift " + "expression (%1) and becomes negative">, + InGroup>, DefaultIgnore; def warn_precedence_bitwise_rel : Warning< "%0 has lower precedence than %1; %1 will be evaluated first">, diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 6b74b863a8c1..1cc20b60f79e 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -4790,7 +4790,8 @@ public: QualType CheckSubtractionOperands( // C99 6.5.6 Expr *&lex, Expr *&rex, SourceLocation OpLoc, QualType* CompLHSTy = 0); QualType CheckShiftOperands( // C99 6.5.7 - Expr *&lex, Expr *&rex, SourceLocation OpLoc, bool isCompAssign = false); + Expr *&lex, Expr *&rex, SourceLocation OpLoc, unsigned Opc, + bool isCompAssign = false); QualType CheckCompareOperands( // C99 6.5.8/9 Expr *&lex, Expr *&rex, SourceLocation OpLoc, unsigned Opc, bool isRelational); diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 1ba8ea62b416..076ffeb26878 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -6626,9 +6626,61 @@ static bool isScopedEnumerationType(QualType T) { return false; } +static void DiagnoseBadShiftValues(Sema& S, Expr *&lex, Expr *&rex, + SourceLocation Loc, unsigned Opc, + QualType LHSTy) { + llvm::APSInt Right; + // Check right/shifter operand + if (rex->isValueDependent() || !rex->isIntegerConstantExpr(Right, S.Context)) + return; + + if (Right.isNegative()) { + S.Diag(Loc, diag::warn_shift_negative) << rex->getSourceRange(); + return; + } + llvm::APInt LeftBits(Right.getBitWidth(), + S.Context.getTypeSize(lex->getType())); + if (Right.uge(LeftBits)) { + S.Diag(Loc, diag::warn_shift_gt_typewidth) << rex->getSourceRange(); + return; + } + if (Opc != BO_Shl) + return; + + // When left shifting an ICE which is signed, we can check for overflow which + // according to C++ has undefined behavior ([expr.shift] 5.8/2). Unsigned + // integers have defined behavior modulo one more than the maximum value + // representable in the result type, so never warn for those. + llvm::APSInt Left; + if (!lex->isIntegerConstantExpr(Left, S.Context) || + LHSTy->hasUnsignedIntegerRepresentation()) + return; + llvm::APInt ResultBits = + static_cast(Right) + Left.getMinSignedBits(); + if (LeftBits.uge(ResultBits)) + return; + llvm::APSInt Result = Left.extend(ResultBits.getLimitedValue()); + Result = Result.shl(Right); + + // If we are only missing a sign bit, this is less likely to result in actual + // bugs -- if the result is cast back to an unsigned type, it will have the + // expected value. Thus we place this behind a different warning that can be + // turned off separately if needed. + if (LeftBits == ResultBits - 1) { + S.Diag(Loc, diag::warn_shift_result_overrides_sign_bit) + << Result.toString(10) << LHSTy + << lex->getSourceRange() << rex->getSourceRange(); + return; + } + + S.Diag(Loc, diag::warn_shift_result_gt_typewidth) + << Result.toString(10) << Result.getMinSignedBits() << LHSTy + << Left.getBitWidth() << lex->getSourceRange() << rex->getSourceRange(); +} + // C99 6.5.7 QualType Sema::CheckShiftOperands(Expr *&lex, Expr *&rex, SourceLocation Loc, - bool isCompAssign) { + unsigned Opc, bool isCompAssign) { // C99 6.5.7p2: Each of the operands shall have integer type. if (!lex->getType()->hasIntegerRepresentation() || !rex->getType()->hasIntegerRepresentation()) @@ -6659,19 +6711,7 @@ QualType Sema::CheckShiftOperands(Expr *&lex, Expr *&rex, SourceLocation Loc, UsualUnaryConversions(rex); // Sanity-check shift operands - llvm::APSInt Right; - // Check right/shifter operand - if (!rex->isValueDependent() && - rex->isIntegerConstantExpr(Right, Context)) { - if (Right.isNegative()) - Diag(Loc, diag::warn_shift_negative) << rex->getSourceRange(); - else { - llvm::APInt LeftBits(Right.getBitWidth(), - Context.getTypeSize(lex->getType())); - if (Right.uge(LeftBits)) - Diag(Loc, diag::warn_shift_gt_typewidth) << rex->getSourceRange(); - } - } + DiagnoseBadShiftValues(*this, lex, rex, Loc, Opc, LHSTy); // "The type of the result is that of the promoted left operand." return LHSTy; @@ -7958,7 +7998,7 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc, break; case BO_Shl: case BO_Shr: - ResultTy = CheckShiftOperands(lhs, rhs, OpLoc); + ResultTy = CheckShiftOperands(lhs, rhs, OpLoc, Opc); break; case BO_LE: case BO_LT: @@ -8005,7 +8045,7 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc, break; case BO_ShlAssign: case BO_ShrAssign: - CompResultTy = CheckShiftOperands(lhs, rhs, OpLoc, true); + CompResultTy = CheckShiftOperands(lhs, rhs, OpLoc, Opc, true); CompLHSTy = CompResultTy; if (!CompResultTy.isNull()) ResultTy = CheckAssignmentOperands(lhs, rhs, OpLoc, CompResultTy); diff --git a/clang/test/Analysis/retain-release-basic-store.m b/clang/test/Analysis/retain-release-basic-store.m index 751dca0ce30b..8c05efef66ee 100644 --- a/clang/test/Analysis/retain-release-basic-store.m +++ b/clang/test/Analysis/retain-release-basic-store.m @@ -69,7 +69,7 @@ extern DADiskRef DADiskCopyWholeDisk( DADiskRef disk ); @interface NSAppleEventManager : NSObject { } @end enum { -kDAReturnSuccess = 0, kDAReturnError = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x01, kDAReturnBusy = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x02, kDAReturnBadArgument = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x03, kDAReturnExclusiveAccess = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x04, kDAReturnNoResources = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x05, kDAReturnNotFound = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x06, kDAReturnNotMounted = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x07, kDAReturnNotPermitted = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x08, kDAReturnNotPrivileged = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x09, kDAReturnNotReady = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x0A, kDAReturnNotWritable = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x0B, kDAReturnUnsupported = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x0C }; +kDAReturnSuccess = 0, kDAReturnError = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x01, kDAReturnBusy = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x02, kDAReturnBadArgument = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x03, kDAReturnExclusiveAccess = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x04, kDAReturnNoResources = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x05, kDAReturnNotFound = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x06, kDAReturnNotMounted = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x07, kDAReturnNotPermitted = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x08, kDAReturnNotPrivileged = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x09, kDAReturnNotReady = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x0A, kDAReturnNotWritable = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x0B, kDAReturnUnsupported = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x0C }; typedef mach_error_t DAReturn; typedef const struct __DADissenter * DADissenterRef; extern DADissenterRef DADissenterCreate( CFAllocatorRef allocator, DAReturn status, CFStringRef string ); diff --git a/clang/test/Analysis/retain-release-gc-only.m b/clang/test/Analysis/retain-release-gc-only.m index 6f1dd92df997..7d7c58fbcd51 100644 --- a/clang/test/Analysis/retain-release-gc-only.m +++ b/clang/test/Analysis/retain-release-gc-only.m @@ -189,7 +189,7 @@ CVTimeStamp; } typedef int CIFormat; @end enum { -kDAReturnSuccess = 0, kDAReturnError = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x01, kDAReturnBusy = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x02, kDAReturnBadArgument = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x03, kDAReturnExclusiveAccess = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x04, kDAReturnNoResources = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x05, kDAReturnNotFound = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x06, kDAReturnNotMounted = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x07, kDAReturnNotPermitted = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x08, kDAReturnNotPrivileged = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x09, kDAReturnNotReady = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x0A, kDAReturnNotWritable = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x0B, kDAReturnUnsupported = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x0C }; +kDAReturnSuccess = 0, kDAReturnError = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x01, kDAReturnBusy = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x02, kDAReturnBadArgument = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x03, kDAReturnExclusiveAccess = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x04, kDAReturnNoResources = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x05, kDAReturnNotFound = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x06, kDAReturnNotMounted = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x07, kDAReturnNotPermitted = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x08, kDAReturnNotPrivileged = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x09, kDAReturnNotReady = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x0A, kDAReturnNotWritable = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x0B, kDAReturnUnsupported = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x0C }; typedef mach_error_t DAReturn; typedef const struct __DADissenter * DADissenterRef; extern DADissenterRef DADissenterCreate( CFAllocatorRef allocator, DAReturn status, CFStringRef string ); diff --git a/clang/test/Analysis/retain-release-region-store.m b/clang/test/Analysis/retain-release-region-store.m index 05b91fcf5c56..ec765e3fe809 100644 --- a/clang/test/Analysis/retain-release-region-store.m +++ b/clang/test/Analysis/retain-release-region-store.m @@ -75,7 +75,7 @@ extern DADiskRef DADiskCopyWholeDisk( DADiskRef disk ); @interface NSAppleEventManager : NSObject { } @end enum { -kDAReturnSuccess = 0, kDAReturnError = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x01, kDAReturnBusy = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x02, kDAReturnBadArgument = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x03, kDAReturnExclusiveAccess = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x04, kDAReturnNoResources = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x05, kDAReturnNotFound = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x06, kDAReturnNotMounted = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x07, kDAReturnNotPermitted = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x08, kDAReturnNotPrivileged = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x09, kDAReturnNotReady = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x0A, kDAReturnNotWritable = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x0B, kDAReturnUnsupported = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x0C }; +kDAReturnSuccess = 0, kDAReturnError = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x01, kDAReturnBusy = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x02, kDAReturnBadArgument = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x03, kDAReturnExclusiveAccess = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x04, kDAReturnNoResources = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x05, kDAReturnNotFound = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x06, kDAReturnNotMounted = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x07, kDAReturnNotPermitted = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x08, kDAReturnNotPrivileged = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x09, kDAReturnNotReady = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x0A, kDAReturnNotWritable = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x0B, kDAReturnUnsupported = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x0C }; typedef mach_error_t DAReturn; typedef const struct __DADissenter * DADissenterRef; extern DADissenterRef DADissenterCreate( CFAllocatorRef allocator, DAReturn status, CFStringRef string ); diff --git a/clang/test/Analysis/retain-release.m b/clang/test/Analysis/retain-release.m index 81e015f5fc44..71e1620aa534 100644 --- a/clang/test/Analysis/retain-release.m +++ b/clang/test/Analysis/retain-release.m @@ -217,7 +217,7 @@ CVTimeStamp; } typedef int CIFormat; @end enum { -kDAReturnSuccess = 0, kDAReturnError = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x01, kDAReturnBusy = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x02, kDAReturnBadArgument = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x03, kDAReturnExclusiveAccess = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x04, kDAReturnNoResources = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x05, kDAReturnNotFound = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x06, kDAReturnNotMounted = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x07, kDAReturnNotPermitted = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x08, kDAReturnNotPrivileged = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x09, kDAReturnNotReady = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x0A, kDAReturnNotWritable = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x0B, kDAReturnUnsupported = (((0x3e)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x0C }; +kDAReturnSuccess = 0, kDAReturnError = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x01, kDAReturnBusy = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x02, kDAReturnBadArgument = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x03, kDAReturnExclusiveAccess = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x04, kDAReturnNoResources = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x05, kDAReturnNotFound = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x06, kDAReturnNotMounted = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x07, kDAReturnNotPermitted = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x08, kDAReturnNotPrivileged = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x09, kDAReturnNotReady = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x0A, kDAReturnNotWritable = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x0B, kDAReturnUnsupported = (((0x3eU)&0x3f)<<26) | (((0x368)&0xfff)<<14) | 0x0C }; typedef mach_error_t DAReturn; typedef const struct __DADissenter * DADissenterRef; extern DADissenterRef DADissenterCreate( CFAllocatorRef allocator, DAReturn status, CFStringRef string ); diff --git a/clang/test/Sema/shift.c b/clang/test/Sema/shift.c index 4273cab98ee3..63c97fe7b029 100644 --- a/clang/test/Sema/shift.c +++ b/clang/test/Sema/shift.c @@ -1,7 +1,9 @@ -// RUN: %clang -Wall -ffreestanding -fsyntax-only -Xclang -verify %s +// RUN: %clang -Wall -Wshift-sign-overflow -ffreestanding -fsyntax-only -Xclang -verify %s #include +#define WORD_BIT (sizeof(int) * CHAR_BIT) + enum { X = 1 << 0, Y = 1 << 1, @@ -32,6 +34,22 @@ void test() { c <<= CHAR_BIT+1; // expected-warning {{shift count >= width of type}} c >>= CHAR_BIT+1; // expected-warning {{shift count >= width of type}} (void)((long)c << CHAR_BIT); + + int i; + i = 1 << (WORD_BIT - 2); + i = 2 << (WORD_BIT - 1); // expected-warning {{the promoted type of the shift expression is 'int'}} + i = 1 << (WORD_BIT - 1); // expected-warning {{overrides the sign bit of the promoted type of the shift expression ('int')}} + i = -1 << (WORD_BIT - 1); + i = 0 << (WORD_BIT - 1); + i = (char)1 << (WORD_BIT - 2); + + unsigned u; + u = 1U << (WORD_BIT - 1); + u = 5U << (WORD_BIT - 1); + + long long int lli; + lli = INT_MIN << 2; // expected-warning {{the promoted type of the shift expression is 'int'}} + lli = 1LL << (sizeof(long long) * CHAR_BIT - 2); } #define a 0