On Wed, 27 Apr 2016, Eric Botcazou wrote:
> > Gimplification is done in-place and thus relies on all processed
> > trees being unshared. This is achieved by unshare_body which
> > in the end uses walk_tree to get at all interesting trees that possibly
> > need unsharing.
> >
> > Unfortunately it doesn't really work because walk_tree only walks
> > types and type-related fields (TYPE_SIZE, TYPE_MIN_VALUE, etc.) in
> > very narrow circumstances.
>
> Right, but well defined and explained:
>
> case DECL_EXPR:
> /* If this is a TYPE_DECL, walk into the fields of the type that it's
> defining. We only want to walk into these fields of a type in this
> case and not in the general case of a mere reference to the type.
>
> The criterion is as follows: if the field can be an expression, it
> must be walked only here. This should be in keeping with the fields
> that are directly gimplified in gimplify_type_sizes in order for the
> mark/copy-if-shared/unmark machinery of the gimplifier to work with
> variable-sized types.
>
> Note that DECLs get walked as part of processing the BIND_EXPR. */
>
> > Thus the following patch which makes the gimplifier unsharing
> > visit all types.
>
> I think this will generate a lot of useless walking in Ada...
>
> > So - any opinion on the "correct" way to fix this?
>
> Add DECL_EXPRs for the types, that's what done in Ada.
Aww, I was hoping for sth that would not require me to fix all
frontends ...
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.
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,
Richard.
