Hi!

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.

Bootstrapped/regtested on s390x-linux, ok for trunk?

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  <ja...@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.

--- gcc/config/s390/s390.cc.jj  2025-01-10 16:09:37.280043199 +0100
+++ gcc/config/s390/s390.cc     2025-01-16 10:43:58.533809724 +0100
@@ -8688,8 +8688,12 @@ print_operand (FILE *file, rtx x, int co
       {
        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 co
       {
        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 co
     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;
--- gcc/testsuite/gcc.target/s390/pr118511.c.jj 2025-01-16 10:51:14.025721964 
+0100
+++ gcc/testsuite/gcc.target/s390/pr118511.c    2025-01-16 10:50:48.009085651 
+0100
@@ -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'" } */
+}

        Jakub

Reply via email to