Re: [PATCH] Use complete_array_type on flexible array member initializers
On 09/07/18 15:36, Bernd Edlinger wrote: > On 09/07/18 08:51, Bernd Edlinger wrote: >> On 09/07/18 00:26, Jeff Law wrote: >>> On 09/06/2018 04:16 PM, Jeff Law wrote: On 09/06/2018 04:01 PM, Jeff Law wrote: > On 09/06/2018 11:12 AM, Bernd Edlinger wrote: > >>> >> >> Ah, thanks a lot. >> >> Okay, this is the status of the STRING-CST semantic-v2 patches: >> >> [PATCH] Check the STRING_CSTs in varasm.c >> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01569.html >> => Unfortunately I forgot to change the Title to [PATCHv2] or so. >> Should I send a ping for this one? >> >> [PATCHv2] Handle overlength strings in the C FE >> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01566.html >> => Should I send a ping for this one? > No need to ping. I've got it here. What's odd is that it's regressing > 87053 . Which is probably a sign that we've got an incorrect test for NUL termination somewhere. >> >> It may be a sign that we should first fix the low level functions >> before the high level stuff. >> >>> I think I've found the issue. I've got more testing to do, but looks >>> like a thinko on my part. >>> >> >> Ah, I forgot, the regression on pr87053 and fortran.dg/pr45636.f90 >> is fixed by this patch: >> >> [PATCH] Adjust c_getstr/c_strlen to new STRING_CST semantic >> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg02013.html >> >> This is a new regression since the patch was initially posted. >> > > Well, actually both patches seem to have a circular dependency. > > If you want we can break this dependency by adding this to the c_getstr > patch: > > --- gcc/fold-const.c 2018-09-07 14:22:50.047964775 +0200 > +++ gcc/fold-const.c 2018-09-07 15:06:46.656989904 +0200 > @@ -14611,6 +14611,10 @@ c_getstr (tree src, unsigned HOST_WIDE_I > unsigned HOST_WIDE_INT string_length = TREE_STRING_LENGTH (src); > unsigned HOST_WIDE_INT string_size = tree_to_uhwi (mem_size); > > + /* Ideally this would turn into a gcc_checking_assert over time. */ > + if (string_length > string_size) > + return NULL; > + > const char *string = TREE_STRING_POINTER (src); > > if (string_length == 0 > > > This should allow it to work with current semantics as well. > Oops, this does not work for strlenopt-49.c... Please make it: --- gcc/fold-const.c2018-09-07 19:39:19.88785 +0200 +++ gcc/fold-const.c2018-09-07 19:30:03.372583484 +0200 @@ -14611,6 +14611,10 @@ c_getstr (tree src, unsigned HOST_WIDE_I unsigned HOST_WIDE_INT string_length = TREE_STRING_LENGTH (src); unsigned HOST_WIDE_INT string_size = tree_to_uhwi (mem_size); + /* Ideally this would turn into a gcc_checking_assert over time. */ + if (string_length > string_size) +string_length = string_size; + const char *string = TREE_STRING_POINTER (src); if (string_length == 0 Bernd.
Re: [PATCH] Use complete_array_type on flexible array member initializers
On 09/07/18 08:51, Bernd Edlinger wrote: > On 09/07/18 00:26, Jeff Law wrote: >> On 09/06/2018 04:16 PM, Jeff Law wrote: >>> On 09/06/2018 04:01 PM, Jeff Law wrote: On 09/06/2018 11:12 AM, Bernd Edlinger wrote: >> > > Ah, thanks a lot. > > Okay, this is the status of the STRING-CST semantic-v2 patches: > > [PATCH] Check the STRING_CSTs in varasm.c > https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01569.html > => Unfortunately I forgot to change the Title to [PATCHv2] or so. > Should I send a ping for this one? > > [PATCHv2] Handle overlength strings in the C FE > https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01566.html > => Should I send a ping for this one? No need to ping. I've got it here. What's odd is that it's regressing 87053 . >>> Which is probably a sign that we've got an incorrect test for NUL >>> termination somewhere. > > It may be a sign that we should first fix the low level functions > before the high level stuff. > >> I think I've found the issue. I've got more testing to do, but looks >> like a thinko on my part. >> > > Ah, I forgot, the regression on pr87053 and fortran.dg/pr45636.f90 > is fixed by this patch: > > [PATCH] Adjust c_getstr/c_strlen to new STRING_CST semantic > https://gcc.gnu.org/ml/gcc-patches/2018-08/msg02013.html > > This is a new regression since the patch was initially posted. > Well, actually both patches seem to have a circular dependency. If you want we can break this dependency by adding this to the c_getstr patch: --- gcc/fold-const.c2018-09-07 14:22:50.047964775 +0200 +++ gcc/fold-const.c2018-09-07 15:06:46.656989904 +0200 @@ -14611,6 +14611,10 @@ c_getstr (tree src, unsigned HOST_WIDE_I unsigned HOST_WIDE_INT string_length = TREE_STRING_LENGTH (src); unsigned HOST_WIDE_INT string_size = tree_to_uhwi (mem_size); + /* Ideally this would turn into a gcc_checking_assert over time. */ + if (string_length > string_size) +return NULL; + const char *string = TREE_STRING_POINTER (src); if (string_length == 0 This should allow it to work with current semantics as well. Bernd.
Re: [PATCH] Use complete_array_type on flexible array member initializers
On 09/07/18 00:26, Jeff Law wrote: > On 09/06/2018 04:16 PM, Jeff Law wrote: >> On 09/06/2018 04:01 PM, Jeff Law wrote: >>> On 09/06/2018 11:12 AM, Bernd Edlinger wrote: >>> > Ah, thanks a lot. Okay, this is the status of the STRING-CST semantic-v2 patches: [PATCH] Check the STRING_CSTs in varasm.c https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01569.html => Unfortunately I forgot to change the Title to [PATCHv2] or so. Should I send a ping for this one? [PATCHv2] Handle overlength strings in the C FE https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01566.html => Should I send a ping for this one? >>> No need to ping. I've got it here. What's odd is that it's regressing >>> 87053 . >> Which is probably a sign that we've got an incorrect test for NUL >> termination somewhere. It may be a sign that we should first fix the low level functions before the high level stuff. > I think I've found the issue. I've got more testing to do, but looks > like a thinko on my part. > Ah, I forgot, the regression on pr87053 and fortran.dg/pr45636.f90 is fixed by this patch: [PATCH] Adjust c_getstr/c_strlen to new STRING_CST semantic https://gcc.gnu.org/ml/gcc-patches/2018-08/msg02013.html This is a new regression since the patch was initially posted. Bernd.
Re: [PATCH] Use complete_array_type on flexible array member initializers
On 09/06/2018 04:16 PM, Jeff Law wrote: > On 09/06/2018 04:01 PM, Jeff Law wrote: >> On 09/06/2018 11:12 AM, Bernd Edlinger wrote: >> >>> >>> Ah, thanks a lot. >>> >>> Okay, this is the status of the STRING-CST semantic-v2 patches: >>> >>> [PATCH] Check the STRING_CSTs in varasm.c >>> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01569.html >>> => Unfortunately I forgot to change the Title to [PATCHv2] or so. >>> Should I send a ping for this one? >>> >>> [PATCHv2] Handle overlength strings in the C FE >>> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01566.html >>> => Should I send a ping for this one? >> No need to ping. I've got it here. What's odd is that it's regressing >> 87053 . > Which is probably a sign that we've got an incorrect test for NUL > termination somewhere. I think I've found the issue. I've got more testing to do, but looks like a thinko on my part. jeff
Re: [PATCH] Use complete_array_type on flexible array member initializers
On 09/06/2018 04:01 PM, Jeff Law wrote: > On 09/06/2018 11:12 AM, Bernd Edlinger wrote: > >>> >> >> Ah, thanks a lot. >> >> Okay, this is the status of the STRING-CST semantic-v2 patches: >> >> [PATCH] Check the STRING_CSTs in varasm.c >> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01569.html >> => Unfortunately I forgot to change the Title to [PATCHv2] or so. >> Should I send a ping for this one? >> >> [PATCHv2] Handle overlength strings in the C FE >> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01566.html >> => Should I send a ping for this one? > No need to ping. I've got it here. What's odd is that it's regressing > 87053 . Which is probably a sign that we've got an incorrect test for NUL termination somewhere. jeff
Re: [PATCH] Use complete_array_type on flexible array member initializers
On 09/06/2018 11:12 AM, Bernd Edlinger wrote: >> > > Ah, thanks a lot. > > Okay, this is the status of the STRING-CST semantic-v2 patches: > > [PATCH] Check the STRING_CSTs in varasm.c > https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01569.html > => Unfortunately I forgot to change the Title to [PATCHv2] or so. > Should I send a ping for this one? > > [PATCHv2] Handle overlength strings in the C FE > https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01566.html > => Should I send a ping for this one? No need to ping. I've got it here. What's odd is that it's regressing 87053 . Jeff
Re: [PATCH] Use complete_array_type on flexible array member initializers
On 09/06/18 17:43, Jeff Law wrote:
> On 09/06/2018 05:05 AM, Bernd Edlinger wrote:
>> On 09/04/18 16:30, Jeff Law wrote:
>>> On 09/03/2018 06:35 AM, Bernd Edlinger wrote:
>>> [ Big snip, dropping lots of context ]
>>>
>> No I don't think so, because at that time BRACE_ENCLOSED_INITIALIZER_P
>> property does no longer work, as I explained in the previous mail.
>>
>> And cp_complete_array_type does use that property:
> [ ... ]
> Jason's the expert here.I'll defer to his expertise. It just seemed
> a bit odd to me that we have a routine to "complete" an array that does
> any special C++ handling (cp_complete_array_type) and we're not using it.
>
Agreed, I have posted an update which uses cp_complete_array_type, since
Jason raised the same point.
But since C++ does not need the recursion here, things are a lot more easy.
The patch still completes the array type _after_ the FE is completely done
with the
parsing, since I want to avoid to destroy the information too early, since
the FE is looking
at the TYPE_DOMAIN==NULL at various places.
>>> FYI, I don't see a follow-up for this patch? Did I miss it? Or is it
>>> under a different subject line?
>>>
>>> jeff
>>>
>> Yes, thanks for reminding me.
>> I had forgotten to post the updated patch.
>>
>> So here is my latest version of the flexible array patch.
>>
>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu (together with the other
>> STRING_CST-v2 patches).
>> Is it OK for trunk?
>>
>>
>> Thanks
>> Bernd.
>>
>>
>> patch-flexarray.diff
>>
>>
>> gcc:
>> 2018-08-30 Bernd Edlinger
>>
>> * varasm.c (output_constructor_regular_field): Check TYPE_SIZE_UNIT of
>> the init value.
>>
>> c-family:
>> 2018-08-30 Bernd Edlinger
>>
>> * c-common.c (complete_flexible_array_elts): New helper function.
>> * c-common.h (complete_flexible_array_elts): Declare.
>>
>> c:
>> 2018-08-30 Bernd Edlinger
>>
>> * c-decl.c (finish_decl): Call complete_flexible_array_elts.
>>
>> cp:
>> 2018-08-30 Bernd Edlinger
>>
>> * decl.c (check_initializer): Call complete_flexible_array_elts.
> Thanks. I fixed up the ChangeLog entry for the cp/ change and installed
> this patch.
>
> As you've probably noted, I'm taking care of the installs on this stuff.
> I'm doing additional testing and as a result I've got a tree with your
> proposed patch "push ready". It'd be silly to not go ahead with the
> push :-)
>
Ah, thanks a lot.
Okay, this is the status of the STRING-CST semantic-v2 patches:
[PATCH] Check the STRING_CSTs in varasm.c
https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01569.html
=> Unfortunately I forgot to change the Title to [PATCHv2] or so.
Should I send a ping for this one?
[PATCHv2] Handle overlength strings in the C FE
https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01566.html
=> Should I send a ping for this one?
[PATCHv2] Handle overlength strings in C++ FE
https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01567.html
=> Apporoved, without the part in vtable-class-hierarchy.c (!)
[PATCHv2] Handle overlength string literals in the fortan FE
https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01568.html
=> Approved.
Additional patches in the same area:
[PATCH] Fix not properly nul-terminated string constants in JIT
https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00370.html
=> This triggered an assertion in the V1-patch only, but is no
pre-condition in the V2-semantic patch. Anyway would be good
to get it in, since strings longer than 200 bytes will not be
zero-terminated.
I have a nice follow-up patch that allows non-zero terminated
strings (mostly found in Ada and GO) to go into the string merge
section.
I did not post it yet to avoid confusion, but if you are
interested to see it, I'll go ahead and post it.
Regarding the nonstr parameter in string_constant,
I still do have the impression that it would be more natural
for string_constant _not_ to care about zero-termination, but instead
move that check to c_strlen and c_getstr or similar high level
functions, where for instance also a location value would be
available.
I could imagine something like an optional out-parameter with
detailed error info
struct nonstr
{
enum { no_error, char_type_mismatch, missing_zero_termination } err;
tree str;
location_t loc;
nonstr(): err(no_error), str(NULL_TREE), loc(UNKNOWN_LOCATION) {}
};
This information would be available in c_strlen here:
if (eltsize != tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (src)
return NULL_TREE;
and here:
/* Don't know what to return if there was no zero termination.
Ideally this would turn into a gcc_checking_assert over time. */
if (len > maxelts - eltoff)
return NULL_TREE;
Thanks
Bernd.
Re: [PATCH] Use complete_array_type on flexible array member initializers
On 09/06/2018 05:05 AM, Bernd Edlinger wrote: > On 09/04/18 16:30, Jeff Law wrote: >> On 09/03/2018 06:35 AM, Bernd Edlinger wrote: >> [ Big snip, dropping lots of context ] >> > No I don't think so, because at that time BRACE_ENCLOSED_INITIALIZER_P > property does no longer work, as I explained in the previous mail. > > And cp_complete_array_type does use that property: [ ... ] Jason's the expert here.I'll defer to his expertise. It just seemed a bit odd to me that we have a routine to "complete" an array that does any special C++ handling (cp_complete_array_type) and we're not using it. >>> Agreed, I have posted an update which uses cp_complete_array_type, since >>> Jason raised the same point. >>> >>> But since C++ does not need the recursion here, things are a lot more easy. >>> The patch still completes the array type _after_ the FE is completely done >>> with the >>> parsing, since I want to avoid to destroy the information too early, since >>> the FE is looking >>> at the TYPE_DOMAIN==NULL at various places. >> FYI, I don't see a follow-up for this patch? Did I miss it? Or is it >> under a different subject line? >> >> jeff >> > Yes, thanks for reminding me. > I had forgotten to post the updated patch. > > So here is my latest version of the flexible array patch. > > Bootstrapped and reg-tested on x86_64-pc-linux-gnu (together with the other > STRING_CST-v2 patches). > Is it OK for trunk? > > > Thanks > Bernd. > > > patch-flexarray.diff > > > gcc: > 2018-08-30 Bernd Edlinger > > * varasm.c (output_constructor_regular_field): Check TYPE_SIZE_UNIT of > the init value. > > c-family: > 2018-08-30 Bernd Edlinger > > * c-common.c (complete_flexible_array_elts): New helper function. > * c-common.h (complete_flexible_array_elts): Declare. > > c: > 2018-08-30 Bernd Edlinger > > * c-decl.c (finish_decl): Call complete_flexible_array_elts. > > cp: > 2018-08-30 Bernd Edlinger > > * decl.c (check_initializer): Call complete_flexible_array_elts. Thanks. I fixed up the ChangeLog entry for the cp/ change and installed this patch. As you've probably noted, I'm taking care of the installs on this stuff. I'm doing additional testing and as a result I've got a tree with your proposed patch "push ready". It'd be silly to not go ahead with the push :-) Jeff
Re: [PATCH] Use complete_array_type on flexible array member initializers
On 09/04/18 16:30, Jeff Law wrote:
> On 09/03/2018 06:35 AM, Bernd Edlinger wrote:
> [ Big snip, dropping lots of context ]
>
>
No I don't think so, because at that time BRACE_ENCLOSED_INITIALIZER_P
property does no longer work, as I explained in the previous mail.
And cp_complete_array_type does use that property:
>>> [ ... ]
>>> Jason's the expert here.I'll defer to his expertise. It just seemed
>>> a bit odd to me that we have a routine to "complete" an array that does
>>> any special C++ handling (cp_complete_array_type) and we're not using it.
>>>
>>
>> Agreed, I have posted an update which uses cp_complete_array_type, since
>> Jason raised the same point.
>>
>> But since C++ does not need the recursion here, things are a lot more easy.
>> The patch still completes the array type _after_ the FE is completely done
>> with the
>> parsing, since I want to avoid to destroy the information too early, since
>> the FE is looking
>> at the TYPE_DOMAIN==NULL at various places.
> FYI, I don't see a follow-up for this patch? Did I miss it? Or is it
> under a different subject line?
>
> jeff
>
Yes, thanks for reminding me.
I had forgotten to post the updated patch.
So here is my latest version of the flexible array patch.
Bootstrapped and reg-tested on x86_64-pc-linux-gnu (together with the other
STRING_CST-v2 patches).
Is it OK for trunk?
Thanks
Bernd.
gcc:
2018-08-30 Bernd Edlinger
* varasm.c (output_constructor_regular_field): Check TYPE_SIZE_UNIT of
the init value.
c-family:
2018-08-30 Bernd Edlinger
* c-common.c (complete_flexible_array_elts): New helper function.
* c-common.h (complete_flexible_array_elts): Declare.
c:
2018-08-30 Bernd Edlinger
* c-decl.c (finish_decl): Call complete_flexible_array_elts.
cp:
2018-08-30 Bernd Edlinger
* decl.c (check_initializer): Call complete_flexible_array_elts.
diff -Npur gcc/c/c-decl.c gcc/c/c-decl.c
--- gcc/c/c-decl.c 2018-08-21 08:17:41.0 +0200
+++ gcc/c/c-decl.c 2018-08-24 12:06:21.374892294 +0200
@@ -5035,6 +5035,8 @@ finish_decl (tree decl, location_t init_
if (init && TREE_CODE (init) == CONSTRUCTOR)
add_flexible_array_elts_to_size (decl, init);
+ complete_flexible_array_elts (DECL_INITIAL (decl));
+
if (DECL_SIZE (decl) == NULL_TREE && TREE_TYPE (decl) != error_mark_node
&& COMPLETE_TYPE_P (TREE_TYPE (decl)))
layout_decl (decl, 0);
diff -Npur gcc/c-family/c-common.c gcc/c-family/c-common.c
--- gcc/c-family/c-common.c 2018-08-17 05:02:11.0 +0200
+++ gcc/c-family/c-common.c 2018-08-24 12:45:56.559011703 +0200
@@ -6427,6 +6427,28 @@ complete_array_type (tree *ptype, tree i
return failure;
}
+/* INIT is an constructor of a structure with a flexible array member.
+ Complete the flexible array member with a domain based on it's value. */
+void
+complete_flexible_array_elts (tree init)
+{
+ tree elt, type;
+
+ if (init == NULL_TREE || TREE_CODE (init) != CONSTRUCTOR)
+return;
+
+ if (vec_safe_is_empty (CONSTRUCTOR_ELTS (init)))
+return;
+
+ elt = CONSTRUCTOR_ELTS (init)->last ().value;
+ type = TREE_TYPE (elt);
+ if (TREE_CODE (type) == ARRAY_TYPE
+ && TYPE_SIZE (type) == NULL_TREE)
+complete_array_type (_TYPE (elt), elt, false);
+ else
+complete_flexible_array_elts (elt);
+}
+
/* Like c_mark_addressable but don't check register qualifier. */
void
c_common_mark_addressable_vec (tree t)
diff -Npur gcc/c-family/c-common.h gcc/c-family/c-common.h
--- gcc/c-family/c-common.h 2018-08-17 05:02:11.0 +0200
+++ gcc/c-family/c-common.h 2018-08-24 12:06:21.375892280 +0200
@@ -1038,6 +1038,7 @@ extern tree fold_offsetof (tree, tree =
tree_code ctx = ERROR_MARK);
extern int complete_array_type (tree *, tree, bool);
+extern void complete_flexible_array_elts (tree);
extern tree builtin_type_for_size (int, bool);
diff -Npur gcc/cp/decl.c gcc/cp/decl.c
--- gcc/cp/decl.c 2018-08-22 22:35:38.0 +0200
+++ gcc/cp/decl.c 2018-08-30 12:06:21.377892252 +0200
@@ -6530,6 +6530,16 @@ check_initializer (tree decl, tree init,
init_code = store_init_value (decl, init, cleanups, flags);
+ if (DECL_INITIAL (decl)
+ && TREE_CODE (DECL_INITIAL (decl)) == CONSTRUCTOR
+ && !vec_safe_is_empty (CONSTRUCTOR_ELTS (DECL_INITIAL (decl
+ {
+ tree elt = CONSTRUCTOR_ELTS (DECL_INITIAL (decl))->last ().value;
+ if (TREE_CODE (TREE_TYPE (elt)) == ARRAY_TYPE
+ && TYPE_SIZE (TREE_TYPE (elt)) == NULL_TREE)
+ cp_complete_array_type (_TYPE (elt), elt, false);
+ }
+
if (pedantic && TREE_CODE (type) == ARRAY_TYPE
&& DECL_INITIAL (decl)
&& TREE_CODE (DECL_INITIAL (decl)) == STRING_CST
diff -Npur gcc/varasm.c gcc/varasm.c
--- gcc/varasm.c 2018-08-16 17:28:11.0 +0200
+++ gcc/varasm.c 2018-08-24 12:06:21.378892238 +0200
@@ -5161,6 +5161,8 @@ output_constructor_regular_field (oc_loc
on the chain is a TYPE_DECL of the enclosing struct. */
const_tree
Re: [PATCH] Use complete_array_type on flexible array member initializers
On 09/03/2018 06:35 AM, Bernd Edlinger wrote: [ Big snip, dropping lots of context ] >>> >>> No I don't think so, because at that time BRACE_ENCLOSED_INITIALIZER_P >>> property does no longer work, as I explained in the previous mail. >>> >>> And cp_complete_array_type does use that property: >> [ ... ] >> Jason's the expert here.I'll defer to his expertise. It just seemed >> a bit odd to me that we have a routine to "complete" an array that does >> any special C++ handling (cp_complete_array_type) and we're not using it. >> > > Agreed, I have posted an update which uses cp_complete_array_type, since > Jason raised the same point. > > But since C++ does not need the recursion here, things are a lot more easy. > The patch still completes the array type _after_ the FE is completely done > with the > parsing, since I want to avoid to destroy the information too early, since > the FE is looking > at the TYPE_DOMAIN==NULL at various places. FYI, I don't see a follow-up for this patch? Did I miss it? Or is it under a different subject line? jeff
Re: [PATCH] Use complete_array_type on flexible array member initializers
On 09/03/2018 05:43, Jeff Law wrote:
> On 08/26/2018 02:52 AM, Bernd Edlinger wrote:
>> On 08/26/18 07:36, Jeff Law wrote:
>>> On 08/24/2018 07:13 AM, Bernd Edlinger wrote:
Hi!
>>> This patch prevents init values of STRING_CST and braced
>>> array initializers to reach the middle-end with incomplete
>>> type.
This will allow further simplifications in the middle-end,
and address existing issues with STRING_CST in a correct
way.
Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?
Thanks
Bernd.
patch-flexarray.diff
gcc:
2018-08-24 Bernd Edlinger
>>>
* varasm.c (output_constructor_regular_field): Check TYPE_SIZE_UNIT of
the init value.
>>> c-family:
2018-08-24 Bernd Edlinger
* c-common.c (complete_flexible_array_elts): New helper function.
* c-common.h (complete_flexible_array_elts): Declare.
c:
2018-08-24 Bernd Edlinger
* c-decl.c (finish_decl): Call complete_flexible_array_elts.
cp:
2018-08-24 Bernd Edlinger
* decl.c (check_initializer): Call complete_flexible_array_elts.
diff -Npur gcc/c/c-decl.c gcc/c/c-decl.c
--- gcc/c/c-decl.c 2018-08-21 08:17:41.0 +0200
+++ gcc/c/c-decl.c 2018-08-24 12:06:21.374892294 +0200
@@ -5035,6 +5035,8 @@ finish_decl (tree decl, location_t init_
if (init && TREE_CODE (init) == CONSTRUCTOR)
add_flexible_array_elts_to_size (decl, init);
+ complete_flexible_array_elts (DECL_INITIAL (decl));
+
if (DECL_SIZE (decl) == NULL_TREE && TREE_TYPE (decl) !=
error_mark_node
&& COMPLETE_TYPE_P (TREE_TYPE (decl)))
layout_decl (decl, 0);
diff -Npur gcc/c-family/c-common.c gcc/c-family/c-common.c
--- gcc/c-family/c-common.c 2018-08-17 05:02:11.0 +0200
+++ gcc/c-family/c-common.c 2018-08-24 12:45:56.559011703 +0200
@@ -6427,6 +6427,28 @@ complete_array_type (tree *ptype, tree i
return failure;
}
+/* INIT is an constructor of a structure with a flexible array member.
+ Complete the flexible array member with a domain based on it's value.
*/
+void
+complete_flexible_array_elts (tree init)
+{
+ tree elt, type;
+
+ if (init == NULL_TREE || TREE_CODE (init) != CONSTRUCTOR)
+return;
+
+ if (vec_safe_is_empty (CONSTRUCTOR_ELTS (init)))
+return;
+
+ elt = CONSTRUCTOR_ELTS (init)->last ().value;
+ type = TREE_TYPE (elt);
+ if (TREE_CODE (type) == ARRAY_TYPE
+ && TYPE_SIZE (type) == NULL_TREE)
+complete_array_type (_TYPE (elt), elt, false);
+ else
+complete_flexible_array_elts (elt);
+}
>>> Shouldn't this be handled in c-decl.c by the call to
>>> add_flexible_array_elts_to_size?Why the recursion when the
>>> CONSTRUCTOR_ELT isn't an array type?
>>>
>>
>> There are tests in the test suite that use something like that:
>> struct {
>>union {
>> struct {
>> int a;
>> char b[];
>> };
>> struct {
>> char x[32];
>> };
>>};
>> } u = { { { 1, "test" } } };
>>
>> So it fails to go through add_flexible_array_elts_to_size.
> Huh? We get into add_flexible_array_elts_to_size just fine. Using your
> testcase above:
Yes, I was not clear enough here.
It is called, but it does only handle the case where the flexible array
is the last element of a CONSTRUCTOR, but not the case here,
where the flexible array is the last element of a CONSTRUCTOR within
another CONSTRUCTOR.
> Breakpoint 3, add_flexible_array_elts_to_size (decl=0x77ff6ab0,
> init=0x7fffe9f3edc8) at /home/gcc/GIT-3/gcc/gcc/c/c-decl.c:4617
> 4617 if (vec_safe_is_empty (CONSTRUCTOR_ELTS (init)))
> (gdb) p debug_tree (decl)
> type size
> unit-size
> align:32 warn_if_not_align:0 symtab:0 alias-set -1
> canonical-type 0x7fffe9f35498
> fields 0x7fffe9f35540>
> BLK j.c:10:4 size unit-size
>
> align:32 warn_if_not_align:0 offset_align 128
> offset
> bit-offset context
> >
> chain >
> public static BLK j.c:11:3 size
> unit-size
> align:32 warn_if_not_align:0 initial >
>
>
>
> So I'm a bit confused. It seems to go through
> add_flexible_array_elts_to_size in my limited testing.
>
>
> Given how closely complete_flexible_array_elts is to
> add_flexible_array_elts_to_size I can't help but continue to wonder if
> we're really papering over a problem in add_flexible_array_elts_to_size.
>
> But it may also be the case that the recursive needs to handle the
> CONSTRUCTORS embedded within other CONSTRUCTORS mean we want two routines.
>
> Basically I want to see a rationale why we want/need two routines here.
>
My major concern here is
Re: [PATCH] Use complete_array_type on flexible array member initializers
On 08/26/2018 02:52 AM, Bernd Edlinger wrote:
> On 08/26/18 07:36, Jeff Law wrote:
>> On 08/24/2018 07:13 AM, Bernd Edlinger wrote:
>>> Hi!
>>>
>>>
>>> This patch prevents init values of STRING_CST and braced
>>> array initializers to reach the middle-end with incomplete
>>> type.
>>>
>>> This will allow further simplifications in the middle-end,
>>> and address existing issues with STRING_CST in a correct
>>> way.
>>>
>>>
>>>
>>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>>> Is it OK for trunk?
>>>
>>>
>>> Thanks
>>> Bernd.
>>>
>>>
>>> patch-flexarray.diff
>>>
>>>
>>> gcc:
>>> 2018-08-24 Bernd Edlinger
>>>
>>> * varasm.c (output_constructor_regular_field): Check TYPE_SIZE_UNIT of
>>> the init value.
>>>
>>> c-family:
>>> 2018-08-24 Bernd Edlinger
>>>
>>> * c-common.c (complete_flexible_array_elts): New helper function.
>>> * c-common.h (complete_flexible_array_elts): Declare.
>>>
>>> c:
>>> 2018-08-24 Bernd Edlinger
>>>
>>> * c-decl.c (finish_decl): Call complete_flexible_array_elts.
>>>
>>> cp:
>>> 2018-08-24 Bernd Edlinger
>>>
>>> * decl.c (check_initializer): Call complete_flexible_array_elts.
>>>
>>>
>>> diff -Npur gcc/c/c-decl.c gcc/c/c-decl.c
>>> --- gcc/c/c-decl.c 2018-08-21 08:17:41.0 +0200
>>> +++ gcc/c/c-decl.c 2018-08-24 12:06:21.374892294 +0200
>>> @@ -5035,6 +5035,8 @@ finish_decl (tree decl, location_t init_
>>> if (init && TREE_CODE (init) == CONSTRUCTOR)
>>> add_flexible_array_elts_to_size (decl, init);
>>>
>>> + complete_flexible_array_elts (DECL_INITIAL (decl));
>>> +
>>> if (DECL_SIZE (decl) == NULL_TREE && TREE_TYPE (decl) !=
>>> error_mark_node
>>> && COMPLETE_TYPE_P (TREE_TYPE (decl)))
>>> layout_decl (decl, 0);
>>> diff -Npur gcc/c-family/c-common.c gcc/c-family/c-common.c
>>> --- gcc/c-family/c-common.c 2018-08-17 05:02:11.0 +0200
>>> +++ gcc/c-family/c-common.c 2018-08-24 12:45:56.559011703 +0200
>>> @@ -6427,6 +6427,28 @@ complete_array_type (tree *ptype, tree i
>>> return failure;
>>> }
>>>
>>> +/* INIT is an constructor of a structure with a flexible array member.
>>> + Complete the flexible array member with a domain based on it's value.
>>> */
>>> +void
>>> +complete_flexible_array_elts (tree init)
>>> +{
>>> + tree elt, type;
>>> +
>>> + if (init == NULL_TREE || TREE_CODE (init) != CONSTRUCTOR)
>>> +return;
>>> +
>>> + if (vec_safe_is_empty (CONSTRUCTOR_ELTS (init)))
>>> +return;
>>> +
>>> + elt = CONSTRUCTOR_ELTS (init)->last ().value;
>>> + type = TREE_TYPE (elt);
>>> + if (TREE_CODE (type) == ARRAY_TYPE
>>> + && TYPE_SIZE (type) == NULL_TREE)
>>> +complete_array_type (_TYPE (elt), elt, false);
>>> + else
>>> +complete_flexible_array_elts (elt);
>>> +}
>> Shouldn't this be handled in c-decl.c by the call to
>> add_flexible_array_elts_to_size?Why the recursion when the
>> CONSTRUCTOR_ELT isn't an array type?
>>
>
> There are tests in the test suite that use something like that:
> struct {
>union {
> struct {
> int a;
> char b[];
> };
> struct {
> char x[32];
> };
>};
> } u = { { { 1, "test" } } };
>
> So it fails to go through add_flexible_array_elts_to_size.
Huh? We get into add_flexible_array_elts_to_size just fine. Using your
testcase above:
Breakpoint 3, add_flexible_array_elts_to_size (decl=0x77ff6ab0,
init=0x7fffe9f3edc8) at /home/gcc/GIT-3/gcc/gcc/c/c-decl.c:4617
4617 if (vec_safe_is_empty (CONSTRUCTOR_ELTS (init)))
(gdb) p debug_tree (decl)
unit-size
align:32 warn_if_not_align:0 symtab:0 alias-set -1
canonical-type 0x7fffe9f35498
fields
BLK j.c:10:4 size unit-size
align:32 warn_if_not_align:0 offset_align 128
offset
bit-offset context
>
chain >
public static BLK j.c:11:3 size
unit-size
align:32 warn_if_not_align:0 initial >
So I'm a bit confused. It seems to go through
add_flexible_array_elts_to_size in my limited testing.
Given how closely complete_flexible_array_elts is to
add_flexible_array_elts_to_size I can't help but continue to wonder if
we're really papering over a problem in add_flexible_array_elts_to_size.
But it may also be the case that the recursive needs to handle the
CONSTRUCTORS embedded within other CONSTRUCTORS mean we want two routines.
Basically I want to see a rationale why we want/need two routines here.
> I am not sure what happens if the string is larger than 32 byte.
> The test suite does not do that.
> Well I just tried, while writing those lines:
>
> struct {
> union {
>struct {
> int a;
> char b[];
>};
>struct {
> char x[4];
>};
> };
> } u = { { { 1, "test" } } };
>
> =>
>
> .file "t.c"
> .text
> .globl u
> .data
> .align 4
> .type u, @object
> .size u, 4
> u:
> .long 1
> .string "test"
> .ident "GCC: (GNU) 9.0.0
Re: [PATCH] Use complete_array_type on flexible array member initializers
On Sun, Aug 26, 2018 at 4:52 AM, Bernd Edlinger
wrote:
> On 08/26/18 07:36, Jeff Law wrote:
>> On 08/24/2018 07:13 AM, Bernd Edlinger wrote:
>>> Hi!
>>>
>>>
>>> This patch prevents init values of STRING_CST and braced
>>> array initializers to reach the middle-end with incomplete
>>> type.
>>>
>>> This will allow further simplifications in the middle-end,
>>> and address existing issues with STRING_CST in a correct
>>> way.
>>>
>>>
>>>
>>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>>> Is it OK for trunk?
>>>
>>>
>>> Thanks
>>> Bernd.
>>>
>>>
>>> patch-flexarray.diff
>>>
>>>
>>> gcc:
>>> 2018-08-24 Bernd Edlinger
>>>
>>> * varasm.c (output_constructor_regular_field): Check TYPE_SIZE_UNIT of
>>> the init value.
>>>
>>> c-family:
>>> 2018-08-24 Bernd Edlinger
>>>
>>> * c-common.c (complete_flexible_array_elts): New helper function.
>>> * c-common.h (complete_flexible_array_elts): Declare.
>>>
>>> c:
>>> 2018-08-24 Bernd Edlinger
>>>
>>> * c-decl.c (finish_decl): Call complete_flexible_array_elts.
>>>
>>> cp:
>>> 2018-08-24 Bernd Edlinger
>>>
>>> * decl.c (check_initializer): Call complete_flexible_array_elts.
>>>
>>>
>>> diff -Npur gcc/c/c-decl.c gcc/c/c-decl.c
>>> --- gcc/c/c-decl.c 2018-08-21 08:17:41.0 +0200
>>> +++ gcc/c/c-decl.c 2018-08-24 12:06:21.374892294 +0200
>>> @@ -5035,6 +5035,8 @@ finish_decl (tree decl, location_t init_
>>> if (init && TREE_CODE (init) == CONSTRUCTOR)
>>> add_flexible_array_elts_to_size (decl, init);
>>>
>>> + complete_flexible_array_elts (DECL_INITIAL (decl));
>>> +
>>> if (DECL_SIZE (decl) == NULL_TREE && TREE_TYPE (decl) !=
>>> error_mark_node
>>>&& COMPLETE_TYPE_P (TREE_TYPE (decl)))
>>> layout_decl (decl, 0);
>>> diff -Npur gcc/c-family/c-common.c gcc/c-family/c-common.c
>>> --- gcc/c-family/c-common.c 2018-08-17 05:02:11.0 +0200
>>> +++ gcc/c-family/c-common.c 2018-08-24 12:45:56.559011703 +0200
>>> @@ -6427,6 +6427,28 @@ complete_array_type (tree *ptype, tree i
>>> return failure;
>>> }
>>>
>>> +/* INIT is an constructor of a structure with a flexible array member.
>>> + Complete the flexible array member with a domain based on it's value.
>>> */
>>> +void
>>> +complete_flexible_array_elts (tree init)
>>> +{
>>> + tree elt, type;
>>> +
>>> + if (init == NULL_TREE || TREE_CODE (init) != CONSTRUCTOR)
>>> +return;
>>> +
>>> + if (vec_safe_is_empty (CONSTRUCTOR_ELTS (init)))
>>> +return;
>>> +
>>> + elt = CONSTRUCTOR_ELTS (init)->last ().value;
>>> + type = TREE_TYPE (elt);
>>> + if (TREE_CODE (type) == ARRAY_TYPE
>>> + && TYPE_SIZE (type) == NULL_TREE)
>>> +complete_array_type (_TYPE (elt), elt, false);
>>> + else
>>> +complete_flexible_array_elts (elt);
>>> +}
>> Shouldn't this be handled in c-decl.c by the call to
>> add_flexible_array_elts_to_size?Why the recursion when the
>> CONSTRUCTOR_ELT isn't an array type?
>>
>
> There are tests in the test suite that use something like that:
> struct {
>union {
> struct {
> int a;
> char b[];
> };
> struct {
> char x[32];
> };
>};
> } u = { { { 1, "test" } } };
>
> So it fails to go through add_flexible_array_elts_to_size.
>
> I am not sure what happens if the string is larger than 32 byte.
> The test suite does not do that.
> Well I just tried, while writing those lines:
>
> struct {
> union {
>struct {
> int a;
> char b[];
>};
>struct {
> char x[4];
>};
> };
> } u = { { { 1, "test" } } };
>
> =>
>
> .file "t.c"
> .text
> .globl u
> .data
> .align 4
> .type u, @object
> .size u, 4
> u:
> .long 1
> .string "test"
> .ident "GCC: (GNU) 9.0.0 20180825 (experimental)"
> .section.note.GNU-stack,"",@progbits
>
> So the .size is too small but that is probably only a fauxpas.
>
>
>>
>>> diff -Npur gcc/cp/decl.c gcc/cp/decl.c
>>> --- gcc/cp/decl.c2018-08-22 22:35:38.0 +0200
>>> +++ gcc/cp/decl.c2018-08-24 12:06:21.377892252 +0200
>>> @@ -6528,6 +6528,8 @@ check_initializer (tree decl, tree init,
>>>
>>>init_code = store_init_value (decl, init, cleanups, flags);
>>>
>>> + complete_flexible_array_elts (DECL_INITIAL (decl));
>>> +
>>>if (pedantic && TREE_CODE (type) == ARRAY_TYPE
>>>&& DECL_INITIAL (decl)
>>>&& TREE_CODE (DECL_INITIAL (decl)) == STRING_CST
>> Should the C++ front-end be going through cp_complete_array_type instead?
>>
>
> No I don't think so, because at that time BRACE_ENCLOSED_INITIALIZER_P
> property does no longer work, as I explained in the previous mail.
>
> And cp_complete_array_type does use that property:
>
> int
> cp_complete_array_type (tree *ptype, tree initial_value, bool do_default)
> {
>int failure;
>tree type, elt_type;
>
>/* Don't get confused by a CONSTRUCTOR for some other type. */
>if
Re: [PATCH] Use complete_array_type on flexible array member initializers
On 08/26/18 07:36, Jeff Law wrote:
> On 08/24/2018 07:13 AM, Bernd Edlinger wrote:
>> Hi!
>>
>>
>> This patch prevents init values of STRING_CST and braced
>> array initializers to reach the middle-end with incomplete
>> type.
>>
>> This will allow further simplifications in the middle-end,
>> and address existing issues with STRING_CST in a correct
>> way.
>>
>>
>>
>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>> Is it OK for trunk?
>>
>>
>> Thanks
>> Bernd.
>>
>>
>> patch-flexarray.diff
>>
>>
>> gcc:
>> 2018-08-24 Bernd Edlinger
>>
>> * varasm.c (output_constructor_regular_field): Check TYPE_SIZE_UNIT of
>> the init value.
>>
>> c-family:
>> 2018-08-24 Bernd Edlinger
>>
>> * c-common.c (complete_flexible_array_elts): New helper function.
>> * c-common.h (complete_flexible_array_elts): Declare.
>>
>> c:
>> 2018-08-24 Bernd Edlinger
>>
>> * c-decl.c (finish_decl): Call complete_flexible_array_elts.
>>
>> cp:
>> 2018-08-24 Bernd Edlinger
>>
>> * decl.c (check_initializer): Call complete_flexible_array_elts.
>>
>>
>> diff -Npur gcc/c/c-decl.c gcc/c/c-decl.c
>> --- gcc/c/c-decl.c 2018-08-21 08:17:41.0 +0200
>> +++ gcc/c/c-decl.c 2018-08-24 12:06:21.374892294 +0200
>> @@ -5035,6 +5035,8 @@ finish_decl (tree decl, location_t init_
>> if (init && TREE_CODE (init) == CONSTRUCTOR)
>> add_flexible_array_elts_to_size (decl, init);
>>
>> + complete_flexible_array_elts (DECL_INITIAL (decl));
>> +
>> if (DECL_SIZE (decl) == NULL_TREE && TREE_TYPE (decl) !=
>> error_mark_node
>>&& COMPLETE_TYPE_P (TREE_TYPE (decl)))
>> layout_decl (decl, 0);
>> diff -Npur gcc/c-family/c-common.c gcc/c-family/c-common.c
>> --- gcc/c-family/c-common.c 2018-08-17 05:02:11.0 +0200
>> +++ gcc/c-family/c-common.c 2018-08-24 12:45:56.559011703 +0200
>> @@ -6427,6 +6427,28 @@ complete_array_type (tree *ptype, tree i
>> return failure;
>> }
>>
>> +/* INIT is an constructor of a structure with a flexible array member.
>> + Complete the flexible array member with a domain based on it's value. */
>> +void
>> +complete_flexible_array_elts (tree init)
>> +{
>> + tree elt, type;
>> +
>> + if (init == NULL_TREE || TREE_CODE (init) != CONSTRUCTOR)
>> +return;
>> +
>> + if (vec_safe_is_empty (CONSTRUCTOR_ELTS (init)))
>> +return;
>> +
>> + elt = CONSTRUCTOR_ELTS (init)->last ().value;
>> + type = TREE_TYPE (elt);
>> + if (TREE_CODE (type) == ARRAY_TYPE
>> + && TYPE_SIZE (type) == NULL_TREE)
>> +complete_array_type (_TYPE (elt), elt, false);
>> + else
>> +complete_flexible_array_elts (elt);
>> +}
> Shouldn't this be handled in c-decl.c by the call to
> add_flexible_array_elts_to_size?Why the recursion when the
> CONSTRUCTOR_ELT isn't an array type?
>
There are tests in the test suite that use something like that:
struct {
union {
struct {
int a;
char b[];
};
struct {
char x[32];
};
};
} u = { { { 1, "test" } } };
So it fails to go through add_flexible_array_elts_to_size.
I am not sure what happens if the string is larger than 32 byte.
The test suite does not do that.
Well I just tried, while writing those lines:
struct {
union {
struct {
int a;
char b[];
};
struct {
char x[4];
};
};
} u = { { { 1, "test" } } };
=>
.file "t.c"
.text
.globl u
.data
.align 4
.type u, @object
.size u, 4
u:
.long 1
.string "test"
.ident "GCC: (GNU) 9.0.0 20180825 (experimental)"
.section.note.GNU-stack,"",@progbits
So the .size is too small but that is probably only a fauxpas.
>
>> diff -Npur gcc/cp/decl.c gcc/cp/decl.c
>> --- gcc/cp/decl.c2018-08-22 22:35:38.0 +0200
>> +++ gcc/cp/decl.c2018-08-24 12:06:21.377892252 +0200
>> @@ -6528,6 +6528,8 @@ check_initializer (tree decl, tree init,
>>
>>init_code = store_init_value (decl, init, cleanups, flags);
>>
>> + complete_flexible_array_elts (DECL_INITIAL (decl));
>> +
>>if (pedantic && TREE_CODE (type) == ARRAY_TYPE
>>&& DECL_INITIAL (decl)
>>&& TREE_CODE (DECL_INITIAL (decl)) == STRING_CST
> Should the C++ front-end be going through cp_complete_array_type instead?
>
No I don't think so, because at that time BRACE_ENCLOSED_INITIALIZER_P
property does no longer work, as I explained in the previous mail.
And cp_complete_array_type does use that property:
int
cp_complete_array_type (tree *ptype, tree initial_value, bool do_default)
{
int failure;
tree type, elt_type;
/* Don't get confused by a CONSTRUCTOR for some other type. */
if (initial_value && TREE_CODE (initial_value) == CONSTRUCTOR
&& !BRACE_ENCLOSED_INITIALIZER_P (initial_value)
&& TREE_CODE (TREE_TYPE (initial_value)) != ARRAY_TYPE)
return 1;
But I need to do that completion for STRING_CST and CONSTRUCTORS
initializing a
Re: [PATCH] Use complete_array_type on flexible array member initializers
On 08/24/2018 07:13 AM, Bernd Edlinger wrote:
> Hi!
>
>
> This patch prevents init values of STRING_CST and braced
> array initializers to reach the middle-end with incomplete
> type.
>
> This will allow further simplifications in the middle-end,
> and address existing issues with STRING_CST in a correct
> way.
>
>
>
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
>
>
> Thanks
> Bernd.
>
>
> patch-flexarray.diff
>
>
> gcc:
> 2018-08-24 Bernd Edlinger
>
> * varasm.c (output_constructor_regular_field): Check TYPE_SIZE_UNIT of
> the init value.
>
> c-family:
> 2018-08-24 Bernd Edlinger
>
> * c-common.c (complete_flexible_array_elts): New helper function.
> * c-common.h (complete_flexible_array_elts): Declare.
>
> c:
> 2018-08-24 Bernd Edlinger
>
> * c-decl.c (finish_decl): Call complete_flexible_array_elts.
>
> cp:
> 2018-08-24 Bernd Edlinger
>
> * decl.c (check_initializer): Call complete_flexible_array_elts.
>
>
> diff -Npur gcc/c/c-decl.c gcc/c/c-decl.c
> --- gcc/c/c-decl.c2018-08-21 08:17:41.0 +0200
> +++ gcc/c/c-decl.c2018-08-24 12:06:21.374892294 +0200
> @@ -5035,6 +5035,8 @@ finish_decl (tree decl, location_t init_
>if (init && TREE_CODE (init) == CONSTRUCTOR)
> add_flexible_array_elts_to_size (decl, init);
>
> + complete_flexible_array_elts (DECL_INITIAL (decl));
> +
>if (DECL_SIZE (decl) == NULL_TREE && TREE_TYPE (decl) !=
> error_mark_node
> && COMPLETE_TYPE_P (TREE_TYPE (decl)))
> layout_decl (decl, 0);
> diff -Npur gcc/c-family/c-common.c gcc/c-family/c-common.c
> --- gcc/c-family/c-common.c 2018-08-17 05:02:11.0 +0200
> +++ gcc/c-family/c-common.c 2018-08-24 12:45:56.559011703 +0200
> @@ -6427,6 +6427,28 @@ complete_array_type (tree *ptype, tree i
>return failure;
> }
>
> +/* INIT is an constructor of a structure with a flexible array member.
> + Complete the flexible array member with a domain based on it's value. */
> +void
> +complete_flexible_array_elts (tree init)
> +{
> + tree elt, type;
> +
> + if (init == NULL_TREE || TREE_CODE (init) != CONSTRUCTOR)
> +return;
> +
> + if (vec_safe_is_empty (CONSTRUCTOR_ELTS (init)))
> +return;
> +
> + elt = CONSTRUCTOR_ELTS (init)->last ().value;
> + type = TREE_TYPE (elt);
> + if (TREE_CODE (type) == ARRAY_TYPE
> + && TYPE_SIZE (type) == NULL_TREE)
> +complete_array_type (_TYPE (elt), elt, false);
> + else
> +complete_flexible_array_elts (elt);
> +}
Shouldn't this be handled in c-decl.c by the call to
add_flexible_array_elts_to_size?Why the recursion when the
CONSTRUCTOR_ELT isn't an array type?
> diff -Npur gcc/cp/decl.c gcc/cp/decl.c
> --- gcc/cp/decl.c 2018-08-22 22:35:38.0 +0200
> +++ gcc/cp/decl.c 2018-08-24 12:06:21.377892252 +0200
> @@ -6528,6 +6528,8 @@ check_initializer (tree decl, tree init,
>
> init_code = store_init_value (decl, init, cleanups, flags);
>
> + complete_flexible_array_elts (DECL_INITIAL (decl));
> +
> if (pedantic && TREE_CODE (type) == ARRAY_TYPE
> && DECL_INITIAL (decl)
> && TREE_CODE (DECL_INITIAL (decl)) == STRING_CST
Should the C++ front-end be going through cp_complete_array_type instead?
Jeff
[PATCH] Use complete_array_type on flexible array member initializers
Hi!
This patch prevents init values of STRING_CST and braced
array initializers to reach the middle-end with incomplete
type.
This will allow further simplifications in the middle-end,
and address existing issues with STRING_CST in a correct
way.
Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?
Thanks
Bernd.
gcc:
2018-08-24 Bernd Edlinger
* varasm.c (output_constructor_regular_field): Check TYPE_SIZE_UNIT of
the init value.
c-family:
2018-08-24 Bernd Edlinger
* c-common.c (complete_flexible_array_elts): New helper function.
* c-common.h (complete_flexible_array_elts): Declare.
c:
2018-08-24 Bernd Edlinger
* c-decl.c (finish_decl): Call complete_flexible_array_elts.
cp:
2018-08-24 Bernd Edlinger
* decl.c (check_initializer): Call complete_flexible_array_elts.
diff -Npur gcc/c/c-decl.c gcc/c/c-decl.c
--- gcc/c/c-decl.c 2018-08-21 08:17:41.0 +0200
+++ gcc/c/c-decl.c 2018-08-24 12:06:21.374892294 +0200
@@ -5035,6 +5035,8 @@ finish_decl (tree decl, location_t init_
if (init && TREE_CODE (init) == CONSTRUCTOR)
add_flexible_array_elts_to_size (decl, init);
+ complete_flexible_array_elts (DECL_INITIAL (decl));
+
if (DECL_SIZE (decl) == NULL_TREE && TREE_TYPE (decl) != error_mark_node
&& COMPLETE_TYPE_P (TREE_TYPE (decl)))
layout_decl (decl, 0);
diff -Npur gcc/c-family/c-common.c gcc/c-family/c-common.c
--- gcc/c-family/c-common.c 2018-08-17 05:02:11.0 +0200
+++ gcc/c-family/c-common.c 2018-08-24 12:45:56.559011703 +0200
@@ -6427,6 +6427,28 @@ complete_array_type (tree *ptype, tree i
return failure;
}
+/* INIT is an constructor of a structure with a flexible array member.
+ Complete the flexible array member with a domain based on it's value. */
+void
+complete_flexible_array_elts (tree init)
+{
+ tree elt, type;
+
+ if (init == NULL_TREE || TREE_CODE (init) != CONSTRUCTOR)
+return;
+
+ if (vec_safe_is_empty (CONSTRUCTOR_ELTS (init)))
+return;
+
+ elt = CONSTRUCTOR_ELTS (init)->last ().value;
+ type = TREE_TYPE (elt);
+ if (TREE_CODE (type) == ARRAY_TYPE
+ && TYPE_SIZE (type) == NULL_TREE)
+complete_array_type (_TYPE (elt), elt, false);
+ else
+complete_flexible_array_elts (elt);
+}
+
/* Like c_mark_addressable but don't check register qualifier. */
void
c_common_mark_addressable_vec (tree t)
diff -Npur gcc/c-family/c-common.h gcc/c-family/c-common.h
--- gcc/c-family/c-common.h 2018-08-17 05:02:11.0 +0200
+++ gcc/c-family/c-common.h 2018-08-24 12:06:21.375892280 +0200
@@ -1038,6 +1038,7 @@ extern tree fold_offsetof (tree, tree =
tree_code ctx = ERROR_MARK);
extern int complete_array_type (tree *, tree, bool);
+extern void complete_flexible_array_elts (tree);
extern tree builtin_type_for_size (int, bool);
diff -Npur gcc/cp/decl.c gcc/cp/decl.c
--- gcc/cp/decl.c 2018-08-22 22:35:38.0 +0200
+++ gcc/cp/decl.c 2018-08-24 12:06:21.377892252 +0200
@@ -6528,6 +6528,8 @@ check_initializer (tree decl, tree init,
init_code = store_init_value (decl, init, cleanups, flags);
+ complete_flexible_array_elts (DECL_INITIAL (decl));
+
if (pedantic && TREE_CODE (type) == ARRAY_TYPE
&& DECL_INITIAL (decl)
&& TREE_CODE (DECL_INITIAL (decl)) == STRING_CST
diff -Npur gcc/varasm.c gcc/varasm.c
--- gcc/varasm.c 2018-08-16 17:28:11.0 +0200
+++ gcc/varasm.c 2018-08-24 12:06:21.378892238 +0200
@@ -5161,6 +5161,8 @@ output_constructor_regular_field (oc_loc
on the chain is a TYPE_DECL of the enclosing struct. */
const_tree next = DECL_CHAIN (local->field);
gcc_assert (!fieldsize || !next || TREE_CODE (next) != FIELD_DECL);
+ tree size = TYPE_SIZE_UNIT (TREE_TYPE (local->val));
+ gcc_checking_assert (compare_tree_int (size, fieldsize) == 0);
}
else
fieldsize = tree_to_uhwi (DECL_SIZE_UNIT (local->field));
