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;

Reply via email to