On 6/21/19 6:28 AM, Richard Biener wrote: > On Fri, Jun 21, 2019 at 12:24 AM Jeff Law <l...@redhat.com> wrote: >> >> As outlined in the BZ, our alias analysis code is context insensitive. >> So when we copy-propagate pointers, we can can and do copy PTA >> information from members to the representative pointer in the copy-of >> chain (we do this when the representative pointer has no associated PTA >> information). >> >> However, [E]VRP can set the non-nullness of a pointer using context >> sensitive information. So we have to be more careful when copying PTA >> information. >> >> We already have similar issues with alignment information as well. This >> patch just extends the hack to avoid copying alignment information in >> some circumstances to also avoid copying the non-nullness property. >> >> Bootstrapped and regression tested on x86_64-linux-gnu. OK for the trunk? > > I think this should be done in reset_flow_sensitive_info () as well > and tree-ssa-copy.c should use that function instead of calling > mark_ptr_info_alignment_unknown. > > I'd probably avoid the new clear_ptr_nonnull function since it might make > people miss the other half (alignment). > > Otherwise looks OK. Given that reset_flow_sensitive_info is used in a variety of other places, the requested changes seemed well past the line that required another bootstrap & regression test, which went fine.
Attached is what I actually installed. I'll backport to gcc-9 after a few days. I'll need to look at a gcc-8 to see if it's potentially needed there as well. jeff
* gcc.c-torture/execute/pr90949.c: New test. * tree-ssa-copy.c (fini_copy_prop): Use reset_flow_sensitive_info. * tree-ssanames.c (reset_flow_sensitive_info): Reset non-null state. diff --git a/gcc/testsuite/gcc.c-torture/execute/pr90949.c b/gcc/testsuite/gcc.c-torture/execute/pr90949.c new file mode 100644 index 00000000000..8c2ae3972d4 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr90949.c @@ -0,0 +1,42 @@ +void __attribute__ ((noipa, noinline)) my_puts (const char *str) { } + +void __attribute__ ((noipa, noinline)) my_free (void *p) { } + + +struct Node +{ + struct Node *child; +}; + +struct Node space[2] = { }; + +struct Node * __attribute__ ((noipa, noinline)) my_malloc (int bytes) +{ + return &space[0]; +} + +void +walk (struct Node *module, int cleanup) +{ + if (module == 0) + { + return; + } + if (!cleanup) + { + my_puts ("No cleanup"); + } + walk (module->child, cleanup); + if (cleanup) + { + my_free (module); + } +} + +int +main () +{ + struct Node *node = my_malloc (sizeof (struct Node)); + node->child = 0; + walk (node, 1); +} diff --git a/gcc/tree-ssa-copy.c b/gcc/tree-ssa-copy.c index 89532633e42..28ff8d3fbe2 100644 --- a/gcc/tree-ssa-copy.c +++ b/gcc/tree-ssa-copy.c @@ -545,13 +545,12 @@ fini_copy_prop (void) duplicate_ssa_name_ptr_info (copy_of[i].value, SSA_NAME_PTR_INFO (var)); /* Points-to information is cfg insensitive, - but alignment info might be cfg sensitive, if it - e.g. is derived from VRP derived non-zero bits. - So, do not copy alignment info if the two SSA_NAMEs - aren't defined in the same basic block. */ + but [E]VRP might record context sensitive alignment + info, non-nullness, etc. So reset context sensitive + info if the two SSA_NAMEs aren't defined in the same + basic block. */ if (var_bb != copy_of_bb) - mark_ptr_info_alignment_unknown - (SSA_NAME_PTR_INFO (copy_of[i].value)); + reset_flow_sensitive_info (copy_of[i].value); } else if (!POINTER_TYPE_P (TREE_TYPE (var)) && SSA_NAME_RANGE_INFO (var) diff --git a/gcc/tree-ssanames.c b/gcc/tree-ssanames.c index 5bac799e9a3..8b80bce8945 100644 --- a/gcc/tree-ssanames.c +++ b/gcc/tree-ssanames.c @@ -820,7 +820,12 @@ reset_flow_sensitive_info (tree name) { /* points-to info is not flow-sensitive. */ if (SSA_NAME_PTR_INFO (name)) - mark_ptr_info_alignment_unknown (SSA_NAME_PTR_INFO (name)); + { + /* [E]VRP can derive context sensitive alignment info and + non-nullness properties. We must reset both. */ + mark_ptr_info_alignment_unknown (SSA_NAME_PTR_INFO (name)); + SSA_NAME_PTR_INFO (name)->pt.null = 1; + } } else SSA_NAME_RANGE_INFO (name) = NULL;