Re: [PATCH] Fix type field walking in gimplifier unsharing

[Richard Biener <rguenther@suse.de>]

On Thu, 28 Apr 2016, Eric Botcazou wrote:

> > Aww, I was hoping for sth that would not require me to fix all
> > frontends ...
> 
> I don't really see how this can work without DECL_EXPR though.  You need to 
> define when the variable-sized expressions are evaluated to lay out the type, 
> otherwise it will be laid out on the first use, which may see a different 
> value of the expressions than the definition point.  The only way to do that 
> for a locally-defined type is to add a DECL_EXPR in GENERIC, so that the 
> gimplifier evaluates the expressions at the right spot.

Ah, so the C++ FE does this correctly but in addition to that it has

          /* When the pointed-to type involves components of variable 
size,
             care must be taken to ensure that the size evaluation code is
             emitted early enough to dominate all the possible later uses
             and late enough for the variables on which it depends to have
             been assigned.

             This is expected to happen automatically when the pointed-to
             type has a name/declaration of it's own, but special 
attention
             is required if the type is anonymous.
...
          if (!TYPE_NAME (type)
              && (decl_context == NORMAL || decl_context == FIELD)
              && at_function_scope_p ()
              && variably_modified_type_p (type, NULL_TREE))
            /* Force evaluation of the SAVE_EXPR.  */
            finish_expr_stmt (TYPE_SIZE (type));

so in this case the type doesn't have an associated TYPE_DECL and thus
we can't build a DECL_EXPR.  To me the correct fix is then to
always force a TYPE_DECL for variable-modified types.

Jason?

Now digging into the Fortran FE equivalent case...

Thanks,
Richard.

> Of course in Ada we have the ACATS testsuite which tests for this kind of 
> things, this explains why it already works.
> 
> > It seems the C frontend does it correctly already - I hit the
> > ubsan issue for c-c++-common/ubsan/pr59667.c and only for the C++ FE
> > for example.  Notice how only the pointed-to type is variable-size
> > here.
> > 
> > C produces
> > 
> > {
> >   unsigned int len = 1;
> >   typedef float <anon>[0:(sizetype) ((long int) SAVE_EXPR <len> +
> > -1)][0:(sizetype) ((long int) SAVE_EXPR <len> + -1)];
> >   float[0:(sizetype) ((long int) SAVE_EXPR <len> + -1)][0:(sizetype)
> > ((long int) SAVE_EXPR <len> + -1)] * P = 0B;
> > 
> >     unsigned int len = 1;
> >     typedef float <anon>[0:(sizetype) ((long int) SAVE_EXPR <len> +
> > -1)][0:(sizetype) ((long int) SAVE_EXPR <len> + -1)];
> >   SAVE_EXPR <len>;, (void) SAVE_EXPR <len>;;
> >     float[0:(sizetype) ((long int) SAVE_EXPR <len> + -1)][0:(sizetype)
> > ((long int) SAVE_EXPR <len> + -1)] * P = 0B;
> >   (*P)[0][0] = 1.0e+0;
> >   return 0;
> > }
> > 
> > the decl-expr is the 'typedef' line.  The C++ FE produces
> > 
> > {
> >   unsigned int len = 1;
> >   float[0:(sizetype) (SAVE_EXPR <(ssizetype) len + -1>)][0:(sizetype)
> > (SAVE_EXPR <(ssizetype) len + -1>)] * P = 0B;
> > 
> >   <<cleanup_point   unsigned int len = 1;>>;
> >   <<cleanup_point <<< Unknown tree: expr_stmt
> >   (void) (((bitsizetype) ((sizetype) (SAVE_EXPR <(ssizetype) len + -1>) +
> > 1) * (bitsizetype) ((sizetype) (SAVE_EXPR <(ssizetype) len + -1>) + 1)) *
> > 32) >>>>>;
> >   <<cleanup_point   float[0:(sizetype) (SAVE_EXPR <(ssizetype) len +
> > -1>)][0:(sizetype) (SAVE_EXPR <(ssizetype) len + -1>)] * P = 0B;>>;
> >   <<cleanup_point <<< Unknown tree: expr_stmt
> >   (void) ((*P)[0][0] = 1.0e+0) >>>>>;
> >   return <retval> = 0;
> > }
> > 
> > notice the lack of a decl-expr here.  It has some weird expr_stmt
> > here covering the sizes though. Possibly because VLA arrays are a GNU
> > extension.
> 
> Indeed.
> 
> > Didn't look into the fortran FE issue but I expect it's similar
> > (it only occurs for pointers to VLAs as well).
> > 
> > I'll try to come up with patches.
> > 
> > Thanks for the hint,
> 
> You're welcome.
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)