From: Jakub Jelinek <jakub@redhat.com>
Date: Fri, 17 Jan 2025 18:27:59 +0000 (+0100)
Subject: s390: Replace some checking assertions with output_operand_lossage [PR118511]
X-Git-Tag: basepoints/gcc-16~2548
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=71280df05f28fd568f8b863dd700c0be2f2443a8;p=thirdparty%2Fgcc.git

s390: Replace some checking assertions with output_operand_lossage [PR118511]

r15-2002 s390: Fully exploit vgm, vgbm, vrepi change added
some code to print_operand and added gcc_checking_asserts in there.
But print_operand ideally should have no assertions in it, as most
of the assumptions can be easily violated by people using it in
inline asm.
This issue in particular was seen by failure to compile s390-tools,
which had in its extended inline asm uses of %r1 and %r2.
I really don't know if they meant %%r1 and %%r2 or %1 and %2 and
will leave that decision to the maintainers, but the thing is that
%r1 and %r2 used to expand like %1 and %2 in GCC 14 and earlier,
now in checking build it ICEs and in --enable-checking=release build
fails to assemble (the checking assert is ignored and the compiler just uses
some uninitialized variables to emit something arbitrary).

With the following patch it is diagnosed as error consistently
regardless if it is release checking or no checking or checking compiler.

Note, I see also
      else if (GET_CODE (x) == UNSPEC && XINT (x, 1) == UNSPEC_TLSLDM)
        {
          fprintf (file, "%s", ":tls_ldcall:");
          const char *name = get_some_local_dynamic_name ();
          gcc_assert (name);
          assemble_name (file, name);
        }
in print_operand, maybe that isn't a big deal because it might be
impossible to construct inline asm argument which is UNSPEC_TLSLDM.
And then there is
        case 'e': case 'f':
        case 's': case 't':
          {
            int start, end;
            int len;
            bool ok;

            len = (code == 's' || code == 'e' ? 64 : 32);
            ok = s390_contiguous_bitmask_p (ival, true, len, &start, &end);
            gcc_assert (ok);
            if (code == 's' || code == 't')
              ival = start;
            else
              ival = end;
          }
          break;
which likely should be also output_operand_lossage but I haven't tried
to reproduce that.

2025-01-17  Jakub Jelinek  <jakub@redhat.com>

	PR target/118511
	* config/s390/s390.cc (print_operand) <case 'p'>: Use
	output_operand_lossage instead of gcc_checking_assert.
	(print_operand) <case 'q'>: Likewise.
	(print_operand) <case 'r'>: Likewise.

	* gcc.target/s390/pr118511.c: New test.
---

diff --git a/gcc/config/s390/s390.cc b/gcc/config/s390/s390.cc
index a98e067bb06f..f166a8a07dd9 100644
--- a/gcc/config/s390/s390.cc
+++ b/gcc/config/s390/s390.cc
@@ -8688,8 +8688,12 @@ print_operand (FILE *file, rtx x, int code)
       {
 	machine_mode mode;
 	short imm;
-	bool b = s390_constant_via_vrepi_p (x, &mode, &imm);
-	gcc_checking_assert (b);
+	if (!s390_constant_via_vrepi_p (x, &mode, &imm))
+	  {
+	    output_operand_lossage ("invalid constant for output "
+				    "modifier '%c'", code);
+	    return;
+	  }
 	switch (mode)
 	  {
 	  case QImode:
@@ -8714,8 +8718,12 @@ print_operand (FILE *file, rtx x, int code)
       {
 	machine_mode mode;
 	int start, end;
-	bool b = s390_constant_via_vgm_p (x, &mode, &start, &end);
-	gcc_checking_assert (b);
+	if (!s390_constant_via_vgm_p (x, &mode, &start, &end))
+	  {
+	    output_operand_lossage ("invalid constant for output "
+				    "modifier '%c'", code);
+	    return;
+	  }
 	switch (mode)
 	  {
 	  case QImode:
@@ -8739,8 +8747,12 @@ print_operand (FILE *file, rtx x, int code)
     case 'r':
       {
 	unsigned mask;
-	bool b = s390_constant_via_vgbm_p (x, &mask);
-	gcc_checking_assert (b);
+	if (!s390_constant_via_vgbm_p (x, &mask))
+	  {
+	    output_operand_lossage ("invalid constant for output "
+				    "modifier '%c'", code);
+	    return;
+	  }
 	fprintf (file, "%u", mask);
       }
       return;
diff --git a/gcc/testsuite/gcc.target/s390/pr118511.c b/gcc/testsuite/gcc.target/s390/pr118511.c
new file mode 100644
index 000000000000..67448e718051
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/pr118511.c
@@ -0,0 +1,11 @@
+/* PR target/118511 */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+void
+foo (void)
+{
+  asm volatile ("# %p0" : : "d" (1));	/* { dg-error "invalid 'asm': invalid constant for output modifier 'p'" } */
+  asm volatile ("# %q0" : : "d" (1));	/* { dg-error "invalid 'asm': invalid constant for output modifier 'q'" } */
+  asm volatile ("# %r0" : : "d" (1));	/* { dg-error "invalid 'asm': invalid constant for output modifier 'r'" } */
+}