[cfe-commits] r173377 - in /cfe/trunk: include/clang/AST/Expr.h include/clang/Basic/DiagnosticASTKinds.td include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/AST/ExprConstant.cpp lib/Sema/SemaChecking.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaExprCXX.cpp lib/Sema/SemaStmt.cpp test/Sema/switch-1.c
Richard Smith
richard at metafoo.co.uk
Thu Jan 24 15:11:55 PST 2013
On Thu, Jan 24, 2013 at 2:11 PM, Fariborz Jahanian <fjahanian at apple.com> wrote:
> Author: fjahanian
> Date: Thu Jan 24 16:11:45 2013
> New Revision: 173377
>
> URL: http://llvm.org/viewvc/llvm-project?rev=173377&view=rev
> Log:
> Patch to check for integer overflow. It has been
> commented on and approved by Richard Smith.
>
> Modified:
> cfe/trunk/include/clang/AST/Expr.h
> cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/include/clang/Sema/Sema.h
> cfe/trunk/lib/AST/ExprConstant.cpp
> cfe/trunk/lib/Sema/SemaChecking.cpp
> cfe/trunk/lib/Sema/SemaDecl.cpp
> cfe/trunk/lib/Sema/SemaExprCXX.cpp
> cfe/trunk/lib/Sema/SemaStmt.cpp
> cfe/trunk/test/Sema/switch-1.c
>
> Modified: cfe/trunk/include/clang/AST/Expr.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=173377&r1=173376&r2=173377&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/Expr.h (original)
> +++ cfe/trunk/include/clang/AST/Expr.h Thu Jan 24 16:11:45 2013
> @@ -570,6 +570,9 @@
> /// integer.
> llvm::APSInt EvaluateKnownConstInt(const ASTContext &Ctx,
> SmallVectorImpl<PartialDiagnosticAt> *Diag=0) const;
> +
> + void EvaluateForOverflow(const ASTContext &Ctx,
> + SmallVectorImpl<PartialDiagnosticAt> *Diag) const;
>
> /// EvaluateAsLValue - Evaluate an expression to see if we can fold it to an
> /// lvalue with link time known address, with no side-effects.
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td?rev=173377&r1=173376&r2=173377&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td Thu Jan 24 16:11:45 2013
> @@ -106,6 +106,9 @@
> "(skipping %0 call%s0 in backtrace; use -fconstexpr-backtrace-limit=0 to "
> "see all)">;
> def note_constexpr_call_here : Note<"in call to '%0'">;
> +def warn_integer_constant_overflow : Warning<
> + "overflow in case constant expression results in value %0 of type %1">,
> + InGroup<DiagGroup<"integer-overflow">>;
This diagnostic text is wrong, since we're checking for overflow in
all expressions, not just case constant expressions.
> // inline asm related.
> let CategoryName = "Inline Assembly Issue" in {
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=173377&r1=173376&r2=173377&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Jan 24 16:11:45 2013
> @@ -5756,9 +5756,6 @@
> def warn_case_value_overflow : Warning<
> "overflow converting case value to switch condition type (%0 to %1)">,
> InGroup<Switch>;
> -def warn_case_constant_overflow : Warning<
> - "overflow in case constant expression results in value %0">,
> - InGroup<Switch>;
> def err_duplicate_case : Error<"duplicate case value '%0'">;
> def err_duplicate_case_differing_expr : Error<
> "duplicate case value: '%0' and '%1' both equal '%2'">;
>
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=173377&r1=173376&r2=173377&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Thu Jan 24 16:11:45 2013
> @@ -3993,7 +3993,8 @@
> : SourceLocation());
> }
> ExprResult ActOnFinishFullExpr(Expr *Expr, SourceLocation CC,
> - bool DiscardedValue = false);
> + bool DiscardedValue = false,
> + bool IsConstexpr = false);
> StmtResult ActOnFinishFullStmt(Stmt *Stmt);
>
> // Marks SS invalid if it represents an incomplete type.
> @@ -7328,11 +7329,13 @@
> SourceLocation ReturnLoc);
> void CheckFloatComparison(SourceLocation Loc, Expr* LHS, Expr* RHS);
> void CheckImplicitConversions(Expr *E, SourceLocation CC = SourceLocation());
> + void CheckForIntOverflow(Expr *E);
> void CheckUnsequencedOperations(Expr *E);
>
> /// \brief Perform semantic checks on a completed expression. This will either
> /// be a full-expression or a default argument expression.
> - void CheckCompletedExpr(Expr *E, SourceLocation CheckLoc = SourceLocation());
> + void CheckCompletedExpr(Expr *E, SourceLocation CheckLoc = SourceLocation(),
> + bool IsConstexpr = false);
>
> void CheckBitFieldInitialization(SourceLocation InitLoc, FieldDecl *Field,
> Expr *Init);
>
> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=173377&r1=173376&r2=173377&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> +++ cfe/trunk/lib/AST/ExprConstant.cpp Thu Jan 24 16:11:45 2013
> @@ -384,13 +384,17 @@
> /// expression is a potential constant expression? If so, some diagnostics
> /// are suppressed.
> bool CheckingPotentialConstantExpression;
> +
> + bool IntOverflowCheckMode;
>
> - EvalInfo(const ASTContext &C, Expr::EvalStatus &S)
> + EvalInfo(const ASTContext &C, Expr::EvalStatus &S,
> + bool OverflowCheckMode=false)
> : Ctx(const_cast<ASTContext&>(C)), EvalStatus(S), CurrentCall(0),
> CallStackDepth(0), NextCallIndex(1),
> BottomFrame(*this, SourceLocation(), 0, 0, 0),
> EvaluatingDecl(0), EvaluatingDeclValue(0), HasActiveDiagnostic(false),
> - CheckingPotentialConstantExpression(false) {}
> + CheckingPotentialConstantExpression(false),
> + IntOverflowCheckMode(OverflowCheckMode) {}
>
> void setEvaluatingDecl(const VarDecl *VD, APValue &Value) {
> EvaluatingDecl = VD;
> @@ -474,6 +478,8 @@
> return OptionalDiagnostic();
> }
>
> + bool getIntOverflowCheckMode() { return IntOverflowCheckMode; }
> +
> /// Diagnose that the evaluation does not produce a C++11 core constant
> /// expression.
> template<typename LocArg>
> @@ -506,8 +512,12 @@
> /// Should we continue evaluation as much as possible after encountering a
> /// construct which can't be folded?
> bool keepEvaluatingAfterFailure() {
> - return CheckingPotentialConstantExpression &&
> - EvalStatus.Diag && EvalStatus.Diag->empty();
> + // Should return true in IntOverflowCheckMode, so that we check for
> + // overflow even if some subexpressions can't be evaluated as constants.
> + // subexpressions can't be evaluated as constants.
> + return IntOverflowCheckMode ||
> + (CheckingPotentialConstantExpression &&
> + EvalStatus.Diag && EvalStatus.Diag->empty());
> }
> };
>
> @@ -4418,8 +4428,14 @@
>
> APSInt Value(Op(LHS.extend(BitWidth), RHS.extend(BitWidth)), false);
> APSInt Result = Value.trunc(LHS.getBitWidth());
> - if (Result.extend(BitWidth) != Value)
> - HandleOverflow(Info, E, Value, E->getType());
> + if (Result.extend(BitWidth) != Value) {
> + if (Info.getIntOverflowCheckMode())
> + Info.Ctx.getDiagnostics().Report(E->getExprLoc(),
> + diag::warn_integer_constant_overflow)
> + << Result.toString(10) << E->getType();
> + else
> + HandleOverflow(Info, E, Value, E->getType());
> + }
> return Result;
> }
>
> @@ -6260,26 +6276,39 @@
> return CheckConstantExpression(Info, E->getExprLoc(), E->getType(), Result);
> }
>
> -/// EvaluateAsRValue - Return true if this is a constant which we can fold using
> -/// any crazy technique (that has nothing to do with language standards) that
> -/// we want to. If this function returns true, it returns the folded constant
> -/// in Result. If this expression is a glvalue, an lvalue-to-rvalue conversion
> -/// will be applied to the result.
> -bool Expr::EvaluateAsRValue(EvalResult &Result, const ASTContext &Ctx) const {
> +static bool FastEvaluateAsRValue(const Expr *Exp, Expr::EvalResult &Result,
> + const ASTContext &Ctx, bool &IsConst) {
> // Fast-path evaluations of integer literals, since we sometimes see files
> // containing vast quantities of these.
> - if (const IntegerLiteral *L = dyn_cast<IntegerLiteral>(this)) {
> + if (const IntegerLiteral *L = dyn_cast<IntegerLiteral>(Exp)) {
> Result.Val = APValue(APSInt(L->getValue(),
> L->getType()->isUnsignedIntegerType()));
> + IsConst = true;
> return true;
> }
> -
> +
> // FIXME: Evaluating values of large array and record types can cause
> // performance problems. Only do so in C++11 for now.
> - if (isRValue() && (getType()->isArrayType() || getType()->isRecordType()) &&
> - !Ctx.getLangOpts().CPlusPlus11)
> - return false;
> + if (Exp->isRValue() && (Exp->getType()->isArrayType() ||
> + Exp->getType()->isRecordType()) &&
> + !Ctx.getLangOpts().CPlusPlus11) {
> + IsConst = false;
> + return true;
> + }
> + return false;
> +}
> +
>
> +/// EvaluateAsRValue - Return true if this is a constant which we can fold using
> +/// any crazy technique (that has nothing to do with language standards) that
> +/// we want to. If this function returns true, it returns the folded constant
> +/// in Result. If this expression is a glvalue, an lvalue-to-rvalue conversion
> +/// will be applied to the result.
> +bool Expr::EvaluateAsRValue(EvalResult &Result, const ASTContext &Ctx) const {
> + bool IsConst;
> + if (FastEvaluateAsRValue(this, Result, Ctx, IsConst))
> + return IsConst;
> +
> EvalInfo Info(Ctx, Result);
> return ::EvaluateAsRValue(Info, this, Result.Val);
> }
> @@ -6376,6 +6405,17 @@
> return EvalResult.Val.getInt();
> }
>
> +void Expr::EvaluateForOverflow(const ASTContext &Ctx,
> + SmallVectorImpl<PartialDiagnosticAt> *Diags) const {
> + bool IsConst;
> + EvalResult EvalResult;
> + EvalResult.Diag = Diags;
> + if (!FastEvaluateAsRValue(this, EvalResult, Ctx, IsConst)) {
> + EvalInfo Info(Ctx, EvalResult, true);
> + (void)::EvaluateAsRValue(Info, this, EvalResult.Val);
> + }
> +}
> +
> bool Expr::EvalResult::isGlobalLValue() const {
> assert(Val.isLValue());
> return IsGlobalLValue(Val.getLValueBase());
>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=173377&r1=173376&r2=173377&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Jan 24 16:11:45 2013
> @@ -5183,6 +5183,18 @@
> AnalyzeImplicitConversions(*this, E, CC);
> }
>
> +/// Diagnose when expression is an integer constant expression and its evaluation
> +/// results in integer overflow
> +void Sema::CheckForIntOverflow (Expr *E) {
> + if (const BinaryOperator *BExpr = dyn_cast<BinaryOperator>(E->IgnoreParens())) {
> + unsigned Opc = BExpr->getOpcode();
> + if (Opc != BO_Add && Opc != BO_Sub && Opc != BO_Mul)
> + return;
> + llvm::SmallVector<PartialDiagnosticAt, 4> Diags;
> + E->EvaluateForOverflow(Context, &Diags);
> + }
> +}
> +
> namespace {
> /// \brief Visitor for expressions which looks for unsequenced operations on the
> /// same object.
> @@ -5622,9 +5634,12 @@
> }
> }
>
> -void Sema::CheckCompletedExpr(Expr *E, SourceLocation CheckLoc) {
> +void Sema::CheckCompletedExpr(Expr *E, SourceLocation CheckLoc,
> + bool IsConstexpr) {
> CheckImplicitConversions(E, CheckLoc);
> CheckUnsequencedOperations(E);
> + if (!IsConstexpr && !E->isValueDependent())
> + CheckForIntOverflow(E);
> }
>
> void Sema::CheckBitFieldInitialization(SourceLocation InitLoc,
>
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=173377&r1=173376&r2=173377&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Thu Jan 24 16:11:45 2013
> @@ -7045,7 +7045,9 @@
> // struct T { S a, b; } t = { Temp(), Temp() }
> //
> // we should destroy the first Temp before constructing the second.
> - ExprResult Result = ActOnFinishFullExpr(Init, VDecl->getLocation());
> + ExprResult Result = ActOnFinishFullExpr(Init, VDecl->getLocation(),
> + false,
> + VDecl->isConstexpr());
> if (Result.isInvalid()) {
> VDecl->setInvalidDecl();
> return;
>
> Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=173377&r1=173376&r2=173377&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Thu Jan 24 16:11:45 2013
> @@ -5469,7 +5469,8 @@
> }
>
> ExprResult Sema::ActOnFinishFullExpr(Expr *FE, SourceLocation CC,
> - bool DiscardedValue) {
> + bool DiscardedValue,
> + bool IsConstexpr) {
> ExprResult FullExpr = Owned(FE);
>
> if (!FullExpr.get())
> @@ -5497,7 +5498,7 @@
> return ExprError();
> }
>
> - CheckCompletedExpr(FullExpr.get(), CC);
> + CheckCompletedExpr(FullExpr.get(), CC, IsConstexpr);
> return MaybeCreateExprWithCleanups(FullExpr);
> }
>
>
> Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=173377&r1=173376&r2=173377&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaStmt.cpp Thu Jan 24 16:11:45 2013
> @@ -338,6 +338,12 @@
> // Recover from an error by just forgetting about it.
> }
> }
> +
> + LHSVal = ActOnFinishFullExpr(LHSVal, LHSVal->getExprLoc(), false,
> + getLangOpts().CPlusPlus11).take();
> + if (RHSVal)
> + RHSVal = ActOnFinishFullExpr(RHSVal, RHSVal->getExprLoc(), false,
> + getLangOpts().CPlusPlus11).take();
>
> CaseStmt *CS = new (Context) CaseStmt(LHSVal, RHSVal, CaseLoc, DotDotDotLoc,
> ColonLoc);
> @@ -732,14 +738,7 @@
> } else {
> // We already verified that the expression has a i-c-e value (C99
> // 6.8.4.2p3) - get that value now.
> - SmallVector<PartialDiagnosticAt, 8> Diags;
> - LoVal = Lo->EvaluateKnownConstInt(Context, &Diags);
> - if (Diags.size() == 1 &&
> - Diags[0].second.getDiagID() == diag::note_constexpr_overflow) {
> - Diag(Lo->getLocStart(), diag::warn_case_constant_overflow) <<
> - LoVal.toString(10);
> - Diag(Diags[0].first, Diags[0].second);
> - }
> + LoVal = Lo->EvaluateKnownConstInt(Context);
>
> // If the LHS is not the same type as the condition, insert an implicit
> // cast.
>
> Modified: cfe/trunk/test/Sema/switch-1.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/switch-1.c?rev=173377&r1=173376&r2=173377&view=diff
> ==============================================================================
> --- cfe/trunk/test/Sema/switch-1.c (original)
> +++ cfe/trunk/test/Sema/switch-1.c Thu Jan 24 16:11:45 2013
> @@ -4,12 +4,12 @@
>
> int f(int i) {
> switch (i) {
> - case 2147483647 + 2: // expected-note {{value 2147483649 is outside the range of representable values of type 'int'}} \
> - // expected-warning {{overflow in case constant expression results in value -2147483647}}
> + case 2147483647 + 2: // expected-warning {{overflow in case constant expression results in value -2147483647 of type 'int'}}
> return 1;
> - case 9223372036854775807L * 4 : // expected-note {{value 36893488147419103228 is outside the range of representable values of type 'long'}} \
> - // expected-warning {{overflow in case constant expression results in value -4}}
> + case 9223372036854775807L * 4: // expected-warning {{overflow in case constant expression results in value -4 of type 'long'}}
> return 2;
> + case (123456 *789012) + 1: // expected-warning {{overflow in case constant expression results in value -1375982336 of type 'int'}}
> + return 3;
> case 2147483647:
> return 0;
> }
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list