* [PATCH RFA] c-family: attribute ((aligned, mode)) [PR100545]
@ 2022-04-27 15:19 Jason Merrill
2022-04-27 16:58 ` Marek Polacek
2022-04-27 17:02 ` Joseph Myers
0 siblings, 2 replies; 6+ messages in thread
From: Jason Merrill @ 2022-04-27 15:19 UTC (permalink / raw)
To: gcc-patches
The problem here was that handle_mode_attribute clobbered the changes of any
previous attribute, only copying type qualifiers to the new type. And
common_handle_aligned_attribute had previously set up the typedef, so when
we later called set_underlying_type it saw DECL_ORIGINAL_TYPE set and just
returned, even though handle_mode_attribute had messed up the TREE_TYPE.
So, let's fix handle_mode_attribute to copy attributes, alignment, and
typedefness to the new type.
Tested x86_64-pc-linux-gnu, OK for trunk now or after 12.1?
PR c/100545
gcc/c-family/ChangeLog:
* c-attribs.cc (handle_mode_attribute): Copy attributes, aligned,
and typedef.
* c-common.cc (set_underlying_type): Add assert.
gcc/testsuite/ChangeLog:
* c-c++-common/attr-mode-1.c: New test.
---
gcc/c-family/c-attribs.cc | 15 ++++++++++++++-
gcc/c-family/c-common.cc | 7 ++++---
gcc/testsuite/c-c++-common/attr-mode-1.c | 3 +++
3 files changed, 21 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/attr-mode-1.c
diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
index 111a33f405a..26876d0f309 100644
--- a/gcc/c-family/c-attribs.cc
+++ b/gcc/c-family/c-attribs.cc
@@ -2199,7 +2199,20 @@ handle_mode_attribute (tree *node, tree name, tree args,
return NULL_TREE;
}
- *node = build_qualified_type (typefm, TYPE_QUALS (type));
+ /* Copy any quals and attributes to the new type. */
+ *node = build_type_attribute_qual_variant (typefm, TYPE_ATTRIBUTES (type),
+ TYPE_QUALS (type));
+ if (TYPE_USER_ALIGN (type))
+ *node = build_aligned_type (*node, TYPE_ALIGN (type));
+ if (typedef_variant_p (type))
+ {
+ /* Set up the typedef all over again. */
+ tree decl = TYPE_NAME (type);
+ DECL_ORIGINAL_TYPE (decl) = NULL_TREE;
+ TREE_TYPE (decl) = *node;
+ set_underlying_type (decl);
+ *node = TREE_TYPE (decl);
+ }
}
return NULL_TREE;
diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
index bb0544eeaea..730faa9e87f 100644
--- a/gcc/c-family/c-common.cc
+++ b/gcc/c-family/c-common.cc
@@ -8153,15 +8153,16 @@ check_missing_format_attribute (tree ltype, tree rtype)
void
set_underlying_type (tree x)
{
- if (x == error_mark_node)
+ if (x == error_mark_node || TREE_TYPE (x) == error_mark_node)
return;
if (DECL_IS_UNDECLARED_BUILTIN (x) && TREE_CODE (TREE_TYPE (x)) != ARRAY_TYPE)
{
if (TYPE_NAME (TREE_TYPE (x)) == 0)
TYPE_NAME (TREE_TYPE (x)) = x;
}
- else if (TREE_TYPE (x) != error_mark_node
- && DECL_ORIGINAL_TYPE (x) == NULL_TREE)
+ else if (DECL_ORIGINAL_TYPE (x))
+ gcc_checking_assert (TYPE_NAME (TREE_TYPE (x)) == x);
+ else
{
tree tt = TREE_TYPE (x);
DECL_ORIGINAL_TYPE (x) = tt;
diff --git a/gcc/testsuite/c-c++-common/attr-mode-1.c b/gcc/testsuite/c-c++-common/attr-mode-1.c
new file mode 100644
index 00000000000..59b20cd99e8
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/attr-mode-1.c
@@ -0,0 +1,3 @@
+// PR c/100545
+
+typedef int fatp_t __attribute__((aligned, mode(SI)));
base-commit: 6a460a2007dd9c527c5f9d5bbbedb852db7c1373
--
2.27.0
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH RFA] c-family: attribute ((aligned, mode)) [PR100545]
2022-04-27 15:19 [PATCH RFA] c-family: attribute ((aligned, mode)) [PR100545] Jason Merrill
@ 2022-04-27 16:58 ` Marek Polacek
2022-04-27 17:02 ` Joseph Myers
1 sibling, 0 replies; 6+ messages in thread
From: Marek Polacek @ 2022-04-27 16:58 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
On Wed, Apr 27, 2022 at 11:19:57AM -0400, Jason Merrill via Gcc-patches wrote:
> The problem here was that handle_mode_attribute clobbered the changes of any
> previous attribute, only copying type qualifiers to the new type. And
> common_handle_aligned_attribute had previously set up the typedef, so when
> we later called set_underlying_type it saw DECL_ORIGINAL_TYPE set and just
> returned, even though handle_mode_attribute had messed up the TREE_TYPE.
> So, let's fix handle_mode_attribute to copy attributes, alignment, and
> typedefness to the new type.
>
> Tested x86_64-pc-linux-gnu, OK for trunk now or after 12.1?
I think I'd slightly prefer 12.1, it doesn't seem to be a regression.
> PR c/100545
>
> gcc/c-family/ChangeLog:
>
> * c-attribs.cc (handle_mode_attribute): Copy attributes, aligned,
> and typedef.
> * c-common.cc (set_underlying_type): Add assert.
>
> gcc/testsuite/ChangeLog:
>
> * c-c++-common/attr-mode-1.c: New test.
> ---
> gcc/c-family/c-attribs.cc | 15 ++++++++++++++-
> gcc/c-family/c-common.cc | 7 ++++---
> gcc/testsuite/c-c++-common/attr-mode-1.c | 3 +++
> 3 files changed, 21 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/c-c++-common/attr-mode-1.c
>
> diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
> index 111a33f405a..26876d0f309 100644
> --- a/gcc/c-family/c-attribs.cc
> +++ b/gcc/c-family/c-attribs.cc
> @@ -2199,7 +2199,20 @@ handle_mode_attribute (tree *node, tree name, tree args,
> return NULL_TREE;
> }
>
> - *node = build_qualified_type (typefm, TYPE_QUALS (type));
> + /* Copy any quals and attributes to the new type. */
> + *node = build_type_attribute_qual_variant (typefm, TYPE_ATTRIBUTES (type),
> + TYPE_QUALS (type));
> + if (TYPE_USER_ALIGN (type))
> + *node = build_aligned_type (*node, TYPE_ALIGN (type));
> + if (typedef_variant_p (type))
> + {
> + /* Set up the typedef all over again. */
> + tree decl = TYPE_NAME (type);
> + DECL_ORIGINAL_TYPE (decl) = NULL_TREE;
> + TREE_TYPE (decl) = *node;
> + set_underlying_type (decl);
> + *node = TREE_TYPE (decl);
> + }
> }
>
> return NULL_TREE;
> diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
> index bb0544eeaea..730faa9e87f 100644
> --- a/gcc/c-family/c-common.cc
> +++ b/gcc/c-family/c-common.cc
> @@ -8153,15 +8153,16 @@ check_missing_format_attribute (tree ltype, tree rtype)
> void
> set_underlying_type (tree x)
> {
> - if (x == error_mark_node)
> + if (x == error_mark_node || TREE_TYPE (x) == error_mark_node)
Maybe error_operand_p?
> return;
> if (DECL_IS_UNDECLARED_BUILTIN (x) && TREE_CODE (TREE_TYPE (x)) != ARRAY_TYPE)
> {
> if (TYPE_NAME (TREE_TYPE (x)) == 0)
> TYPE_NAME (TREE_TYPE (x)) = x;
> }
> - else if (TREE_TYPE (x) != error_mark_node
> - && DECL_ORIGINAL_TYPE (x) == NULL_TREE)
> + else if (DECL_ORIGINAL_TYPE (x))
> + gcc_checking_assert (TYPE_NAME (TREE_TYPE (x)) == x);
> + else
> {
> tree tt = TREE_TYPE (x);
> DECL_ORIGINAL_TYPE (x) = tt;
> diff --git a/gcc/testsuite/c-c++-common/attr-mode-1.c b/gcc/testsuite/c-c++-common/attr-mode-1.c
> new file mode 100644
> index 00000000000..59b20cd99e8
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/attr-mode-1.c
> @@ -0,0 +1,3 @@
> +// PR c/100545
> +
> +typedef int fatp_t __attribute__((aligned, mode(SI)));
I think you need to add "dg-options -g", otherwise the bug doesn't
manifest.
Marek
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH RFA] c-family: attribute ((aligned, mode)) [PR100545]
2022-04-27 15:19 [PATCH RFA] c-family: attribute ((aligned, mode)) [PR100545] Jason Merrill
2022-04-27 16:58 ` Marek Polacek
@ 2022-04-27 17:02 ` Joseph Myers
2022-04-27 23:48 ` Jason Merrill
1 sibling, 1 reply; 6+ messages in thread
From: Joseph Myers @ 2022-04-27 17:02 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
On Wed, 27 Apr 2022, Jason Merrill via Gcc-patches wrote:
> + if (typedef_variant_p (type))
> + {
> + /* Set up the typedef all over again. */
This seems wrong when the typedef is just being used in another
declaration with the mode attribute, as opposed to being defined using the
mode attribute. E.g. the following test is valid and accepted before the
patch, but wrongly rejected after the patch because the typedef has had
its type changed.
typedef int I;
int x;
I y __attribute__ ((mode(QI)));
extern I x;
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH RFA] c-family: attribute ((aligned, mode)) [PR100545]
2022-04-27 17:02 ` Joseph Myers
@ 2022-04-27 23:48 ` Jason Merrill
2022-04-29 19:20 ` Jason Merrill
0 siblings, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2022-04-27 23:48 UTC (permalink / raw)
To: Joseph Myers; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 619 bytes --]
On 4/27/22 13:02, Joseph Myers wrote:
> On Wed, 27 Apr 2022, Jason Merrill via Gcc-patches wrote:
>
>> + if (typedef_variant_p (type))
>> + {
>> + /* Set up the typedef all over again. */
>
> This seems wrong when the typedef is just being used in another
> declaration with the mode attribute, as opposed to being defined using the
> mode attribute. E.g. the following test is valid and accepted before the
> patch, but wrongly rejected after the patch because the typedef has had
> its type changed.
>
> typedef int I;
> int x;
> I y __attribute__ ((mode(QI)));
> extern I x;
Ah, good point. Fixed thus:
[-- Attachment #2: 0001-c-family-attribute-aligned-mode-PR100545.patch --]
[-- Type: text/x-patch, Size: 3732 bytes --]
From 35210478aa12623dcf027b7a14ea5216d93a46e4 Mon Sep 17 00:00:00 2001
From: Jason Merrill <jason@redhat.com>
Date: Fri, 15 Apr 2022 16:27:05 -0400
Subject: [PATCH] c-family: attribute ((aligned, mode)) [PR100545]
To: gcc-patches@gcc.gnu.org
The problem here was that handle_mode_attribute clobbered the changes of any
previous attribute, only copying type qualifiers to the new type. And
common_handle_aligned_attribute had previously set up the typedef, so when
we later called set_underlying_type it saw DECL_ORIGINAL_TYPE set and just
returned, even though handle_mode_attribute had messed up the TREE_TYPE.
So, let's fix handle_mode_attribute to copy attributes, alignment, and
typedefness to the new type.
PR c/100545
gcc/c-family/ChangeLog:
* c-attribs.cc (handle_mode_attribute): Copy attributes, aligned,
and typedef.
* c-common.cc (set_underlying_type): Add assert.
gcc/testsuite/ChangeLog:
* c-c++-common/attr-mode-1.c: New test.
* c-c++-common/attr-mode-2.c: New test.
---
gcc/c-family/c-attribs.cc | 16 +++++++++++++++-
gcc/c-family/c-common.cc | 7 ++++---
gcc/testsuite/c-c++-common/attr-mode-1.c | 3 +++
gcc/testsuite/c-c++-common/attr-mode-2.c | 4 ++++
4 files changed, 26 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/attr-mode-1.c
create mode 100644 gcc/testsuite/c-c++-common/attr-mode-2.c
diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
index 111a33f405a..b1953a45f9b 100644
--- a/gcc/c-family/c-attribs.cc
+++ b/gcc/c-family/c-attribs.cc
@@ -2199,7 +2199,21 @@ handle_mode_attribute (tree *node, tree name, tree args,
return NULL_TREE;
}
- *node = build_qualified_type (typefm, TYPE_QUALS (type));
+ /* Copy any quals and attributes to the new type. */
+ *node = build_type_attribute_qual_variant (typefm, TYPE_ATTRIBUTES (type),
+ TYPE_QUALS (type));
+ if (TYPE_USER_ALIGN (type))
+ *node = build_aligned_type (*node, TYPE_ALIGN (type));
+
+ tree decl = node[2];
+ if (decl && TYPE_NAME (type) == decl)
+ {
+ /* Set up the typedef all over again. */
+ DECL_ORIGINAL_TYPE (decl) = NULL_TREE;
+ TREE_TYPE (decl) = *node;
+ set_underlying_type (decl);
+ *node = TREE_TYPE (decl);
+ }
}
return NULL_TREE;
diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
index bb0544eeaea..730faa9e87f 100644
--- a/gcc/c-family/c-common.cc
+++ b/gcc/c-family/c-common.cc
@@ -8153,15 +8153,16 @@ check_missing_format_attribute (tree ltype, tree rtype)
void
set_underlying_type (tree x)
{
- if (x == error_mark_node)
+ if (x == error_mark_node || TREE_TYPE (x) == error_mark_node)
return;
if (DECL_IS_UNDECLARED_BUILTIN (x) && TREE_CODE (TREE_TYPE (x)) != ARRAY_TYPE)
{
if (TYPE_NAME (TREE_TYPE (x)) == 0)
TYPE_NAME (TREE_TYPE (x)) = x;
}
- else if (TREE_TYPE (x) != error_mark_node
- && DECL_ORIGINAL_TYPE (x) == NULL_TREE)
+ else if (DECL_ORIGINAL_TYPE (x))
+ gcc_checking_assert (TYPE_NAME (TREE_TYPE (x)) == x);
+ else
{
tree tt = TREE_TYPE (x);
DECL_ORIGINAL_TYPE (x) = tt;
diff --git a/gcc/testsuite/c-c++-common/attr-mode-1.c b/gcc/testsuite/c-c++-common/attr-mode-1.c
new file mode 100644
index 00000000000..59b20cd99e8
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/attr-mode-1.c
@@ -0,0 +1,3 @@
+// PR c/100545
+
+typedef int fatp_t __attribute__((aligned, mode(SI)));
diff --git a/gcc/testsuite/c-c++-common/attr-mode-2.c b/gcc/testsuite/c-c++-common/attr-mode-2.c
new file mode 100644
index 00000000000..de65f49c6b6
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/attr-mode-2.c
@@ -0,0 +1,4 @@
+typedef int I;
+int x;
+I y __attribute__ ((mode(QI)));
+extern I x;
--
2.27.0
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH RFA] c-family: attribute ((aligned, mode)) [PR100545]
2022-04-27 23:48 ` Jason Merrill
@ 2022-04-29 19:20 ` Jason Merrill
2022-04-29 19:46 ` Joseph Myers
0 siblings, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2022-04-29 19:20 UTC (permalink / raw)
To: Joseph Myers; +Cc: gcc-patches
On 4/27/22 19:48, Jason Merrill wrote:
> On 4/27/22 13:02, Joseph Myers wrote:
>> On Wed, 27 Apr 2022, Jason Merrill via Gcc-patches wrote:
>>
>>> + if (typedef_variant_p (type))
>>> + {
>>> + /* Set up the typedef all over again. */
>>
>> This seems wrong when the typedef is just being used in another
>> declaration with the mode attribute, as opposed to being defined using
>> the
>> mode attribute. E.g. the following test is valid and accepted before the
>> patch, but wrongly rejected after the patch because the typedef has had
>> its type changed.
>>
>> typedef int I;
>> int x;
>> I y __attribute__ ((mode(QI)));
>> extern I x;
>
> Ah, good point. Fixed thus:
Marek pointed out elsewhere that the first testcase should have
// { dg-additional-options -g }
OK for 13 with that change?
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-04-29 19:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-27 15:19 [PATCH RFA] c-family: attribute ((aligned, mode)) [PR100545] Jason Merrill
2022-04-27 16:58 ` Marek Polacek
2022-04-27 17:02 ` Joseph Myers
2022-04-27 23:48 ` Jason Merrill
2022-04-29 19:20 ` Jason Merrill
2022-04-29 19:46 ` Joseph Myers
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).