On Tue, Oct 06, 2020 at 01:06:37AM -0400, Jason Merrill via Gcc-patches wrote:
> On 10/1/20 1:08 PM, Marek Polacek wrote:
> > @@ -2496,8 +2502,72 @@ cxx_eval_call_expression (const constexpr_ctx *ctx,
> > tree t,
> > new_obj = NULL_TREE;
> > }
> > }
> > + /* Verify that the object we're invoking the function on is sane. */
> > + else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (fun)
> > + /* maybe_add_lambda_conv_op creates a null 'this' pointer. */
> > + && !LAMBDA_TYPE_P (CP_DECL_CONTEXT (fun)))
>
> Let's look at lambda_static_thunk_p of ctx->call->fundef->decl instead; we
> don't want to allow calling a lambda op() with a null object from other
> contexts.
OK, fixed.
> > + {
> > + tree thisarg = TREE_VEC_ELT (new_call.bindings, 0);
> > + if (integer_zerop (thisarg))
> > + {
> > + if (!ctx->quiet)
> > + error_at (loc, "member call on null pointer is not allowed "
> > + "in a constant expression");
> > + *non_constant_p = true;
> > + result = error_mark_node;
> > + }
> > + else
> > + {
> > + STRIP_NOPS (thisarg);
> > + if (TREE_CODE (thisarg) == ADDR_EXPR)
> > + thisarg = TREE_OPERAND (thisarg, 0);
> > + /* Detect out-of-bounds accesses. */
> > + if (TREE_CODE (thisarg) == ARRAY_REF)
> > + {
> > + eval_and_check_array_index (ctx, thisarg, /*allow_one_past*/false,
> > + non_constant_p, overflow_p);
> > + if (*non_constant_p)
> > + result = error_mark_node;
> > + }
> > + /* Detect using an inactive member of a union. */
> > + else if (TREE_CODE (thisarg) == COMPONENT_REF)
> > + {
> > + cxx_eval_component_reference (ctx, thisarg, /*lval*/false,
> > + non_constant_p, overflow_p);
> > + if (*non_constant_p)
> > + result = error_mark_node;
> > + }
> > + /* Detect other invalid accesses like
> > - tree result = NULL_TREE;
> > + X x;
> > + (&x)[1].foo();
> > +
> > + where we'll end up with &x p+ 1. */
>
> Is there any way to combine this POINTER_PLUS_EXPR handling with
> cxx_fold_pointer_plus_expression or cxx_fold_indirect_ref?
cxx_fold_pointer_plus_expression looked like a place where the new checking
could go but in the end it didn't work. E.g., it STRIP_NOPS the lhs of the
POINTER_PLUS_EXPR so any casts would get lost.
> > + else if (TREE_CODE (thisarg) == POINTER_PLUS_EXPR)
> > + {
> > + tree op0 = TREE_OPERAND (thisarg, 0);
> > + /* This shouldn't trigger if we're accessing a base class of
> > + the object in question. */
> > + if (TREE_CODE (op0) == ADDR_EXPR
> > + && DECL_P (TREE_OPERAND (op0, 0)))
> > + {
> > + if (!ctx->quiet)
> > + {
> > + tree op1 = TREE_OPERAND (thisarg, 1);
> > + if (integer_onep (op1))
> > + error_at (loc, "cannot dereference one-past-the-end "
> > + "pointer in a constant expression");
> > + else
> > + error_at (loc, "cannot access element %qE of a "
> > + "non-array object in a constant expression",
> > + op1);
> > + }
> > + *non_constant_p = true;
> > + result = error_mark_node;
> > + }
> > + }
> > + }
> > + }
>
> These checks seem like the requirement that "A reference shall be
> initialized to refer to a valid object or function.", which matches with the
> change to describe the object argument as a reference rather than a pointer.
> Let's split this out into a separate function that expresses this, and
> perhaps then also use it to check the initializer of a reference variable.
I've moved it to a new function called cxx_verify_object_argument.
But checking the initializers of reference variables is still not handled
in this patch, because I don't know how to do that. For something like
constexpr const int &r1 = (&x)[1].m;
store_init_value calls cxx_constant_init for the init, which is
(const int &) &(&x + 4)->m
It could also be something like
(const int &) &arr[3].m
Maybe cxx_eval_constant_expression/NOP_EXPR should set some flag so that
we perform some checking when we encounter a POINTER_PLUS_EXPR or an
ARRAY_REF when evaluating op0 of the NOP_EXPR?
I've attached constexpr-ref13.C which is a testcase I played with; we
diagnose no errors there.
> And let's call it from cxx_bind_parameters_in_call.
Done.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
This PR points out that when we're invoking a non-static member function
on a null instance during constant evaluation, we should reject.
cxx_eval_call_expression calls cxx_bind_parameters_in_call which
evaluates function arguments, but it won't detect problems like these.
Well, ok, so use integer_zerop to detect a null 'this'. This also
detects member calls on a variable whose lifetime has ended, because
check_return_expr creates an artificial nullptr:
10195 else if (!processing_template_decl
10196 && maybe_warn_about_returning_address_of_local (retval,
loc)
10197 && INDIRECT_TYPE_P (valtype))
10198 retval = build2 (COMPOUND_EXPR, TREE_TYPE (retval), retval,
10199 build_zero_cst (TREE_TYPE (retval)));
It would be great if we could somehow distinguish between those two
cases, but experiments with setting TREE_THIS_VOLATILE on the zero
didn't work, so I left it be.
But by the same token, we should detect out-of-bounds accesses. For
this I'm (ab)using eval_and_check_array_index so that I don't have
to reimplement bounds checking yet again. But this only works for
ARRAY_REFs, so won't detect
X x;
(&x)[0].foo(); // ok
(&x)[1].foo(); // bad
so I've added a special handling of POINTER_PLUS_EXPRs.
While here, we should also detect using an inactive union member. For
that, I'm using cxx_eval_component_reference.
gcc/cp/ChangeLog:
PR c++/97230
* constexpr.c (eval_and_check_array_index): Forward declare.
(cxx_eval_component_reference): Likewise.
(cxx_verify_object_argument): New function.
(cxx_bind_parameters_in_call): Call it.
gcc/testsuite/ChangeLog:
PR c++/97230
* g++.dg/cpp0x/constexpr-member-fn1.C: New test.
---
gcc/cp/constexpr.c | 103 +++++++++++++++---
.../g++.dg/cpp0x/constexpr-member-fn1.C | 44 ++++++++
2 files changed, 134 insertions(+), 13 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-member-fn1.C
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index a118f8a810b..2155ed1ca19 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1548,6 +1548,69 @@ free_constructor (tree t)
}
}
+static tree eval_and_check_array_index (const constexpr_ctx *, tree, bool,
+ bool *, bool *);
+static tree cxx_eval_component_reference (const constexpr_ctx *, tree,
+ bool, bool *, bool *);
+
+/* Verify that the object THISARG we're invoking the function on is sane. */
+
+static void
+cxx_verify_object_argument (const constexpr_ctx *ctx, tree thisarg,
+ bool *non_constant_p, bool *overflow_p)
+{
+ location_t loc = cp_expr_loc_or_input_loc (thisarg);
+ if (integer_zerop (thisarg))
+ {
+ if (!ctx->quiet)
+ error_at (loc, "member call on null pointer is not allowed "
+ "in a constant expression");
+ *non_constant_p = true;
+ }
+ else
+ {
+ STRIP_NOPS (thisarg);
+ if (TREE_CODE (thisarg) == ADDR_EXPR)
+ thisarg = TREE_OPERAND (thisarg, 0);
+ /* Detect out-of-bounds accesses. */
+ if (TREE_CODE (thisarg) == ARRAY_REF)
+ eval_and_check_array_index (ctx, thisarg, /*allow_one_past*/false,
+ non_constant_p, overflow_p);
+ /* Detect using an inactive member of a union. */
+ else if (TREE_CODE (thisarg) == COMPONENT_REF)
+ cxx_eval_component_reference (ctx, thisarg, /*lval*/false,
+ non_constant_p, overflow_p);
+ /* Detect other invalid accesses like
+
+ X x;
+ (&x)[1].foo();
+
+ where we'll end up with &x p+ 1. */
+ else if (TREE_CODE (thisarg) == POINTER_PLUS_EXPR)
+ {
+ tree op0 = TREE_OPERAND (thisarg, 0);
+ /* This shouldn't trigger if we're accessing a base class of
+ the object in question. */
+ if (TREE_CODE (op0) == ADDR_EXPR
+ && DECL_P (TREE_OPERAND (op0, 0)))
+ {
+ if (!ctx->quiet)
+ {
+ tree op1 = TREE_OPERAND (thisarg, 1);
+ if (integer_onep (op1))
+ error_at (loc, "cannot dereference one-past-the-end "
+ "pointer in a constant expression");
+ else
+ error_at (loc, "cannot access element %qE of a "
+ "non-array object in a constant expression",
+ op1);
+ }
+ *non_constant_p = true;
+ }
+ }
+ }
+}
+
/* Subroutine of cxx_eval_call_expression.
We are processing a call expression (either CALL_EXPR or
AGGR_INIT_EXPR) in the context of CTX. Evaluate
@@ -1605,21 +1668,35 @@ cxx_bind_parameters_in_call (const constexpr_ctx *ctx,
tree t,
/* For virtual calls, adjust the this argument, so that it is
the object on which the method is called, rather than
one of its bases. */
- if (i == 0 && DECL_VIRTUAL_P (fun))
+ if (i == 0)
{
- tree addr = arg;
- STRIP_NOPS (addr);
- if (TREE_CODE (addr) == ADDR_EXPR)
+ if (DECL_VIRTUAL_P (fun))
+ {
+ tree addr = arg;
+ STRIP_NOPS (addr);
+ if (TREE_CODE (addr) == ADDR_EXPR)
+ {
+ tree obj = TREE_OPERAND (addr, 0);
+ while (TREE_CODE (obj) == COMPONENT_REF
+ && DECL_FIELD_IS_BASE (TREE_OPERAND (obj, 1))
+ && !same_type_ignoring_top_level_qualifiers_p
+ (TREE_TYPE (obj), DECL_CONTEXT (fun)))
+ obj = TREE_OPERAND (obj, 0);
+ if (obj != TREE_OPERAND (addr, 0))
+ arg = build_fold_addr_expr_with_type (obj,
+ TREE_TYPE (arg));
+ }
+ }
+ else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (fun)
+ && !DECL_CONSTRUCTOR_P (fun)
+ && !(ctx->call
+ && ctx->call->fundef
+ && lambda_static_thunk_p (ctx->call->fundef->decl)))
{
- tree obj = TREE_OPERAND (addr, 0);
- while (TREE_CODE (obj) == COMPONENT_REF
- && DECL_FIELD_IS_BASE (TREE_OPERAND (obj, 1))
- && !same_type_ignoring_top_level_qualifiers_p
- (TREE_TYPE (obj), DECL_CONTEXT (fun)))
- obj = TREE_OPERAND (obj, 0);
- if (obj != TREE_OPERAND (addr, 0))
- arg = build_fold_addr_expr_with_type (obj,
- TREE_TYPE (arg));
+ cxx_verify_object_argument (ctx, arg, non_constant_p,
+ overflow_p);
+ if (*non_constant_p && ctx->quiet)
+ return;
}
}
TREE_VEC_ELT (binds, i) = arg;
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-member-fn1.C
b/gcc/testsuite/g++.dg/cpp0x/constexpr-member-fn1.C
new file mode 100644
index 00000000000..8ba5b87286f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-member-fn1.C
@@ -0,0 +1,44 @@
+// PR c++/97230
+// { dg-do compile { target c++11 } }
+
+struct X {
+ constexpr int foo () const { return 1; }
+};
+
+constexpr X *eval () { return nullptr; }
+constexpr const X *dead (X tmp) { return &tmp; } // { dg-warning "address of
local variable" }
+
+static constexpr X x;
+static constexpr X arr[3];
+
+constexpr const X *gimme () { return &x; }
+
+union U {
+ int i;
+ X x;
+};
+constexpr U u{42};
+
+void
+fn ()
+{
+ constexpr auto x1 = ((X *) nullptr)->foo(); // { dg-error "member call on
null pointer" }
+ constexpr auto x2 = (eval())->foo(); // { dg-error "member call on null
pointer" }
+ constexpr auto x3 = dead ({})->foo(); // { dg-error "member call" }
+ constexpr auto x4 = (&x)[0].foo();
+ constexpr auto x5 = (&x)[1].foo(); // { dg-error "cannot dereference
one-past-the-end pointer" }
+ constexpr auto x6 = (&x)[2].foo(); // { dg-error "cannot access element .2.
of a non-array object" }
+ constexpr auto x7 = (&x)[3].foo(); // { dg-error "cannot access element .3.
of a non-array object" }
+ constexpr auto x8 = arr[2].foo();
+ constexpr auto x9 = arr[3].foo(); // { dg-error "outside the bounds" }
+ constexpr auto *p = &arr[0];
+ constexpr auto x10 = (*p).foo();
+ constexpr auto x11 = p->foo();
+ constexpr auto x12 = (*(p + 1)).foo();
+ constexpr auto x13 = (*(p + 3)).foo(); // { dg-error "outside the bounds" }
+ constexpr auto x14 = (p + 3)->foo(); // { dg-error "outside the bounds" }
+ constexpr auto x15 = gimme()->foo();
+ constexpr auto x16 = (gimme() + 1)->foo(); // { dg-error "cannot dereference
one-past-the-end pointer" }
+ constexpr auto x17 = (gimme() + 2)->foo(); // { dg-error "cannot access
element .2. of a non-array object" }
+ constexpr auto x18 = u.x.foo(); // { dg-error "accessing .U::x. member
instead of initialized .U::i. member" }
+}
base-commit: 1e247c60df52e93c9814a3a1789a63bc07aa4542
--
2.26.2
// PR c++/97230
// { dg-do compile { target c++11 } }
struct X { int m; };
constexpr X x{};
constexpr X arr[3]{};
constexpr const X *gimme () { return &x; }
constexpr const int &r0 = (&x)[0].m;
constexpr const int &r1 = (&x)[1].m; // { dg-error "" }
constexpr const int &r2 = (&x)[2].m; // { dg-error "" }
constexpr const int &r3 = gimme()->m;
constexpr const int &r4 = (gimme() + 1)->m; // { dg-error "" }
constexpr const int &r5 = (gimme() + 2)->m; // { dg-error "" }
constexpr const int &r6 = arr[2].m;
constexpr const int &r7 = arr[3].m; // { dg-error "" }
constexpr auto *p = &arr[0];
constexpr const int &r8 = (*p).m;
constexpr const int &r9 = p->m;
constexpr const int &r10 = (p + 2)->m;
constexpr const int &r11 = (p + 3)->m; // { dg-error "" }
