* [PATCH] Update array address space in c_build_qualified_type
@ 2023-06-21 5:57 SenthilKumar.Selvaraj
2023-06-21 8:15 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: SenthilKumar.Selvaraj @ 2023-06-21 5:57 UTC (permalink / raw)
To: gcc-patches; +Cc: richard.guenther
Hi,
When c-typeck.cc:c_build_qualified_type builds an array type
from its element type, it does not copy the address space of
the element type to the array type itself. This is unlike
tree.cc:build_array_type_1, which explicitly does
TYPE_ADDR_SPACE (t) = TYPE_ADDR_SPACE (elt_type);
This causes the ICE described in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86869.
struct S {
char y[2];
};
extern const __memx struct S *s;
extern void bar(const __memx void*);
void foo(void) {
bar(&s->y);
}
build_component_ref calls c_build_qualified_type, passing in the
array type and quals including the address space (ADDR_SPACE_MEMX
in this case). Because of this missing address space copy, the
returned array type remains in the generic address space. Later
down the line, expand_expr_addr_expr detects the mismatch in
address space/mode and tries to convert, and that leads to the
ICE described in the bug.
This patch sets the address space of the array type to that of the
element type.
Regression tests for avr look ok. Ok for trunk?
Regards
Senthil
PR 86869
gcc/c/ChangeLog:
* c-typeck.cc (c_build_qualified_type): Set
TYPE_ADDR_SPACE for ARRAY_TYPE.
gcc/testsuite/ChangeLog:
* gcc.target/avr/pr86869.c: New test.
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index 22e240a3c2a..d4ab1d1bd46 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -16284,6 +16284,7 @@ c_build_qualified_type (tree type, int type_quals, tree orig_qual_type,
t = build_variant_type_copy (type);
TREE_TYPE (t) = element_type;
+ TYPE_ADDR_SPACE (t) = TYPE_ADDR_SPACE (element_type);
if (TYPE_STRUCTURAL_EQUALITY_P (element_type)
|| (domain && TYPE_STRUCTURAL_EQUALITY_P (domain)))
diff --git a/gcc/testsuite/gcc.target/avr/pr86869.c b/gcc/testsuite/gcc.target/avr/pr86869.c
new file mode 100644
index 00000000000..54cd984276e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/avr/pr86869.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-std=gnu99" } */
+
+extern void bar(const __memx void* p);
+
+struct S {
+ char y[2];
+};
+extern const __memx struct S *s;
+
+void foo(void) {
+ bar(&s->y);
+}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Update array address space in c_build_qualified_type
2023-06-21 5:57 [PATCH] Update array address space in c_build_qualified_type SenthilKumar.Selvaraj
@ 2023-06-21 8:15 ` Richard Biener
2023-06-21 18:37 ` Joseph Myers
0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2023-06-21 8:15 UTC (permalink / raw)
To: SenthilKumar.Selvaraj, Joseph S. Myers; +Cc: gcc-patches
On Wed, Jun 21, 2023 at 7:57 AM <SenthilKumar.Selvaraj@microchip.com> wrote:
>
> Hi,
>
> When c-typeck.cc:c_build_qualified_type builds an array type
> from its element type, it does not copy the address space of
> the element type to the array type itself. This is unlike
> tree.cc:build_array_type_1, which explicitly does
>
> TYPE_ADDR_SPACE (t) = TYPE_ADDR_SPACE (elt_type);
>
> This causes the ICE described in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86869.
>
> struct S {
> char y[2];
> };
>
> extern const __memx struct S *s;
>
> extern void bar(const __memx void*);
>
> void foo(void) {
> bar(&s->y);
> }
>
> build_component_ref calls c_build_qualified_type, passing in the
> array type and quals including the address space (ADDR_SPACE_MEMX
> in this case). Because of this missing address space copy, the
> returned array type remains in the generic address space. Later
> down the line, expand_expr_addr_expr detects the mismatch in
> address space/mode and tries to convert, and that leads to the
> ICE described in the bug.
>
> This patch sets the address space of the array type to that of the
> element type.
>
> Regression tests for avr look ok. Ok for trunk?
The patch looks OK to me but please let a C frontend maintainer
double-check (I've CCed Joseph for this).
Thanks,
Richard.
> Regards
> Senthil
>
> PR 86869
>
> gcc/c/ChangeLog:
>
> * c-typeck.cc (c_build_qualified_type): Set
> TYPE_ADDR_SPACE for ARRAY_TYPE.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/avr/pr86869.c: New test.
>
> diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
> index 22e240a3c2a..d4ab1d1bd46 100644
> --- a/gcc/c/c-typeck.cc
> +++ b/gcc/c/c-typeck.cc
> @@ -16284,6 +16284,7 @@ c_build_qualified_type (tree type, int type_quals, tree orig_qual_type,
>
> t = build_variant_type_copy (type);
> TREE_TYPE (t) = element_type;
> + TYPE_ADDR_SPACE (t) = TYPE_ADDR_SPACE (element_type);
>
> if (TYPE_STRUCTURAL_EQUALITY_P (element_type)
> || (domain && TYPE_STRUCTURAL_EQUALITY_P (domain)))
> diff --git a/gcc/testsuite/gcc.target/avr/pr86869.c b/gcc/testsuite/gcc.target/avr/pr86869.c
> new file mode 100644
> index 00000000000..54cd984276e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/avr/pr86869.c
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-options "-std=gnu99" } */
> +
> +extern void bar(const __memx void* p);
> +
> +struct S {
> + char y[2];
> +};
> +extern const __memx struct S *s;
> +
> +void foo(void) {
> + bar(&s->y);
> +}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Update array address space in c_build_qualified_type
2023-06-21 8:15 ` Richard Biener
@ 2023-06-21 18:37 ` Joseph Myers
2023-06-22 9:55 ` SenthilKumar.Selvaraj
0 siblings, 1 reply; 4+ messages in thread
From: Joseph Myers @ 2023-06-21 18:37 UTC (permalink / raw)
To: Richard Biener; +Cc: SenthilKumar.Selvaraj, gcc-patches
On Wed, 21 Jun 2023, Richard Biener via Gcc-patches wrote:
> > This patch sets the address space of the array type to that of the
> > element type.
> >
> > Regression tests for avr look ok. Ok for trunk?
>
> The patch looks OK to me but please let a C frontend maintainer
> double-check (I've CCed Joseph for this).
The question would be whether there are any TYPE_QUALS uses in the C front
end that behave incorrectly given TYPE_ADDR_SPACE (acting as qualifiers)
being set on an array type - conceptually, before C2x, array types are
unqualified, only the element types are qualified.
The fact that this changed in C2x gives a shortcut to doing that analysis
- you don't need to check all TYPE_QUALS uses in the front end, only a
limited number of places that might handle qualifiers on arrays that
already have conditionals to do things differently in C2x mode. But some
sort of analysis of those places, to see how they'd react to an array type
itself having TYPE_ADDR_SPACE set, would be helpful. If you're lucky, all
those places only care about TYPE_QUALS on the element type and not on the
array type itself.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Update array address space in c_build_qualified_type
2023-06-21 18:37 ` Joseph Myers
@ 2023-06-22 9:55 ` SenthilKumar.Selvaraj
0 siblings, 0 replies; 4+ messages in thread
From: SenthilKumar.Selvaraj @ 2023-06-22 9:55 UTC (permalink / raw)
To: joseph, richard.guenther; +Cc: gcc-patches
On Wed, 2023-06-21 at 18:37 +0000, Joseph Myers wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On Wed, 21 Jun 2023, Richard Biener via Gcc-patches wrote:
>
> > > This patch sets the address space of the array type to that of the
> > > element type.
> > >
> > > Regression tests for avr look ok. Ok for trunk?
> >
> > The patch looks OK to me but please let a C frontend maintainer
> > double-check (I've CCed Joseph for this).
>
> The question would be whether there are any TYPE_QUALS uses in the C front
> end that behave incorrectly given TYPE_ADDR_SPACE (acting as qualifiers)
> being set on an array type - conceptually, before C2x, array types are
> unqualified, only the element types are qualified.
Hmm, but tree.cc:build_array_type_1 sets the address space of the element
type to the array type, and the relevant commit's ChangeLog entry (from 2009)
says "Inherit array address space from element type."
On the avr target, for an array like
const __flash int arr[] = {1,2,3};
I can see that the array type gets the address space of the element type
already, even before this patch. Surely that must have caused any code that
doesn't expect address space in array type quals to be broken then?
Regards
Senthil
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-06-22 9:55 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-21 5:57 [PATCH] Update array address space in c_build_qualified_type SenthilKumar.Selvaraj
2023-06-21 8:15 ` Richard Biener
2023-06-21 18:37 ` Joseph Myers
2023-06-22 9:55 ` SenthilKumar.Selvaraj
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).