* [PATCH] Use complete_array_type on flexible array member initializers
@ 2018-08-24 13:13 Bernd Edlinger
2018-08-26 5:36 ` Jeff Law
0 siblings, 1 reply; 16+ messages in thread
From: Bernd Edlinger @ 2018-08-24 13:13 UTC (permalink / raw)
To: gcc-patches, Richard Biener, Joseph Myers, Nathan Sidwell, Jason Merrill
[-- Attachment #1: Type: text/plain, Size: 360 bytes --]
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.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-flexarray.diff --]
[-- Type: text/x-patch; name="patch-flexarray.diff", Size: 3751 bytes --]
gcc:
2018-08-24 Bernd Edlinger <bernd.edlinger@hotmail.de>
* varasm.c (output_constructor_regular_field): Check TYPE_SIZE_UNIT of
the init value.
c-family:
2018-08-24 Bernd Edlinger <bernd.edlinger@hotmail.de>
* c-common.c (complete_flexible_array_elts): New helper function.
* c-common.h (complete_flexible_array_elts): Declare.
c:
2018-08-24 Bernd Edlinger <bernd.edlinger@hotmail.de>
* c-decl.c (finish_decl): Call complete_flexible_array_elts.
cp:
2018-08-24 Bernd Edlinger <bernd.edlinger@hotmail.de>
* 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.000000000 +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.000000000 +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 (&TREE_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.000000000 +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.000000000 +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.000000000 +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));
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Use complete_array_type on flexible array member initializers
2018-08-24 13:13 [PATCH] Use complete_array_type on flexible array member initializers Bernd Edlinger
@ 2018-08-26 5:36 ` Jeff Law
2018-08-26 8:52 ` Bernd Edlinger
0 siblings, 1 reply; 16+ messages in thread
From: Jeff Law @ 2018-08-26 5:36 UTC (permalink / raw)
To: Bernd Edlinger, gcc-patches, Richard Biener, Joseph Myers,
Nathan Sidwell, Jason Merrill
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 <bernd.edlinger@hotmail.de>
>
> * varasm.c (output_constructor_regular_field): Check TYPE_SIZE_UNIT of
> the init value.
>
> c-family:
> 2018-08-24 Bernd Edlinger <bernd.edlinger@hotmail.de>
>
> * c-common.c (complete_flexible_array_elts): New helper function.
> * c-common.h (complete_flexible_array_elts): Declare.
>
> c:
> 2018-08-24 Bernd Edlinger <bernd.edlinger@hotmail.de>
>
> * c-decl.c (finish_decl): Call complete_flexible_array_elts.
>
> cp:
> 2018-08-24 Bernd Edlinger <bernd.edlinger@hotmail.de>
>
> * 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.000000000 +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.000000000 +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 (&TREE_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.000000000 +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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Use complete_array_type on flexible array member initializers
2018-08-26 5:36 ` Jeff Law
@ 2018-08-26 8:52 ` Bernd Edlinger
2018-08-29 21:11 ` Jason Merrill
2018-09-03 3:44 ` Jeff Law
0 siblings, 2 replies; 16+ messages in thread
From: Bernd Edlinger @ 2018-08-26 8:52 UTC (permalink / raw)
To: Jeff Law, gcc-patches, Richard Biener, Joseph Myers,
Nathan Sidwell, Jason Merrill
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 <bernd.edlinger@hotmail.de>
>>
>> * varasm.c (output_constructor_regular_field): Check TYPE_SIZE_UNIT of
>> the init value.
>>
>> c-family:
>> 2018-08-24 Bernd Edlinger <bernd.edlinger@hotmail.de>
>>
>> * c-common.c (complete_flexible_array_elts): New helper function.
>> * c-common.h (complete_flexible_array_elts): Declare.
>>
>> c:
>> 2018-08-24 Bernd Edlinger <bernd.edlinger@hotmail.de>
>>
>> * c-decl.c (finish_decl): Call complete_flexible_array_elts.
>>
>> cp:
>> 2018-08-24 Bernd Edlinger <bernd.edlinger@hotmail.de>
>>
>> * 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.000000000 +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.000000000 +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 (&TREE_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.c 2018-08-22 22:35:38.000000000 +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?
>
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 flexible array, of a structure of a union
within a structure.
I tried to come up with a test case where the cp_complete_array_type
might make a difference (looking into string constant with extra braces),
but it worked:
$ cat test.cc
struct {
int a;
char b[];
} xx = { 1, { "test" } };
$ cat test.cc
struct {
union {
struct {
int a;
char b[];
};
struct {
char c[32];
};
};
} xx = { 1, "test" };
$ gcc test.cc
test.cc:11:21: error: initialization of flexible array member in a nested context
11 | } xx = { 1, "test" };
| ^
So since this recursive thing appears to be disallowed in C++ it would allow to use
cp_complete_array_type without the recursion.
But for consistency I would stay with complete_flexible_array_elts, even for C++.
However if you like it better, I am ready to change that hunk to use cp_complete_array_type.
Thanks
Bernd.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Use complete_array_type on flexible array member initializers
2018-08-26 8:52 ` Bernd Edlinger
@ 2018-08-29 21:11 ` Jason Merrill
2018-09-03 3:44 ` Jeff Law
1 sibling, 0 replies; 16+ messages in thread
From: Jason Merrill @ 2018-08-29 21:11 UTC (permalink / raw)
To: Bernd Edlinger
Cc: Jeff Law, gcc-patches, Richard Biener, Joseph Myers, Nathan Sidwell
On Sun, Aug 26, 2018 at 4:52 AM, Bernd Edlinger
<bernd.edlinger@hotmail.de> 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 <bernd.edlinger@hotmail.de>
>>>
>>> * varasm.c (output_constructor_regular_field): Check TYPE_SIZE_UNIT of
>>> the init value.
>>>
>>> c-family:
>>> 2018-08-24 Bernd Edlinger <bernd.edlinger@hotmail.de>
>>>
>>> * c-common.c (complete_flexible_array_elts): New helper function.
>>> * c-common.h (complete_flexible_array_elts): Declare.
>>>
>>> c:
>>> 2018-08-24 Bernd Edlinger <bernd.edlinger@hotmail.de>
>>>
>>> * c-decl.c (finish_decl): Call complete_flexible_array_elts.
>>>
>>> cp:
>>> 2018-08-24 Bernd Edlinger <bernd.edlinger@hotmail.de>
>>>
>>> * 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.000000000 +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.000000000 +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 (&TREE_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.c 2018-08-22 22:35:38.000000000 +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?
>>
>
> 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 flexible array, of a structure of a union
> within a structure.
That check only returns early on CONSTRUCTORs for a non-array type,
which I assume you don't care about.
> I tried to come up with a test case where the cp_complete_array_type
> might make a difference (looking into string constant with extra braces),
> but it worked:
>
>
> $ cat test.cc
> struct {
> int a;
> char b[];
> } xx = { 1, { "test" } };
>
> $ cat test.cc
> struct {
> union {
> struct {
> int a;
> char b[];
> };
> struct {
> char c[32];
> };
> };
> } xx = { 1, "test" };
> $ gcc test.cc
> test.cc:11:21: error: initialization of flexible array member in a nested context
> 11 | } xx = { 1, "test" };
> | ^
The extra braces that code handles are like
struct A
{
int i;
char a[];
};
struct A a = { 1, { "test" } };
...but by the time you're calling complete_flexible_array_elts those
extra braces have already been removed by reshape_init.
The flag copying at the end of cp_complete_array_type is likely to be
important, though, so let's use it.
Jason
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Use complete_array_type on flexible array member initializers
2018-08-26 8:52 ` Bernd Edlinger
2018-08-29 21:11 ` Jason Merrill
@ 2018-09-03 3:44 ` Jeff Law
2018-09-03 12:35 ` Bernd Edlinger
1 sibling, 1 reply; 16+ messages in thread
From: Jeff Law @ 2018-09-03 3:44 UTC (permalink / raw)
To: Bernd Edlinger, gcc-patches, Richard Biener, Joseph Myers,
Nathan Sidwell, Jason Merrill
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 <bernd.edlinger@hotmail.de>
>>>
>>> * varasm.c (output_constructor_regular_field): Check TYPE_SIZE_UNIT of
>>> the init value.
>>>
>>> c-family:
>>> 2018-08-24 Bernd Edlinger <bernd.edlinger@hotmail.de>
>>>
>>> * c-common.c (complete_flexible_array_elts): New helper function.
>>> * c-common.h (complete_flexible_array_elts): Declare.
>>>
>>> c:
>>> 2018-08-24 Bernd Edlinger <bernd.edlinger@hotmail.de>
>>>
>>> * c-decl.c (finish_decl): Call complete_flexible_array_elts.
>>>
>>> cp:
>>> 2018-08-24 Bernd Edlinger <bernd.edlinger@hotmail.de>
>>>
>>> * 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.000000000 +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.000000000 +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 (&TREE_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=0x7ffff7ff6ab0,
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)
<var_decl 0x7ffff7ff6ab0 u
type <record_type 0x7fffe9f35498 type_0 BLK
size <integer_cst 0x7fffe9e2c000 constant 256>
unit-size <integer_cst 0x7fffe9e2c0f0 constant 32>
align:32 warn_if_not_align:0 symtab:0 alias-set -1
canonical-type 0x7fffe9f35498
fields <field_decl 0x7fffe9e357b8 D.1914 type <union_type
0x7fffe9f35540>
BLK j.c:10:4 size <integer_cst 0x7fffe9e2c000 256> unit-size
<integer_cst 0x7fffe9e2c0f0 32>
align:32 warn_if_not_align:0 offset_align 128
offset <integer_cst 0x7fffe9e0dcc0 constant 0>
bit-offset <integer_cst 0x7fffe9e0dd08 constant 0> context
<record_type 0x7fffe9f35498>>
chain <type_decl 0x7fffe9e35260 D.1905>>
public static BLK j.c:11:3 size <integer_cst 0x7fffe9e2c000 256>
unit-size <integer_cst 0x7fffe9e2c0f0 32>
align:32 warn_if_not_align:0 initial <constructor 0x7fffe9f3edc8>>
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 20180825 (experimental)"
> .section .note.GNU-stack,"",@progbits
>
> So the .size is too small but that is probably only a fauxpas.
Looks wrong to me as well. But I don't think that's the core issue
here. Let's tackle that separately.
>
>
>>
>>> diff -Npur gcc/cp/decl.c gcc/cp/decl.c
>>> --- gcc/cp/decl.c 2018-08-22 22:35:38.000000000 +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?
>>
>
> 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.
Jeff
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Use complete_array_type on flexible array member initializers
2018-09-03 3:44 ` Jeff Law
@ 2018-09-03 12:35 ` Bernd Edlinger
2018-09-04 14:31 ` Jeff Law
0 siblings, 1 reply; 16+ messages in thread
From: Bernd Edlinger @ 2018-09-03 12:35 UTC (permalink / raw)
To: Jeff Law, gcc-patches, Richard Biener, Joseph Myers,
Nathan Sidwell, Jason Merrill
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 <bernd.edlinger@hotmail.de>
>>>
>>>> * varasm.c (output_constructor_regular_field): Check TYPE_SIZE_UNIT of
>>>> the init value.
>>>>
>>> c-family:
>>>> 2018-08-24 Bernd Edlinger <bernd.edlinger@hotmail.de>
>>>>
>>>> * c-common.c (complete_flexible_array_elts): New helper function.
>>>> * c-common.h (complete_flexible_array_elts): Declare.
>>>>
>>>> c:
>>>> 2018-08-24 Bernd Edlinger <bernd.edlinger@hotmail.de>
>>>>
>>>> * c-decl.c (finish_decl): Call complete_flexible_array_elts.
>>>>
>>>> cp:
>>>> 2018-08-24 Bernd Edlinger <bernd.edlinger@hotmail.de>
>>>>
>>>> * 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.000000000 +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.000000000 +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 (&TREE_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=0x7ffff7ff6ab0,
> 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)
> <var_decl 0x7ffff7ff6ab0 u
> type <record_type 0x7fffe9f35498 type_0 BLK
> size <integer_cst 0x7fffe9e2c000 constant 256>
> unit-size <integer_cst 0x7fffe9e2c0f0 constant 32>
> align:32 warn_if_not_align:0 symtab:0 alias-set -1
> canonical-type 0x7fffe9f35498
> fields <field_decl 0x7fffe9e357b8 D.1914 type <union_type
> 0x7fffe9f35540>
> BLK j.c:10:4 size <integer_cst 0x7fffe9e2c000 256> unit-size
> <integer_cst 0x7fffe9e2c0f0 32>
> align:32 warn_if_not_align:0 offset_align 128
> offset <integer_cst 0x7fffe9e0dcc0 constant 0>
> bit-offset <integer_cst 0x7fffe9e0dd08 constant 0> context
> <record_type 0x7fffe9f35498>>
> chain <type_decl 0x7fffe9e35260 D.1905>>
> public static BLK j.c:11:3 size <integer_cst 0x7fffe9e2c000 256>
> unit-size <integer_cst 0x7fffe9e2c0f0 32>
> align:32 warn_if_not_align:0 initial <constructor 0x7fffe9f3edc8>>
>
>
>
> 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 this:
By making the array type complete we destroy the information, that
the array type was incomplete, which may be used somewhere else
in the front-end, therefore I want to do that as late as possible.
>
>> 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.
> Looks wrong to me as well. But I don't think that's the core issue
> here. Let's tackle that separately.
>
>
>>
>>
>>
>>>> diff -Npur gcc/cp/decl.c gcc/cp/decl.c
>>>> --- gcc/cp/decl.c 2018-08-22 22:35:38.000000000 +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?
>>>
>>
>> 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.
Bernd.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Use complete_array_type on flexible array member initializers
2018-09-03 12:35 ` Bernd Edlinger
@ 2018-09-04 14:31 ` Jeff Law
2018-09-06 11:05 ` Bernd Edlinger
0 siblings, 1 reply; 16+ messages in thread
From: Jeff Law @ 2018-09-04 14:31 UTC (permalink / raw)
To: Bernd Edlinger, gcc-patches, Richard Biener, Joseph Myers,
Nathan Sidwell, Jason Merrill
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Use complete_array_type on flexible array member initializers
2018-09-04 14:31 ` Jeff Law
@ 2018-09-06 11:05 ` Bernd Edlinger
2018-09-06 15:44 ` Jeff Law
0 siblings, 1 reply; 16+ messages in thread
From: Bernd Edlinger @ 2018-09-06 11:05 UTC (permalink / raw)
To: Jeff Law, gcc-patches, Richard Biener, Joseph Myers,
Nathan Sidwell, Jason Merrill
[-- Attachment #1: Type: text/plain, Size: 1447 bytes --]
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.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-flexarray.diff --]
[-- Type: text/x-patch; name="patch-flexarray.diff", Size: 4114 bytes --]
gcc:
2018-08-30 Bernd Edlinger <bernd.edlinger@hotmail.de>
* varasm.c (output_constructor_regular_field): Check TYPE_SIZE_UNIT of
the init value.
c-family:
2018-08-30 Bernd Edlinger <bernd.edlinger@hotmail.de>
* c-common.c (complete_flexible_array_elts): New helper function.
* c-common.h (complete_flexible_array_elts): Declare.
c:
2018-08-30 Bernd Edlinger <bernd.edlinger@hotmail.de>
* c-decl.c (finish_decl): Call complete_flexible_array_elts.
cp:
2018-08-30 Bernd Edlinger <bernd.edlinger@hotmail.de>
* 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.000000000 +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.000000000 +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 (&TREE_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.000000000 +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.000000000 +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 (&TREE_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.000000000 +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));
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Use complete_array_type on flexible array member initializers
2018-09-06 11:05 ` Bernd Edlinger
@ 2018-09-06 15:44 ` Jeff Law
2018-09-06 17:12 ` Bernd Edlinger
0 siblings, 1 reply; 16+ messages in thread
From: Jeff Law @ 2018-09-06 15:44 UTC (permalink / raw)
To: Bernd Edlinger, gcc-patches, Richard Biener, Joseph Myers,
Nathan Sidwell, Jason Merrill
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 <bernd.edlinger@hotmail.de>
>
> * varasm.c (output_constructor_regular_field): Check TYPE_SIZE_UNIT of
> the init value.
>
> c-family:
> 2018-08-30 Bernd Edlinger <bernd.edlinger@hotmail.de>
>
> * c-common.c (complete_flexible_array_elts): New helper function.
> * c-common.h (complete_flexible_array_elts): Declare.
>
> c:
> 2018-08-30 Bernd Edlinger <bernd.edlinger@hotmail.de>
>
> * c-decl.c (finish_decl): Call complete_flexible_array_elts.
>
> cp:
> 2018-08-30 Bernd Edlinger <bernd.edlinger@hotmail.de>
>
> * 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Use complete_array_type on flexible array member initializers
2018-09-06 15:44 ` Jeff Law
@ 2018-09-06 17:12 ` Bernd Edlinger
2018-09-06 22:01 ` Jeff Law
0 siblings, 1 reply; 16+ messages in thread
From: Bernd Edlinger @ 2018-09-06 17:12 UTC (permalink / raw)
To: Jeff Law, gcc-patches, Richard Biener, Joseph Myers,
Nathan Sidwell, Jason Merrill
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 <bernd.edlinger@hotmail.de>
>>
>> * varasm.c (output_constructor_regular_field): Check TYPE_SIZE_UNIT of
>> the init value.
>>
>> c-family:
>> 2018-08-30 Bernd Edlinger <bernd.edlinger@hotmail.de>
>>
>> * c-common.c (complete_flexible_array_elts): New helper function.
>> * c-common.h (complete_flexible_array_elts): Declare.
>>
>> c:
>> 2018-08-30 Bernd Edlinger <bernd.edlinger@hotmail.de>
>>
>> * c-decl.c (finish_decl): Call complete_flexible_array_elts.
>>
>> cp:
>> 2018-08-30 Bernd Edlinger <bernd.edlinger@hotmail.de>
>>
>> * 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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Use complete_array_type on flexible array member initializers
2018-09-06 17:12 ` Bernd Edlinger
@ 2018-09-06 22:01 ` Jeff Law
2018-09-06 22:16 ` Jeff Law
0 siblings, 1 reply; 16+ messages in thread
From: Jeff Law @ 2018-09-06 22:01 UTC (permalink / raw)
To: Bernd Edlinger, gcc-patches, Richard Biener, Joseph Myers,
Nathan Sidwell, Jason Merrill
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Use complete_array_type on flexible array member initializers
2018-09-06 22:01 ` Jeff Law
@ 2018-09-06 22:16 ` Jeff Law
2018-09-06 22:26 ` Jeff Law
0 siblings, 1 reply; 16+ messages in thread
From: Jeff Law @ 2018-09-06 22:16 UTC (permalink / raw)
To: Bernd Edlinger, gcc-patches, Richard Biener, Joseph Myers,
Nathan Sidwell, Jason Merrill
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Use complete_array_type on flexible array member initializers
2018-09-06 22:16 ` Jeff Law
@ 2018-09-06 22:26 ` Jeff Law
2018-09-07 6:51 ` Bernd Edlinger
0 siblings, 1 reply; 16+ messages in thread
From: Jeff Law @ 2018-09-06 22:26 UTC (permalink / raw)
To: Bernd Edlinger, gcc-patches, Richard Biener, Joseph Myers,
Nathan Sidwell, Jason Merrill
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Use complete_array_type on flexible array member initializers
2018-09-06 22:26 ` Jeff Law
@ 2018-09-07 6:51 ` Bernd Edlinger
2018-09-07 13:36 ` Bernd Edlinger
0 siblings, 1 reply; 16+ messages in thread
From: Bernd Edlinger @ 2018-09-07 6:51 UTC (permalink / raw)
To: Jeff Law, gcc-patches, Richard Biener, Joseph Myers,
Nathan Sidwell, Jason Merrill
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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Use complete_array_type on flexible array member initializers
2018-09-07 6:51 ` Bernd Edlinger
@ 2018-09-07 13:36 ` Bernd Edlinger
2018-09-07 17:44 ` Bernd Edlinger
0 siblings, 1 reply; 16+ messages in thread
From: Bernd Edlinger @ 2018-09-07 13:36 UTC (permalink / raw)
To: Jeff Law, gcc-patches, Richard Biener, Joseph Myers,
Nathan Sidwell, Jason Merrill
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.
Bernd.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Use complete_array_type on flexible array member initializers
2018-09-07 13:36 ` Bernd Edlinger
@ 2018-09-07 17:44 ` Bernd Edlinger
0 siblings, 0 replies; 16+ messages in thread
From: Bernd Edlinger @ 2018-09-07 17:44 UTC (permalink / raw)
To: Jeff Law, gcc-patches, Richard Biener, Joseph Myers,
Nathan Sidwell, Jason Merrill
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.c 2018-09-07 19:39:19.555588785 +0200
+++ gcc/fold-const.c 2018-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.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2018-09-07 17:44 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-24 13:13 [PATCH] Use complete_array_type on flexible array member initializers Bernd Edlinger
2018-08-26 5:36 ` Jeff Law
2018-08-26 8:52 ` Bernd Edlinger
2018-08-29 21:11 ` Jason Merrill
2018-09-03 3:44 ` Jeff Law
2018-09-03 12:35 ` Bernd Edlinger
2018-09-04 14:31 ` Jeff Law
2018-09-06 11:05 ` Bernd Edlinger
2018-09-06 15:44 ` Jeff Law
2018-09-06 17:12 ` Bernd Edlinger
2018-09-06 22:01 ` Jeff Law
2018-09-06 22:16 ` Jeff Law
2018-09-06 22:26 ` Jeff Law
2018-09-07 6:51 ` Bernd Edlinger
2018-09-07 13:36 ` Bernd Edlinger
2018-09-07 17:44 ` Bernd Edlinger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).