On Jan 26, 2021, Richard Biener <richard.guent...@gmail.com> wrote:
> So while I think it's safe let's look at if we can improve tree-nested.c,
> like I see (probably not the correct place):
*nod*, it's just not the *only* place.
> seeing how we adjust current_function_decl around the
> recompute_tree_invariant_for_addr_expr call but not the
> gsi_gimplify_val one (we already pass it a nesting_info,
> not sure if wi->info is the same as the 'info' used above though),
> so eventually we can fix it in one place?
There are pieces of nested function lowering for which we set cfun and
current_function_decl while walking each function, and there are other
pieces that just don't bother, and we only set up current_function_decl
temporarily for ADDR_EXPR handling.
This patch adjusts both of the ADDR_EXPR handlers that override
current_function_decl, so that the temporary overriding remains in
effect during the re-gimplification. That is enough to avoid the
problem. But I'm not very happy with this temporary overriding, it
seems fishy. I'd rather we set things up for the entire duration of the
walking of each function.
But that's only relevant because we rely on current_function_decl for
address handling. It's not clear to me that we should, as the other
patch demonstrated. With it, we could probably even do away with these
overriders.
But, for this stage, this is probably as conservative a change as we
could possibly hope for. I've regstrapped it on x86_64-linux-gnu, and
also bootstrapped it with asan and ubsan. Ok to install?
restore current_function_decl after re-gimplifying nested ADDR_EXPRs
From: Alexandre Oliva <ol...@adacore.com>
Ada makes extensive use of nested functions, which turn all automatic
variables of the enclosing function that are used in nested ones into
members of an artificial FRAME record type.
The address of a local variable is usually passed to asan marking
functions without using a temporary. asan_expand_mark_ifn will reject
an ADDR_EXPRs if it's split out from the call into an SSA_NAMEs.
Taking the address of a member of FRAME within a nested function was
not regarded as a gimple val: while introducing FRAME variables,
current_function_decl pointed to the outermost function, even while
processing a nested function, so decl_address_invariant_p, checking
that the context of the variable is current_function_decl, returned
false for such ADDR_EXPRs.
decl_address_invariant_p, called when determining whether an
expression is a legitimate gimple value, compares the context of
automatic variables with current_function_decl. Some of the
tree-nested function processing doesn't set current_function_decl, but
ADDR_EXPR-processing bits temporarily override it. However, they
restore it before re-gimplifying, which causes even ADDR_EXPRs
referencing automatic variables in the FRAME struct of a nested
function to not be regarded as address-invariant.
This patch moves the restores of current_function_decl in the
ADDR_EXPR-handling bits after the re-gimplification, so that the
correct current_function_decl is used when testing for address
invariance.
for gcc/ChangeLog
* tree-nested.c (convert_nonlocal_reference_op): Move
current_function_decl restore after re-gimplification.
(convert_local_reference_op): Likewise.
for gcc/testsuite/ChangeLog
* gcc.dg/asan/nested-1.c: New.
---
gcc/testsuite/gcc.dg/asan/nested-1.c | 24 ++++++++++++++++++++++++
gcc/tree-nested.c | 4 ++--
2 files changed, 26 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/asan/nested-1.c
diff --git a/gcc/testsuite/gcc.dg/asan/nested-1.c
b/gcc/testsuite/gcc.dg/asan/nested-1.c
new file mode 100644
index 0000000000000..87e842098077c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asan/nested-1.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=address" } */
+
+int f(int i) {
+ auto int h() {
+ int r;
+ int *p;
+
+ {
+ int x[3];
+
+ auto int g() {
+ return x[i];
+ }
+
+ p = &r;
+ *p = g();
+ }
+
+ return *p;
+ }
+
+ return h();
+}
diff --git a/gcc/tree-nested.c b/gcc/tree-nested.c
index 1b52669b622aa..addd6eef9aba6 100644
--- a/gcc/tree-nested.c
+++ b/gcc/tree-nested.c
@@ -1214,7 +1214,6 @@ convert_nonlocal_reference_op (tree *tp, int
*walk_subtrees, void *data)
save_context = current_function_decl;
current_function_decl = info->context;
recompute_tree_invariant_for_addr_expr (t);
- current_function_decl = save_context;
/* If the callback converted the address argument in a context
where we only accept variables (and min_invariant, presumably),
@@ -1222,6 +1221,7 @@ convert_nonlocal_reference_op (tree *tp, int
*walk_subtrees, void *data)
if (save_val_only)
*tp = gsi_gimplify_val ((struct nesting_info *) wi->info,
t, &wi->gsi);
+ current_function_decl = save_context;
}
}
break;
@@ -1969,13 +1969,13 @@ convert_local_reference_op (tree *tp, int
*walk_subtrees, void *data)
save_context = current_function_decl;
current_function_decl = info->context;
recompute_tree_invariant_for_addr_expr (t);
- current_function_decl = save_context;
/* If we are in a context where we only accept values, then
compute the address into a temporary. */
if (save_val_only)
*tp = gsi_gimplify_val ((struct nesting_info *) wi->info,
t, &wi->gsi);
+ current_function_decl = save_context;
}
break;
--
Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/
Free Software Activist GNU Toolchain Engineer
Vim, Vi, Voltei pro Emacs -- GNUlius Caesar
