Skip to content

Commit

Permalink
[clang] perform semantic checking in constant context
Browse files Browse the repository at this point in the history
Summary:
Since the addition of __builtin_is_constant_evaluated the result of an expression can change based on whether it is evaluated in constant context. a lot of semantic checking performs evaluations with out specifying context. which can lead to wrong diagnostics.
for example:
```
constexpr int i0 = (long long)__builtin_is_constant_evaluated() * (1ll << 33); //#1
constexpr int i1 = (long long)!__builtin_is_constant_evaluated() * (1ll << 33); //#2
```
before the patch, #2 was diagnosed incorrectly and #1 wasn't diagnosed.
after the patch #1 is diagnosed as it should and #2 isn't.

Changes:
 - add a flag to Sema to passe in constant context mode.
 - in SemaChecking.cpp calls to Expr::Evaluate* are now done in constant context when they should.
 - in SemaChecking.cpp diagnostics for UB are not checked for in constant context because an error will be emitted by the constant evaluator.
 - in SemaChecking.cpp diagnostics for construct that cannot appear in constant context are not checked for in constant context.
 - in SemaChecking.cpp diagnostics on constant expression are always emitted because constant expression are always evaluated.
 - semantic checking for initialization of constexpr variables is now done in constant context.
 - adapt test that were depending on warning changes.
 - add test.

Reviewers: rsmith

Reviewed By: rsmith

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D62009

llvm-svn: 363488
  • Loading branch information
Ralender committed Jun 15, 2019
1 parent e1aa69f commit 0bb4d46
Show file tree
Hide file tree
Showing 8 changed files with 215 additions and 96 deletions.
21 changes: 12 additions & 9 deletions clang/include/clang/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,8 @@ class Expr : public ValueStmt {
/// which we can fold and convert to a boolean condition using
/// any crazy technique that we want to, even if the expression has
/// side-effects.
bool EvaluateAsBooleanCondition(bool &Result, const ASTContext &Ctx) const;
bool EvaluateAsBooleanCondition(bool &Result, const ASTContext &Ctx,
bool InConstantContext = false) const;

enum SideEffectsKind {
SE_NoSideEffects, ///< Strictly evaluate the expression.
Expand All @@ -611,20 +612,21 @@ class Expr : public ValueStmt {
/// EvaluateAsInt - Return true if this is a constant which we can fold and
/// convert to an integer, using any crazy technique that we want to.
bool EvaluateAsInt(EvalResult &Result, const ASTContext &Ctx,
SideEffectsKind AllowSideEffects = SE_NoSideEffects) const;
SideEffectsKind AllowSideEffects = SE_NoSideEffects,
bool InConstantContext = false) const;

/// EvaluateAsFloat - Return true if this is a constant which we can fold and
/// convert to a floating point value, using any crazy technique that we
/// want to.
bool
EvaluateAsFloat(llvm::APFloat &Result, const ASTContext &Ctx,
SideEffectsKind AllowSideEffects = SE_NoSideEffects) const;
bool EvaluateAsFloat(llvm::APFloat &Result, const ASTContext &Ctx,
SideEffectsKind AllowSideEffects = SE_NoSideEffects,
bool InConstantContext = false) const;

/// EvaluateAsFloat - Return true if this is a constant which we can fold and
/// convert to a fixed point value.
bool EvaluateAsFixedPoint(
EvalResult &Result, const ASTContext &Ctx,
SideEffectsKind AllowSideEffects = SE_NoSideEffects) const;
bool EvaluateAsFixedPoint(EvalResult &Result, const ASTContext &Ctx,
SideEffectsKind AllowSideEffects = SE_NoSideEffects,
bool InConstantContext = false) const;

/// isEvaluatable - Call EvaluateAsRValue to see if this expression can be
/// constant folded without side-effects, but discard the result.
Expand Down Expand Up @@ -660,7 +662,8 @@ class Expr : public ValueStmt {

/// EvaluateAsLValue - Evaluate an expression to see if we can fold it to an
/// lvalue with link time known address, with no side-effects.
bool EvaluateAsLValue(EvalResult &Result, const ASTContext &Ctx) const;
bool EvaluateAsLValue(EvalResult &Result, const ASTContext &Ctx,
bool InConstantContext = false) const;

/// EvaluateAsInitializer - Evaluate an expression as if it were the
/// initializer of the given declaration. Returns true if the initializer
Expand Down
2 changes: 2 additions & 0 deletions clang/include/clang/Basic/DiagnosticASTKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ def note_constexpr_past_end_subobject : Note<
"access array element of|ERROR|"
"access real component of|access imaginary component of}0 "
"pointer past the end of object">;
def note_non_null_attribute_failed : Note<
"null passed to a callee that requires a non-null argument">;
def note_constexpr_null_subobject : Note<
"cannot %select{access base class of|access derived class of|access field of|"
"access array element of|perform pointer arithmetic on|"
Expand Down
9 changes: 9 additions & 0 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -797,6 +797,15 @@ class Sema {
}
};

/// Used to change context to isConstantEvaluated without pushing a heavy
/// ExpressionEvaluationContextRecord object.
bool isConstantEvaluatedOverride;

bool isConstantEvaluated() {
return ExprEvalContexts.back().isConstantEvaluated() ||
isConstantEvaluatedOverride;
}

/// RAII object to handle the state changes required to synthesize
/// a function body.
class SynthesizedFunctionScope {
Expand Down
59 changes: 45 additions & 14 deletions clang/lib/AST/ExprConstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
#include "clang/Basic/Builtins.h"
#include "clang/Basic/FixedPoint.h"
#include "clang/Basic/TargetInfo.h"
#include "llvm/ADT/SmallBitVector.h"
#include "llvm/Support/SaveAndRestore.h"
#include "llvm/Support/raw_ostream.h"
#include <cstring>
Expand Down Expand Up @@ -5091,9 +5092,25 @@ typedef SmallVector<APValue, 8> ArgVector;
}

/// EvaluateArgs - Evaluate the arguments to a function call.
static bool EvaluateArgs(ArrayRef<const Expr*> Args, ArgVector &ArgValues,
EvalInfo &Info) {
static bool EvaluateArgs(ArrayRef<const Expr *> Args, ArgVector &ArgValues,
EvalInfo &Info, const FunctionDecl *Callee) {
bool Success = true;
llvm::SmallBitVector ForbiddenNullArgs;
if (Callee->hasAttr<NonNullAttr>()) {
ForbiddenNullArgs.resize(Args.size());
for (const auto *Attr : Callee->specific_attrs<NonNullAttr>()) {
if (!Attr->args_size()) {
ForbiddenNullArgs.set();
break;
} else
for (auto Idx : Attr->args()) {
unsigned ASTIdx = Idx.getASTIndex();
if (ASTIdx >= Args.size())
continue;
ForbiddenNullArgs[ASTIdx] = 1;
}
}
}
for (ArrayRef<const Expr*>::iterator I = Args.begin(), E = Args.end();
I != E; ++I) {
if (!Evaluate(ArgValues[I - Args.begin()], Info, *I)) {
Expand All @@ -5102,6 +5119,13 @@ static bool EvaluateArgs(ArrayRef<const Expr*> Args, ArgVector &ArgValues,
if (!Info.noteFailure())
return false;
Success = false;
} else if (!ForbiddenNullArgs.empty() &&
ForbiddenNullArgs[I - Args.begin()] &&
ArgValues[I - Args.begin()].isNullPointer()) {
Info.CCEDiag(*I, diag::note_non_null_attribute_failed);
if (!Info.noteFailure())
return false;
Success = false;
}
}
return Success;
Expand All @@ -5114,7 +5138,7 @@ static bool HandleFunctionCall(SourceLocation CallLoc,
EvalInfo &Info, APValue &Result,
const LValue *ResultSlot) {
ArgVector ArgValues(Args.size());
if (!EvaluateArgs(Args, ArgValues, Info))
if (!EvaluateArgs(Args, ArgValues, Info, Callee))
return false;

if (!Info.CheckCallLimit(CallLoc))
Expand Down Expand Up @@ -5338,7 +5362,7 @@ static bool HandleConstructorCall(const Expr *E, const LValue &This,
const CXXConstructorDecl *Definition,
EvalInfo &Info, APValue &Result) {
ArgVector ArgValues(Args.size());
if (!EvaluateArgs(Args, ArgValues, Info))
if (!EvaluateArgs(Args, ArgValues, Info, Definition))
return false;

return HandleConstructorCall(E, This, ArgValues.data(), Definition,
Expand Down Expand Up @@ -11855,54 +11879,61 @@ bool Expr::EvaluateAsRValue(EvalResult &Result, const ASTContext &Ctx,
return ::EvaluateAsRValue(this, Result, Ctx, Info);
}

bool Expr::EvaluateAsBooleanCondition(bool &Result,
const ASTContext &Ctx) const {
bool Expr::EvaluateAsBooleanCondition(bool &Result, const ASTContext &Ctx,
bool InConstantContext) const {
assert(!isValueDependent() &&
"Expression evaluator can't be called on a dependent expression.");
EvalResult Scratch;
return EvaluateAsRValue(Scratch, Ctx) &&
return EvaluateAsRValue(Scratch, Ctx, InConstantContext) &&
HandleConversionToBool(Scratch.Val, Result);
}

bool Expr::EvaluateAsInt(EvalResult &Result, const ASTContext &Ctx,
SideEffectsKind AllowSideEffects) const {
SideEffectsKind AllowSideEffects,
bool InConstantContext) const {
assert(!isValueDependent() &&
"Expression evaluator can't be called on a dependent expression.");
EvalInfo Info(Ctx, Result, EvalInfo::EM_IgnoreSideEffects);
Info.InConstantContext = InConstantContext;
return ::EvaluateAsInt(this, Result, Ctx, AllowSideEffects, Info);
}

bool Expr::EvaluateAsFixedPoint(EvalResult &Result, const ASTContext &Ctx,
SideEffectsKind AllowSideEffects) const {
SideEffectsKind AllowSideEffects,
bool InConstantContext) const {
assert(!isValueDependent() &&
"Expression evaluator can't be called on a dependent expression.");
EvalInfo Info(Ctx, Result, EvalInfo::EM_IgnoreSideEffects);
Info.InConstantContext = InConstantContext;
return ::EvaluateAsFixedPoint(this, Result, Ctx, AllowSideEffects, Info);
}

bool Expr::EvaluateAsFloat(APFloat &Result, const ASTContext &Ctx,
SideEffectsKind AllowSideEffects) const {
SideEffectsKind AllowSideEffects,
bool InConstantContext) const {
assert(!isValueDependent() &&
"Expression evaluator can't be called on a dependent expression.");

if (!getType()->isRealFloatingType())
return false;

EvalResult ExprResult;
if (!EvaluateAsRValue(ExprResult, Ctx) || !ExprResult.Val.isFloat() ||
if (!EvaluateAsRValue(ExprResult, Ctx, InConstantContext) ||
!ExprResult.Val.isFloat() ||
hasUnacceptableSideEffect(ExprResult, AllowSideEffects))
return false;

Result = ExprResult.Val.getFloat();
return true;
}

bool Expr::EvaluateAsLValue(EvalResult &Result, const ASTContext &Ctx) const {
bool Expr::EvaluateAsLValue(EvalResult &Result, const ASTContext &Ctx,
bool InConstantContext) const {
assert(!isValueDependent() &&
"Expression evaluator can't be called on a dependent expression.");

EvalInfo Info(Ctx, Result, EvalInfo::EM_ConstantFold);

Info.InConstantContext = InConstantContext;
LValue LV;
if (!EvaluateLValue(this, LV, Info) || Result.HasSideEffects ||
!CheckLValueConstantExpression(Info, getExprLoc(),
Expand Down Expand Up @@ -12685,7 +12716,7 @@ bool Expr::isPotentialConstantExprUnevaluated(Expr *E,
// Fabricate a call stack frame to give the arguments a plausible cover story.
ArrayRef<const Expr*> Args;
ArgVector ArgValues(0);
bool Success = EvaluateArgs(Args, ArgValues, Info);
bool Success = EvaluateArgs(Args, ArgValues, Info, FD);
(void)Success;
assert(Success &&
"Failed to set up arguments for potential constant evaluation");
Expand Down
1 change: 1 addition & 0 deletions clang/lib/Sema/Sema.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
ThreadSafetyDeclCache(nullptr), VarDataSharingAttributesStack(nullptr),
CurScope(nullptr), Ident_super(nullptr), Ident___float128(nullptr) {
TUScope = nullptr;
isConstantEvaluatedOverride = false;

LoadedExternalKnownNamespaces = false;
for (unsigned I = 0; I != NSAPI::NumNSNumberLiteralMethods; ++I)
Expand Down
Loading

0 comments on commit 0bb4d46

Please sign in to comment.