From: Richard Smith Date: Sat, 27 Apr 2019 02:58:17 +0000 (+0000) Subject: Reinstate r359059, reverted in r359361, with a fix to properly prevent X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=009d37df084524bc027fa893b47b17faba9815fa;p=clang Reinstate r359059, reverted in r359361, with a fix to properly prevent us emitting the operand of __builtin_constant_p if it has side-effects. Original commit message: Fix interactions between __builtin_constant_p and constexpr to match current trunk GCC. GCC permits information from outside the operand of __builtin_constant_p (but in the same constant evaluation context) to be used within that operand; clang now does so too. A few other minor deviations from GCC's behavior showed up in my testing and are also fixed (matching GCC): * Clang now supports nullptr_t as the argument type for __builtin_constant_p * Clang now returns true from __builtin_constant_p if called with a null pointer * Clang now returns true from __builtin_constant_p if called with an integer cast to pointer type git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@359367 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/AST/ExprConstant.cpp b/lib/AST/ExprConstant.cpp index a30e83c632..24b28971af 100644 --- a/lib/AST/ExprConstant.cpp +++ b/lib/AST/ExprConstant.cpp @@ -7801,19 +7801,33 @@ EvaluateBuiltinClassifyType(const CallExpr *E, const LangOptions &LangOpts) { } /// EvaluateBuiltinConstantPForLValue - Determine the result of -/// __builtin_constant_p when applied to the given lvalue. +/// __builtin_constant_p when applied to the given pointer. /// -/// An lvalue is only "constant" if it is a pointer or reference to the first -/// character of a string literal. -template -static bool EvaluateBuiltinConstantPForLValue(const LValue &LV) { - const Expr *E = LV.getLValueBase().template dyn_cast(); - return E && isa(E) && LV.getLValueOffset().isZero(); +/// A pointer is only "constant" if it is null (or a pointer cast to integer) +/// or it points to the first character of a string literal. +static bool EvaluateBuiltinConstantPForLValue(const APValue &LV) { + APValue::LValueBase Base = LV.getLValueBase(); + if (Base.isNull()) { + // A null base is acceptable. + return true; + } else if (const Expr *E = Base.dyn_cast()) { + if (!isa(E)) + return false; + return LV.getLValueOffset().isZero(); + } else { + // Any other base is not constant enough for GCC. + return false; + } } /// EvaluateBuiltinConstantP - Evaluate __builtin_constant_p as similarly to /// GCC as we can manage. -static bool EvaluateBuiltinConstantP(ASTContext &Ctx, const Expr *Arg) { +static bool EvaluateBuiltinConstantP(EvalInfo &Info, const Expr *Arg) { + // Constant-folding is always enabled for the operand of __builtin_constant_p + // (even when the enclosing evaluation context otherwise requires a strict + // language-specific constant expression). + FoldConstant Fold(Info, true); + QualType ArgType = Arg->getType(); // __builtin_constant_p always has one operand. The rules which gcc follows @@ -7821,34 +7835,30 @@ static bool EvaluateBuiltinConstantP(ASTContext &Ctx, const Expr *Arg) { // // - If the operand is of integral, floating, complex or enumeration type, // and can be folded to a known value of that type, it returns 1. - // - If the operand and can be folded to a pointer to the first character - // of a string literal (or such a pointer cast to an integral type), it - // returns 1. + // - If the operand can be folded to a pointer to the first character + // of a string literal (or such a pointer cast to an integral type) + // or to a null pointer or an integer cast to a pointer, it returns 1. // // Otherwise, it returns 0. // // FIXME: GCC also intends to return 1 for literals of aggregate types, but - // its support for this does not currently work. - if (ArgType->isIntegralOrEnumerationType()) { - Expr::EvalResult Result; - if (!Arg->EvaluateAsRValue(Result, Ctx) || Result.HasSideEffects) + // its support for this did not work prior to GCC 9 and is not yet well + // understood. + if (ArgType->isIntegralOrEnumerationType() || ArgType->isFloatingType() || + ArgType->isAnyComplexType() || ArgType->isPointerType() || + ArgType->isNullPtrType()) { + APValue V; + if (!::EvaluateAsRValue(Info, Arg, V)) { + Fold.keepDiagnostics(); return false; + } - APValue &V = Result.Val; - if (V.getKind() == APValue::Int) - return true; + // For a pointer (possibly cast to integer), there are special rules. if (V.getKind() == APValue::LValue) return EvaluateBuiltinConstantPForLValue(V); - } else if (ArgType->isFloatingType() || ArgType->isAnyComplexType()) { - return Arg->isEvaluatable(Ctx); - } else if (ArgType->isPointerType() || Arg->isGLValue()) { - LValue LV; - Expr::EvalStatus Status; - EvalInfo Info(Ctx, Status, EvalInfo::EM_ConstantFold); - if ((Arg->isGLValue() ? EvaluateLValue(Arg, LV, Info) - : EvaluatePointer(Arg, LV, Info)) && - !Status.HasSideEffects) - return EvaluateBuiltinConstantPForLValue(LV); + + // Otherwise, any constant value is good enough. + return V.getKind() != APValue::Uninitialized; } // Anything else isn't considered to be sufficiently constant. @@ -8258,18 +8268,15 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E, } case Builtin::BI__builtin_constant_p: { - auto Arg = E->getArg(0); - if (EvaluateBuiltinConstantP(Info.Ctx, Arg)) + const Expr *Arg = E->getArg(0); + if (EvaluateBuiltinConstantP(Info, Arg)) return Success(true, E); - auto ArgTy = Arg->IgnoreImplicit()->getType(); - if (!Info.InConstantContext && !Arg->HasSideEffects(Info.Ctx) && - !ArgTy->isAggregateType() && !ArgTy->isPointerType()) { - // We can delay calculation of __builtin_constant_p until after - // inlining. Note: This diagnostic won't be shown to the user. + else if (Info.InConstantContext) + return Success(false, E); + else { Info.FFDiag(E, diag::note_invalid_subexpr_in_const_expr); return false; } - return Success(false, E); } case Builtin::BI__builtin_is_constant_evaluated: diff --git a/lib/CodeGen/CGBuiltin.cpp b/lib/CodeGen/CGBuiltin.cpp index bc5f6cfe43..5abb62c560 100644 --- a/lib/CodeGen/CGBuiltin.cpp +++ b/lib/CodeGen/CGBuiltin.cpp @@ -2027,8 +2027,17 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, const Expr *Arg = E->getArg(0); QualType ArgType = Arg->getType(); - if (!hasScalarEvaluationKind(ArgType) || ArgType->isFunctionType()) - // We can only reason about scalar types. + // FIXME: The allowance for Obj-C pointers and block pointers is historical + // and likely a mistake. + if (!ArgType->isIntegralOrEnumerationType() && !ArgType->isFloatingType() && + !ArgType->isObjCObjectPointerType() && !ArgType->isBlockPointerType()) + // Per the GCC documentation, only numeric constants are recognized after + // inlining. + return RValue::get(ConstantInt::get(ResultType, 0)); + + if (Arg->HasSideEffects(getContext())) + // The argument is unevaluated, so be conservative if it might have + // side-effects. return RValue::get(ConstantInt::get(ResultType, 0)); Value *ArgValue = EmitScalarExpr(Arg); diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index de061e62aa..4c2d1dc768 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -1199,10 +1199,14 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID, if (checkArgCount(*this, TheCall, 1)) return true; TheCall->setType(Context.IntTy); break; - case Builtin::BI__builtin_constant_p: + case Builtin::BI__builtin_constant_p: { if (checkArgCount(*this, TheCall, 1)) return true; + ExprResult Arg = DefaultFunctionArrayLvalueConversion(TheCall->getArg(0)); + if (Arg.isInvalid()) return true; + TheCall->setArg(0, Arg.get()); TheCall->setType(Context.IntTy); break; + } case Builtin::BI__builtin_launder: return SemaBuiltinLaunder(*this, TheCall); case Builtin::BI__sync_fetch_and_add: diff --git a/test/CodeGen/builtin-constant-p.c b/test/CodeGen/builtin-constant-p.c index f1cd06ad4a..36c45988d6 100644 --- a/test/CodeGen/builtin-constant-p.c +++ b/test/CodeGen/builtin-constant-p.c @@ -177,3 +177,11 @@ void test17() { // CHECK: call void asm sideeffect "", {{.*}}(i32 -1) __asm__ __volatile__("" :: "n"( (__builtin_constant_p(test17_v) || 0) ? 1 : -1)); } + +int test18_f(); +// CHECK: define void @test18 +// CHECK-NOT: call {{.*}}test18_f +void test18() { + int a, b; + (void)__builtin_constant_p((a = b, test18_f())); +} diff --git a/test/SemaCXX/builtin-constant-p.cpp b/test/SemaCXX/builtin-constant-p.cpp new file mode 100644 index 0000000000..252c7bbac5 --- /dev/null +++ b/test/SemaCXX/builtin-constant-p.cpp @@ -0,0 +1,61 @@ +// RUN: %clang_cc1 -std=c++17 -verify %s + +using intptr_t = __INTPTR_TYPE__; + +// Test interaction of constexpr and __builtin_constant_p. + +template constexpr bool bcp(T t) { + return __builtin_constant_p(t); +} +template constexpr bool bcp_fold(T t) { + return __builtin_constant_p(((void)(intptr_t)&t, t)); +} + +constexpr intptr_t ensure_fold_is_generally_not_enabled = // expected-error {{constant expression}} + (intptr_t)&ensure_fold_is_generally_not_enabled; // expected-note {{cast}} + +constexpr intptr_t ptr_to_int(const void *p) { + return __builtin_constant_p(1) ? (intptr_t)p : (intptr_t)p; +} + +constexpr int *int_to_ptr(intptr_t n) { + return __builtin_constant_p(1) ? (int*)n : (int*)n; +} + +int x; + +// Integer and floating point constants encountered during constant expression +// evaluation are considered constant. So is nullptr_t. +static_assert(bcp(1)); +static_assert(bcp_fold(1)); +static_assert(bcp(1.0)); +static_assert(bcp_fold(1.0)); +static_assert(bcp(nullptr)); +static_assert(bcp_fold(nullptr)); + +// Pointers to the start of strings are considered constant. +static_assert(bcp("foo")); +static_assert(bcp_fold("foo")); + +// Null pointers are considered constant. +static_assert(bcp(nullptr)); +static_assert(bcp_fold(nullptr)); +static_assert(bcp(nullptr)); +static_assert(bcp_fold(nullptr)); + +// Other pointers are not. +static_assert(!bcp(&x)); +static_assert(!bcp_fold(&x)); + +// Pointers cast to integers follow the rules for pointers. +static_assert(bcp(ptr_to_int("foo"))); +static_assert(bcp_fold(ptr_to_int("foo"))); +static_assert(!bcp(ptr_to_int(&x))); +static_assert(!bcp_fold(ptr_to_int(&x))); + +// Integers cast to pointers follow the integer rules. +static_assert(bcp(int_to_ptr(0))); +static_assert(bcp_fold(int_to_ptr(0))); +static_assert(bcp(int_to_ptr(123))); // GCC rejects these due to not recognizing +static_assert(bcp_fold(int_to_ptr(123))); // the bcp conditional in 'int_to_ptr' ... +static_assert(__builtin_constant_p((int*)123)); // ... but GCC accepts this diff --git a/test/SemaCXX/enable_if.cpp b/test/SemaCXX/enable_if.cpp index 4bc974dafc..fd1375136a 100644 --- a/test/SemaCXX/enable_if.cpp +++ b/test/SemaCXX/enable_if.cpp @@ -522,3 +522,14 @@ void test() { InConstantContext::foo("abc"); } } // namespace InConstantContext + +namespace StringLiteralDetector { + void need_string_literal(const char *p) __attribute__((enable_if(__builtin_constant_p(p), "argument is not a string literal"))); // expected-note 2{{not a string literal}} + void test(const char *unknown) { + need_string_literal("foo"); + need_string_literal(unknown); // expected-error {{no matching function}} + constexpr char str[] = "bar"; + need_string_literal(str); // expected-error {{no matching function}} + } +} +