diff mbox

[fortran] Re: Make array_at_struct_end_p to grok MEM_REFs

Message ID 20160524122259.GC60007@kam.mff.cuni.cz
State New
Headers show

Commit Message

Jan Hubicka May 24, 2016, 12:23 p.m. UTC
> 
> Ah, yes.  Now I see.
> 
> >  The test I updated that looks for DECL simply assumes
> > that declarations can not be accessed past their end.
> > It would make more sense to use object size machinery here somehow.
> > (i.e. even in fortran we have accesses to mallocated buffers of constant size).
> > But this probably could be better handled at niter side where we can also deal with
> > case of real trailing arrays of known size.
> 
> But then I'm not sure that TYPE_SIZE (TREE_TYPE (ref)) == NULL is
> handled correctly.  I suppose you can hope for the array to be the
> one forcing it NULL and thus its TYPE_DOMAIN max val being NULL ...

Hmm, you are probably right. If we can have array with TYPE_DOMAIN != NULL
and sane bounds, but with TYPE_SIZE == NULL, we probably need to punt on NULL
TYPE_SIZE.  I can add it just to be sure.

I am testing

Comments

Richard Biener May 24, 2016, 12:37 p.m. UTC | #1
On Tue, 24 May 2016, Jan Hubicka wrote:

> > 
> > Ah, yes.  Now I see.
> > 
> > >  The test I updated that looks for DECL simply assumes
> > > that declarations can not be accessed past their end.
> > > It would make more sense to use object size machinery here somehow.
> > > (i.e. even in fortran we have accesses to mallocated buffers of constant size).
> > > But this probably could be better handled at niter side where we can also deal with
> > > case of real trailing arrays of known size.
> > 
> > But then I'm not sure that TYPE_SIZE (TREE_TYPE (ref)) == NULL is
> > handled correctly.  I suppose you can hope for the array to be the
> > one forcing it NULL and thus its TYPE_DOMAIN max val being NULL ...
> 
> Hmm, you are probably right. If we can have array with TYPE_DOMAIN != NULL
> and sane bounds, but with TYPE_SIZE == NULL, we probably need to punt on NULL
> TYPE_SIZE.  I can add it just to be sure.

As a MEM_REF embeds a VIEW_CONVERT you can placement-new

struct { int a[5]; char b[]; };

ontop of char X[24]; and access MEM_REF[&x].a[3] (not at struct end)
and MEM_REF[&x].b[4] but _both_ accesses would have TYPE_SIZE NULL.

So I'm not sure TYPE_SIZE tells you anything here...

Richard.

> I am testing
> 
> Index: tree.c
> ===================================================================
> --- tree.c	(revision 236557)
> +++ tree.c	(working copy)
> @@ -13079,7 +13079,8 @@ array_at_struct_end_p (tree ref)
>    tree size = NULL;
>  
>    if (TREE_CODE (ref) == MEM_REF
> -      && TREE_CODE (TREE_OPERAND (ref, 0)) == ADDR_EXPR)
> +      && TREE_CODE (TREE_OPERAND (ref, 0)) == ADDR_EXPR
> +      && TYPE_SIZE (TREE_TYPE (ref)))
>      {
>        size = TYPE_SIZE (TREE_TYPE (ref));
>        ref = TREE_OPERAND (TREE_OPERAND (ref, 0), 0);
> 
>
Jan Hubicka May 24, 2016, 12:43 p.m. UTC | #2
> > Hmm, you are probably right. If we can have array with TYPE_DOMAIN != NULL
> > and sane bounds, but with TYPE_SIZE == NULL, we probably need to punt on NULL
> > TYPE_SIZE.  I can add it just to be sure.
> 
> As a MEM_REF embeds a VIEW_CONVERT you can placement-new
> 
> struct { int a[5]; char b[]; };

Yep. This is what I am trying to handle with the TYPE_SIZE condition.
> 
> ontop of char X[24]; and access MEM_REF[&x].a[3] (not at struct end)
> and MEM_REF[&x].b[4] but _both_ accesses would have TYPE_SIZE NULL.
> 
> So I'm not sure TYPE_SIZE tells you anything here...

Well, here when parsing  MEM_REF[&x].a[3] array_at_struct_end_p should return
true because it parses the handled components and will see FIELD_REF for .a
that is not at end:

  while (handled_component_p (ref))                                             
    {                                                                           
      /* If the reference chain contains a component reference to a             
         non-union type and there follows another field the reference           
         is not at the end of a structure.  */                                  
      if (TREE_CODE (ref) == COMPONENT_REF                                      
          && TREE_CODE (TREE_TYPE (TREE_OPERAND (ref, 0))) == RECORD_TYPE)      
        {                                                                       
          tree nextf = DECL_CHAIN (TREE_OPERAND (ref, 1));                      
          while (nextf && TREE_CODE (nextf) != FIELD_DECL)                      
            nextf = DECL_CHAIN (nextf);                                         
          if (nextf)                                                            
            return false;                                                       
        }                                                                       
                                                                                
      ref = TREE_OPERAND (ref, 0);                                              
    }                                                                           

The size compare is meant to make difference between
struct a { int a[5]; char b[5]; };
placed in char buf[sizeof(struct a)]
or in placed in char buf[sizeof(struct a)+5]

The REF seen at this pace is the REF of ourter type after unwinding handled compoennts,
so it should have TYPE_SIZE defined in this case I think.

Honza
> 
> Richard.
> 
> > I am testing
> > 
> > Index: tree.c
> > ===================================================================
> > --- tree.c	(revision 236557)
> > +++ tree.c	(working copy)
> > @@ -13079,7 +13079,8 @@ array_at_struct_end_p (tree ref)
> >    tree size = NULL;
> >  
> >    if (TREE_CODE (ref) == MEM_REF
> > -      && TREE_CODE (TREE_OPERAND (ref, 0)) == ADDR_EXPR)
> > +      && TREE_CODE (TREE_OPERAND (ref, 0)) == ADDR_EXPR
> > +      && TYPE_SIZE (TREE_TYPE (ref)))
> >      {
> >        size = TYPE_SIZE (TREE_TYPE (ref));
> >        ref = TREE_OPERAND (TREE_OPERAND (ref, 0), 0);
> > 
> > 
> 
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
diff mbox

Patch

Index: tree.c
===================================================================
--- tree.c	(revision 236557)
+++ tree.c	(working copy)
@@ -13079,7 +13079,8 @@  array_at_struct_end_p (tree ref)
   tree size = NULL;
 
   if (TREE_CODE (ref) == MEM_REF
-      && TREE_CODE (TREE_OPERAND (ref, 0)) == ADDR_EXPR)
+      && TREE_CODE (TREE_OPERAND (ref, 0)) == ADDR_EXPR
+      && TYPE_SIZE (TREE_TYPE (ref)))
     {
       size = TYPE_SIZE (TREE_TYPE (ref));
       ref = TREE_OPERAND (TREE_OPERAND (ref, 0), 0);