On Jan 22, 2021, Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> Yeah, I guess such addresses could be decl_address_invariant_p by changing
> the
> || DECL_CONTEXT (op) == current_function_decl
> || decl_function_context (op) == current_function_decl)
> to sth like
> || auto_var_p (op)
> but I guess it won't help much since the access in the nested function
> will be through the static chain pointer and thus &chain->member
> which definitely isn't a gimple val.
I think we only get poison/unpoison calls in the scope of the automatic
variables, so it won't be the case that these calls will get such
indirect frame references: they will only get references to the own
function's frame, and those have invariant addresses, and thus can be
regarded as such, and as gimple vals.
So I gave this alternate change a spin, and both regstrap and asan+ubsan
bootstrap completed successfully.
Given the considerations of risk about the assert you pointed out, I'm
now inclined to regard this change as safer and superior. Do you all
concur? Ok to install if so?
(I see the code in tree.c was untabified, and one of my changes
introduced a tab that misaligned stuff. Tabify, untabify, or leave it
inconsistent as in the tested patch below?
regard the address of auto vars and consts as invariant
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.
This patch changes decl_address_invariant_p to disregard
current_function_decl, and regard all automatic variables as having
invariant addresses, regardless of nesting. This may initially
include references to variables in other nesting levels, but once they
become references to enclosing frames, the indirection makes them
non-gimple_vals. As long as poisoning and unpoisoning calls doesn't
kick in for variables in other frames, this shouldn't be a problem.
for gcc/ChangeLog
* tree.c (decl_address_invariant_p): Accept auto variables and
constants.
for gcc/testsuite/ChangeLog
* gcc.dg/asan/nested-1.c: New.
---
gcc/testsuite/gcc.dg/asan/nested-1.c | 24 ++++++++++++++++++++++++
gcc/tree.c | 5 ++---
2 files changed, 26 insertions(+), 3 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.c b/gcc/tree.c
index 287e5001dc3b3..3de3085f42c8a 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -3590,14 +3590,13 @@ decl_address_invariant_p (const_tree op)
case VAR_DECL:
if ((TREE_STATIC (op) || DECL_EXTERNAL (op))
|| DECL_THREAD_LOCAL_P (op)
- || DECL_CONTEXT (op) == current_function_decl
- || decl_function_context (op) == current_function_decl)
+ || auto_var_p (op))
return true;
break;
case CONST_DECL:
if ((TREE_STATIC (op) || DECL_EXTERNAL (op))
- || decl_function_context (op) == current_function_decl)
+ || auto_var_p (op))
return true;
break;
--
Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/
Free Software Activist GNU Toolchain Engineer
Vim, Vi, Voltei pro Emacs -- GNUlius Caesar
