On Wed, Sep 10, 2025 at 02:20:58AM -0700, Andrew Pinski wrote:
> On Tue, Aug 26, 2025 at 10:42 PM Stefan Schulze Frielinghaus
> <stefa...@linux.ibm.com> wrote:
> >
> > Bootstrapped and regtested on x86_64 and s390. Ok for mainline?
> >
> > -- >8 --
> >
> > In case an asm operand is an error node, constraints etc. are still
> > validated. Furthermore, all other operands are gimplified, although an
> > error is returned in the end anyway. For hard register constraints an
> > operand is required in order to determine the mode from which the number
> > of registers follows. Therefore, instead of adding extra guards, bail
> > out early.
> >
> > gcc/ChangeLog:
> >
> > PR 121391
> > * gimplify.cc (gimplify_asm_expr): In case an asm operand is an
> > error node, bail out early.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.dg/pr121391-1.c: New test.
> > * gcc.dg/pr121391-2.c: New test.
> > ---
> > gcc/gimplify.cc | 4 ++++
> > gcc/testsuite/gcc.dg/pr121391-1.c | 9 +++++++++
> > gcc/testsuite/gcc.dg/pr121391-2.c | 9 +++++++++
> > 3 files changed, 22 insertions(+)
> > create mode 100644 gcc/testsuite/gcc.dg/pr121391-1.c
> > create mode 100644 gcc/testsuite/gcc.dg/pr121391-2.c
> >
> > diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
> > index ca1fa2189cb..2b790923fa1 100644
> > --- a/gcc/gimplify.cc
> > +++ b/gcc/gimplify.cc
> > @@ -7930,6 +7930,8 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p,
> > gimple_seq *post_p)
> > bool ok;
> > size_t constraint_len;
> >
> > + if (TREE_VALUE (link) == error_mark_node)
> > + return GS_ERROR;
>
> Maybe use error_operand_p instead of a direct comparison to error_mark_node.
Thanks for pointing this out. Wasn't aware of the predicate. This
would then in turn render the check below for an error mark node for the
type superfluous. I attached an updated patch.
>
> Otherwise this is almost obvious.
Ok, if bootstrap is still successful and no one objects, I will commit
the patch.
Thanks,
Stefan
>
> Thanks,
> Andrew
>
> > link_next = TREE_CHAIN (link);
> >
> > oconstraints[i]
> > @@ -8155,6 +8157,8 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p,
> > gimple_seq *post_p)
> > int input_num = 0;
> > for (link = ASM_INPUTS (expr); link; ++input_num, ++i, link = link_next)
> > {
> > + if (TREE_VALUE (link) == error_mark_node)
> > + return GS_ERROR;
> > link_next = TREE_CHAIN (link);
> > constraint = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (link)));
> > reg_info.operand = TREE_VALUE (link);
> > diff --git a/gcc/testsuite/gcc.dg/pr121391-1.c
> > b/gcc/testsuite/gcc.dg/pr121391-1.c
> > new file mode 100644
> > index 00000000000..6a015210ef9
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/pr121391-1.c
> > @@ -0,0 +1,9 @@
> > +/* { dg-do compile { target aarch64*-*-* arm*-*-* powerpc*-*-* s390*-*-*
> > x86_64-*-* } } */
> > +
> > +/* For the non existing variable we are faced with an error mark node
> > during
> > + gimplify_asm_expr(). */
> > +
> > +void test (void)
> > +{
> > + __asm__ __volatile__ ("" : "={2}" (non_existing_var)); /* { dg-error
> > {'non_existing_var' undeclared} } */
> > +}
> > diff --git a/gcc/testsuite/gcc.dg/pr121391-2.c
> > b/gcc/testsuite/gcc.dg/pr121391-2.c
> > new file mode 100644
> > index 00000000000..c03f0ab283b
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/pr121391-2.c
> > @@ -0,0 +1,9 @@
> > +/* { dg-do compile { target aarch64*-*-* arm*-*-* powerpc*-*-* s390*-*-*
> > x86_64-*-* } } */
> > +
> > +/* For the non existing variable we are faced with an error mark node
> > during
> > + gimplify_asm_expr(). */
> > +
> > +void test (void)
> > +{
> > + __asm__ __volatile__ ("" :: "{2}" (non_existing_var)); /* { dg-error
> > {'non_existing_var' undeclared} } */
> > +}
> > --
> > 2.49.0
> >
>From fa47775efcdfab8146bb0b12db9e90cbe64f979c Mon Sep 17 00:00:00 2001
From: Stefan Schulze Frielinghaus <stefa...@linux.ibm.com>
Date: Wed, 10 Sep 2025 11:52:56 +0200
Subject: [PATCH v2] Bail out early during gimplify_asm_expr [PR121391]
From: Stefan Schulze Frielinghaus <stefa...@gcc.gnu.org>
In case an asm operand is an error node, constraints etc. are still
validated. Furthermore, all other operands are gimplified, although an
error is returned in the end anyway. For hard register constraints an
operand is required in order to determine the mode from which the number
of registers follows. Therefore, instead of adding extra guards, bail
out early.
gcc/ChangeLog:
PR 121391
* gimplify.cc (gimplify_asm_expr): In case an asm operand is an
error node, bail out early.
gcc/testsuite/ChangeLog:
* gcc.dg/pr121391-1.c: New test.
* gcc.dg/pr121391-2.c: New test.
---
gcc/gimplify.cc | 18 ++++++++++--------
gcc/testsuite/gcc.dg/pr121391-1.c | 9 +++++++++
gcc/testsuite/gcc.dg/pr121391-2.c | 9 +++++++++
3 files changed, 28 insertions(+), 8 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/pr121391-1.c
create mode 100644 gcc/testsuite/gcc.dg/pr121391-2.c
diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
index ca1fa2189cb..90096835665 100644
--- a/gcc/gimplify.cc
+++ b/gcc/gimplify.cc
@@ -7930,6 +7930,8 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p,
gimple_seq *post_p)
bool ok;
size_t constraint_len;
+ if (error_operand_p (TREE_VALUE (link)))
+ return GS_ERROR;
link_next = TREE_CHAIN (link);
oconstraints[i]
@@ -7951,10 +7953,9 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p,
gimple_seq *post_p)
/* If we can't make copies, we can only accept memory.
Similarly for VLAs. */
tree outtype = TREE_TYPE (TREE_VALUE (link));
- if (outtype != error_mark_node
- && (TREE_ADDRESSABLE (outtype)
- || !COMPLETE_TYPE_P (outtype)
- || !tree_fits_poly_uint64_p (TYPE_SIZE_UNIT (outtype))))
+ if (TREE_ADDRESSABLE (outtype)
+ || !COMPLETE_TYPE_P (outtype)
+ || !tree_fits_poly_uint64_p (TYPE_SIZE_UNIT (outtype)))
{
if (allows_mem)
allows_reg = 0;
@@ -8155,6 +8156,8 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p,
gimple_seq *post_p)
int input_num = 0;
for (link = ASM_INPUTS (expr); link; ++input_num, ++i, link = link_next)
{
+ if (error_operand_p (TREE_VALUE (link)))
+ return GS_ERROR;
link_next = TREE_CHAIN (link);
constraint = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (link)));
reg_info.operand = TREE_VALUE (link);
@@ -8169,10 +8172,9 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p,
gimple_seq *post_p)
/* If we can't make copies, we can only accept memory. */
tree intype = TREE_TYPE (TREE_VALUE (link));
- if (intype != error_mark_node
- && (TREE_ADDRESSABLE (intype)
- || !COMPLETE_TYPE_P (intype)
- || !tree_fits_poly_uint64_p (TYPE_SIZE_UNIT (intype))))
+ if (TREE_ADDRESSABLE (intype)
+ || !COMPLETE_TYPE_P (intype)
+ || !tree_fits_poly_uint64_p (TYPE_SIZE_UNIT (intype)))
{
if (allows_mem)
allows_reg = 0;
diff --git a/gcc/testsuite/gcc.dg/pr121391-1.c
b/gcc/testsuite/gcc.dg/pr121391-1.c
new file mode 100644
index 00000000000..6a015210ef9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr121391-1.c
@@ -0,0 +1,9 @@
+/* { dg-do compile { target aarch64*-*-* arm*-*-* powerpc*-*-* s390*-*-*
x86_64-*-* } } */
+
+/* For the non existing variable we are faced with an error mark node during
+ gimplify_asm_expr(). */
+
+void test (void)
+{
+ __asm__ __volatile__ ("" : "={2}" (non_existing_var)); /* { dg-error
{'non_existing_var' undeclared} } */
+}
diff --git a/gcc/testsuite/gcc.dg/pr121391-2.c
b/gcc/testsuite/gcc.dg/pr121391-2.c
new file mode 100644
index 00000000000..c03f0ab283b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr121391-2.c
@@ -0,0 +1,9 @@
+/* { dg-do compile { target aarch64*-*-* arm*-*-* powerpc*-*-* s390*-*-*
x86_64-*-* } } */
+
+/* For the non existing variable we are faced with an error mark node during
+ gimplify_asm_expr(). */
+
+void test (void)
+{
+ __asm__ __volatile__ ("" :: "{2}" (non_existing_var)); /* { dg-error
{'non_existing_var' undeclared} } */
+}
--
2.49.0
