On Wed, Dec 17, 2014 at 03:28:15PM +0100, Richard Biener wrote:
> On Wed, 17 Dec 2014, Jakub Jelinek wrote:
> 
> > Hi!
> > 
> > MEM_REF (the only tcc_reference code with 2 operands) has TREE_READONLY set
> > whenever all the arguments are TREE_READONLY, which is wrong, if the
> > pointer/reference is read-only, it doesn't say anything about whether
> > what it points to is also read-only.
> > 
> > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> > trunk?
> 
> Thinking about this some more I think that we should instead do sth like
> 
>   read_only = 1;
>   side_effects = TREE_SIDE_EFFECTS (t);
> 
>   if (code == MEM_REF)
>    {
>      if (TREE_CODE (arg0) == ADDR_EXPR)
>        {
>          tree t = arg0;
>          PROCESS_ARG (0);
>        }
>      else
>        read_only = 0;

This doesn't work, PROCESS_ARG among other things assigns the arguments,
computing the flags is just one thing.  So the above would crash if arg0 is
NULL (also happens), and if it is ADDR_EXPR would attempt to assign
TREE_OPERANDS (arg0, 0) = arg0; and otherwise leave the argument NULL.

Here is a new version of the patch, the flags should be preinitialized to
0 by make_node_stat, so can be assigned for *MEM_REFs only if arg0 is
ADDR_EXPR, 0 is right otherwise for both flags.  In generic code MEM_REF
is the only 2 operand tcc_reference, but C++ FE has some further ones,
so I need to keep the old TREE_THIS_VOLATILE assignment.  There are no
5 operand tcc_references beyond TARGET_MEM_REF, but I'd keep it this way
as is anyway.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2015-01-06  Jakub Jelinek  <ja...@redhat.com>

        PR sanitizer/64336
        * tree.c (build2_stat): Fix up initialization of TREE_READONLY
        and TREE_THIS_VOLATILE for MEM_REFs.
        (build5_stat): Fix up initialization of TREE_READONLY and
        TREE_THIS_VOLATILE for TARGET_MEM_REFs.

--- gcc/tree.c.jj       2015-01-05 13:07:15.000000000 +0100
+++ gcc/tree.c  2015-01-06 12:33:46.466016025 +0100
@@ -4358,12 +4358,24 @@ build2_stat (enum tree_code code, tree t
   PROCESS_ARG (0);
   PROCESS_ARG (1);
 
-  TREE_READONLY (t) = read_only;
-  TREE_CONSTANT (t) = constant;
   TREE_SIDE_EFFECTS (t) = side_effects;
-  TREE_THIS_VOLATILE (t)
-    = (TREE_CODE_CLASS (code) == tcc_reference
-       && arg0 && TREE_THIS_VOLATILE (arg0));
+  if (code == MEM_REF)
+    {
+      if (arg0 && TREE_CODE (arg0) == ADDR_EXPR)
+       {
+         tree o = TREE_OPERAND (arg0, 0);
+         TREE_READONLY (t) = TREE_READONLY (o);
+         TREE_THIS_VOLATILE (t) = TREE_THIS_VOLATILE (o);
+       }
+    }
+  else
+    {
+      TREE_READONLY (t) = read_only;
+      TREE_CONSTANT (t) = constant;
+      TREE_THIS_VOLATILE (t)
+       = (TREE_CODE_CLASS (code) == tcc_reference
+          && arg0 && TREE_THIS_VOLATILE (arg0));
+    }
 
   return t;
 }
@@ -4458,9 +4470,19 @@ build5_stat (enum tree_code code, tree t
   PROCESS_ARG (4);
 
   TREE_SIDE_EFFECTS (t) = side_effects;
-  TREE_THIS_VOLATILE (t)
-    = (TREE_CODE_CLASS (code) == tcc_reference
-       && arg0 && TREE_THIS_VOLATILE (arg0));
+  if (code == TARGET_MEM_REF)
+    {
+      if (arg0 && TREE_CODE (arg0) == ADDR_EXPR)
+       {
+         tree o = TREE_OPERAND (arg0, 0);
+         TREE_READONLY (t) = TREE_READONLY (o);
+         TREE_THIS_VOLATILE (t) = TREE_THIS_VOLATILE (o);
+       }
+    }
+  else
+    TREE_THIS_VOLATILE (t)
+      = (TREE_CODE_CLASS (code) == tcc_reference
+        && arg0 && TREE_THIS_VOLATILE (arg0));
 
   return t;
 }


        Jakub

Reply via email to