Skip to content

Commit

Permalink
c++: Track lifetimes in constant evaluation [PR70331, PR96630, PR98675]
Browse files Browse the repository at this point in the history
This adds rudimentary lifetime tracking in C++ constexpr contexts,
allowing the compiler to report errors with using values after their
backing has gone out of scope. We don't yet handle other ways of ending
lifetimes (e.g. explicit destructor calls).

	PR c++/70331
	PR c++/96630
	PR c++/98675

gcc/cp/ChangeLog:

	* constexpr.cc (constexpr_global_ctx::put_value): Mark value as
        in lifetime.
        (constexpr_global_ctx::remove_value): Mark value as expired.
        (cxx_eval_call_expression): Remove comment that is no longer
        applicable.
	(non_const_var_error): Add check for expired values.
	(cxx_eval_constant_expression): Add checks for expired values.
        Forget local variables at end of bind expressions. Forget
        temporaries at end of cleanup points.
	* module.cc (trees_out::core_bools): Write out the new flag.
	(trees_in::core_bools): Read in the new flag.

gcc/ChangeLog:

	* tree-core.h (struct tree_decl_common): New flag to check if
        value lifetime has expired.
	* tree.h (DECL_EXPIRED): Access the new flag.
	* print-tree.cc (print_node): Print the new flag.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/constexpr-ice20.C: Update error raised by test.
	* g++.dg/cpp1y/constexpr-lifetime1.C: New test.
	* g++.dg/cpp1y/constexpr-lifetime2.C: New test.
	* g++.dg/cpp1y/constexpr-lifetime3.C: New test.
	* g++.dg/cpp1y/constexpr-lifetime4.C: New test.
	* g++.dg/cpp1y/constexpr-lifetime5.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
  • Loading branch information
wreien authored and ouuleilei-bot committed Mar 28, 2023
1 parent fcb4115 commit a072cdd
Show file tree
Hide file tree
Showing 11 changed files with 138 additions and 15 deletions.
66 changes: 53 additions & 13 deletions gcc/cp/constexpr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1185,10 +1185,17 @@ class constexpr_global_ctx {
void put_value (tree t, tree v)
{
bool already_in_map = values.put (t, v);
if (!already_in_map && DECL_P (t))
DECL_EXPIRED (t) = false;
if (!already_in_map && modifiable)
modifiable->add (t);
}
void remove_value (tree t) { values.remove (t); }
void remove_value (tree t)
{
if (DECL_P (t))
DECL_EXPIRED (t) = true;
values.remove (t);
}
};

/* Helper class for constexpr_global_ctx. In some cases we want to avoid
Expand Down Expand Up @@ -3157,10 +3164,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
for (tree save_expr : save_exprs)
ctx->global->remove_value (save_expr);

/* Remove the parms/result from the values map. Is it worth
bothering to do this when the map itself is only live for
one constexpr evaluation? If so, maybe also clear out
other vars from call, maybe in BIND_EXPR handling? */
/* Remove the parms/result from the values map. */
ctx->global->remove_value (res);
for (tree parm = parms; parm; parm = TREE_CHAIN (parm))
ctx->global->remove_value (parm);
Expand Down Expand Up @@ -5708,6 +5712,13 @@ non_const_var_error (location_t loc, tree r, bool fundef_p)
inform (DECL_SOURCE_LOCATION (r), "allocated here");
return;
}
if (DECL_EXPIRED (r))
{
if (constexpr_error (loc, fundef_p, "accessing object outside its "
"lifetime"))
inform (DECL_SOURCE_LOCATION (r), "declared here");
return;
}
if (!constexpr_error (loc, fundef_p, "the value of %qD is not usable in "
"a constant expression", r))
return;
Expand Down Expand Up @@ -6995,7 +7006,7 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
else
{
if (!ctx->quiet)
error ("%qE is not a constant expression", t);
non_const_var_error (loc, t, /*fundef_p*/false);
*non_constant_p = true;
}
break;
Expand Down Expand Up @@ -7048,6 +7059,13 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
r = build_constructor (TREE_TYPE (t), NULL);
TREE_CONSTANT (r) = true;
}
else if (DECL_EXPIRED (t))
{
if (!ctx->quiet)
non_const_var_error (loc, r, /*fundef_p*/false);
*non_constant_p = true;
break;
}
else if (ctx->strict)
r = decl_really_constant_value (t, /*unshare_p=*/false);
else
Expand Down Expand Up @@ -7093,7 +7111,15 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
else
{
if (!ctx->quiet)
error ("%qE is not a constant expression", t);
{
if (DECL_EXPIRED (r))
{
error_at (loc, "accessing object outside its lifetime");
inform (DECL_SOURCE_LOCATION (r), "declared here");
}
else
error_at (loc, "%qE is not a constant expression", t);
}
*non_constant_p = true;
}
break;
Expand Down Expand Up @@ -7315,17 +7341,28 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
auto_vec<tree, 2> cleanups;
vec<tree> *prev_cleanups = ctx->global->cleanups;
ctx->global->cleanups = &cleanups;
r = cxx_eval_constant_expression (ctx, TREE_OPERAND (t, 0),

auto_vec<tree, 10> save_exprs;
constexpr_ctx new_ctx = *ctx;
new_ctx.save_exprs = &save_exprs;

r = cxx_eval_constant_expression (&new_ctx, TREE_OPERAND (t, 0),
lval,
non_constant_p, overflow_p,
jump_target);

ctx->global->cleanups = prev_cleanups;
unsigned int i;
tree cleanup;
/* Evaluate the cleanups. */
FOR_EACH_VEC_ELT_REVERSE (cleanups, i, cleanup)
cxx_eval_constant_expression (ctx, cleanup, vc_discard,
cxx_eval_constant_expression (&new_ctx, cleanup, vc_discard,
non_constant_p, overflow_p);

/* Forget SAVE_EXPRs and TARGET_EXPRs created by this
full-expression. */
for (tree save_expr : save_exprs)
ctx->global->remove_value (save_expr);
}
break;

Expand Down Expand Up @@ -7831,10 +7868,13 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
non_constant_p, overflow_p, jump_target);

case BIND_EXPR:
return cxx_eval_constant_expression (ctx, BIND_EXPR_BODY (t),
lval,
non_constant_p, overflow_p,
jump_target);
r = cxx_eval_constant_expression (ctx, BIND_EXPR_BODY (t),
lval,
non_constant_p, overflow_p,
jump_target);
for (tree decl = BIND_EXPR_VARS (t); decl; decl = DECL_CHAIN (decl))
ctx->global->remove_value (decl);
return r;

case PREINCREMENT_EXPR:
case POSTINCREMENT_EXPR:
Expand Down
2 changes: 2 additions & 0 deletions gcc/cp/module.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5435,6 +5435,7 @@ trees_out::core_bools (tree t)
WB (t->decl_common.decl_read_flag);
WB (t->decl_common.decl_nonshareable_flag);
WB (t->decl_common.decl_not_flexarray);
WB (t->decl_common.decl_expired_flag);
}

if (CODE_CONTAINS_STRUCT (code, TS_DECL_WITH_VIS))
Expand Down Expand Up @@ -5580,6 +5581,7 @@ trees_in::core_bools (tree t)
RB (t->decl_common.decl_read_flag);
RB (t->decl_common.decl_nonshareable_flag);
RB (t->decl_common.decl_not_flexarray);
RB (t->decl_common.decl_expired_flag);
}

if (CODE_CONTAINS_STRUCT (code, TS_DECL_WITH_VIS))
Expand Down
4 changes: 4 additions & 0 deletions gcc/print-tree.cc
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,10 @@ print_node (FILE *file, const char *prefix, tree node, int indent,
&& DECL_BY_REFERENCE (node))
fputs (" passed-by-reference", file);

if ((code == VAR_DECL || code == PARM_DECL || code == RESULT_DECL)
&& DECL_EXPIRED (node))
fputs (" expired", file);

if (CODE_CONTAINS_STRUCT (code, TS_DECL_WITH_VIS) && DECL_DEFER_OUTPUT (node))
fputs (" defer-output", file);

Expand Down
2 changes: 1 addition & 1 deletion gcc/testsuite/g++.dg/cpp0x/constexpr-ice20.C
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@
typedef bool (*Function)(int);
constexpr bool check(int x, Function p) { return p(x); } // { dg-message "in .constexpr. expansion of" }

static_assert(check(2, check), ""); // { dg-error "conversion|constant|in .constexpr. expansion of" }
static_assert(check(2, check), ""); // { dg-error "conversion|constant|lifetime|in .constexpr. expansion of" }
13 changes: 13 additions & 0 deletions gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// PR c++/96630
// { dg-do compile { target c++14 } }

struct S {
int x = 0;
constexpr const int& get() const { return x; }
};

constexpr const int& test() {
auto local = S{}; // { dg-message "note: declared here" }
return local.get();
}
constexpr int x = test(); // { dg-error "accessing object outside its lifetime" }
20 changes: 20 additions & 0 deletions gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// PR c++/98675
// { dg-do compile { target c++14 } }

struct S {
int x = 0;
constexpr const int& get() const { return x; }
};

constexpr int error() {
const auto& local = S{}.get(); // { dg-message "note: declared here" }
return local;
}
constexpr int x = error(); // { dg-error "accessing object outside its lifetime" }

constexpr int ok() {
// temporary should only be destroyed after end of full-expression
auto local = S{}.get();
return local;
}
constexpr int y = ok();
13 changes: 13 additions & 0 deletions gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// PR c++/70331
// { dg-do compile { target c++14 } }

constexpr int f(int i) {
int *p = &i;
if (i == 0) {
int j = 123; // { dg-message "note: declared here" }
p = &j;
}
return *p;
}

constexpr int i = f(0); // { dg-error "accessing object outside its lifetime" }
11 changes: 11 additions & 0 deletions gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// { dg-do compile { target c++14 } }

constexpr const double& test() {
const double& local = 3.0; // { dg-message "note: declared here" }
return local;
}

static_assert(test() == 3.0, ""); // { dg-error "constant|accessing object outside its lifetime" }

// no deference, shouldn't error
static_assert((test(), true), "");
11 changes: 11 additions & 0 deletions gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime5.C
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// { dg-do compile { target c++14 } }
// { dg-options "-Wno-return-local-addr" }

constexpr const int& id(int x) { return x; }

constexpr bool test() {
const int& y = id(3);
return y == 3;
}

constexpr bool x = test(); // { dg-error "" }
5 changes: 4 additions & 1 deletion gcc/tree-core.h
Original file line number Diff line number Diff line change
Expand Up @@ -1834,7 +1834,10 @@ struct GTY(()) tree_decl_common {
/* In FIELD_DECL, this is DECL_NOT_FLEXARRAY. */
unsigned int decl_not_flexarray : 1;

/* 13 bits unused. */
/* In VAR_DECL, PARM_DECL, or RESULT_DECL, this is DECL_EXPIRED. */
unsigned int decl_expired_flag : 1;

/* 12 bits unused. */

/* UID for points-to sets, stable over copying from inlining. */
unsigned int pt_uid;
Expand Down
6 changes: 6 additions & 0 deletions gcc/tree.h
Original file line number Diff line number Diff line change
Expand Up @@ -2697,6 +2697,12 @@ extern tree vector_element_bits_tree (const_tree);
??? Need to figure out some way to check this isn't a PARM_DECL. */
#define DECL_INITIAL(NODE) (DECL_COMMON_CHECK (NODE)->decl_common.initial)

/* Used in a VAR_DECL, PARM_DECL, or RESULT_DECL to indicate whether
this declaration is currently in lifetime for constant evaluation
purposes. */
#define DECL_EXPIRED(NODE) \
(DECL_COMMON_CHECK(NODE)->decl_common.decl_expired_flag)

/* Holds the size of the datum, in bits, as a tree expression.
Need not be constant and may be null. May be less than TYPE_SIZE
for a C++ FIELD_DECL representing a base class subobject with its
Expand Down

0 comments on commit a072cdd

Please sign in to comment.