This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Don't make all MEM_REFs with TREE_READONLY arguments TREE_READONLY (PR sanitizer/64336)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Richard Biener <rguenther at suse dot de>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Tue, 6 Jan 2015 15:03:11 +0100
- Subject: Re: [PATCH] Don't make all MEM_REFs with TREE_READONLY arguments TREE_READONLY (PR sanitizer/64336)
- Authentication-results: sourceware.org; auth=none
- References: <20141217131815 dot GX1667 at tucnak dot redhat dot com> <alpine dot LSU dot 2 dot 11 dot 1412171520250 dot 25296 at zhemvz dot fhfr dot qr>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
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 <jakub@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