On 10/1/20 1:08 PM, Marek Polacek wrote:
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.
Does this approach seem sensible?
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
gcc/cp/ChangeLog:
PR c++/97230
* constexpr.c (eval_and_check_array_index): Forward declare.
(cxx_eval_component_reference): Likewise.
(cxx_eval_call_expression): Verify the 'this' pointer for
non-static member functions.
gcc/testsuite/ChangeLog:
PR c++/97230
* g++.dg/cpp0x/constexpr-member-fn1.C: New test.
---
gcc/cp/constexpr.c | 72 ++++++++++++++++++-
.../g++.dg/cpp0x/constexpr-member-fn1.C | 44 ++++++++++++
2 files changed, 115 insertions(+), 1 deletion(-)
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..f62f37ce384 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -2181,6 +2181,11 @@ cxx_eval_thunk_call (const constexpr_ctx *ctx, tree t,
tree thunk_fndecl,
non_constant_p, overflow_p);
}
+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 *);
+
/* Subroutine of cxx_eval_constant_expression.
Evaluate the call expression tree T in the context of OLD_CALL expression
evaluation. */
@@ -2467,6 +2472,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree
t,
if (*non_constant_p)
return t;
+ tree result = NULL_TREE;
depth_ok = push_cx_call_context (t);
/* Remember the object we are constructing. */
@@ -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.
+ {
+ 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?
+ 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.
And let's call it from cxx_bind_parameters_in_call.
constexpr_call *entry = NULL;
if (depth_ok && !non_constant_args && ctx->strict)
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: 04b99da898a9817e72fedb4063589648b7961ac5
