]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
[RISC-V][PR target/106544] Avoid ICEs due to bogus asms
authorJeff Law <jlaw@ventanamicro.com>
Mon, 30 Dec 2024 20:51:55 +0000 (13:51 -0700)
committerKito Cheng <kito.cheng@sifive.com>
Wed, 9 Apr 2025 14:03:08 +0000 (22:03 +0800)
This is a fix for a bug Andrew P filed a while back where essentially a poorly
crafted asm statement could trigger a ICE during assembly output.  Various
cases will use INTVAL (op) without verifying the operand is a CONST_INT node
first.

The usual way to handle this is via output_operand_lossage, which this patch
implements.

I focused primarily on the CONST_INT cases, there could well be other problems
in this space, if so they should get distinct bugs with testcases.

Tested in my tester on rv32 and rv64.  Waiting for pre-commit testing before
moving forward.

PR target/106544
gcc/

* config/riscv/riscv.cc (riscv_print_operand): Issue an error for
invalid operands rather than invalidly accessing INTVAL of an
object that is not a CONST_INT.  Fix one error string for 'N'.

gcc/testsuite
* gcc.target/riscv/pr106544.c: New test.

gcc/config/riscv/riscv.cc
gcc/testsuite/gcc.target/riscv/pr106544.c [new file with mode: 0644]

index a6a3b2745c53b0e4671e99e5f2c02762c6c0d1d5..584c3d52c5e73ebd6512af8ef9c37737fcde8eab 100644 (file)
@@ -6269,41 +6269,74 @@ riscv_print_operand (FILE *file, rtx op, int letter)
       fputs (GET_RTX_NAME (reverse_condition (code)), file);
       break;
 
-    case 'A': {
-      const enum memmodel model = memmodel_base (INTVAL (op));
-      if (riscv_memmodel_needs_amo_acquire (model)
-         && riscv_memmodel_needs_amo_release (model))
-       fputs (".aqrl", file);
-      else if (riscv_memmodel_needs_amo_acquire (model))
-       fputs (".aq", file);
-      else if (riscv_memmodel_needs_amo_release (model))
-       fputs (".rl", file);
+    case 'A':
+      if (!CONST_INT_P (op))
+       output_operand_lossage ("invalid operand for '%%%c'", letter);
+      else
+       {
+         const enum memmodel model = memmodel_base (INTVAL (op));
+         if (riscv_memmodel_needs_amo_acquire (model)
+             && riscv_memmodel_needs_amo_release (model))
+           fputs (".aqrl", file);
+         else if (riscv_memmodel_needs_amo_acquire (model))
+           fputs (".aq", file);
+         else if (riscv_memmodel_needs_amo_release (model))
+           fputs (".rl", file);
+       }
       break;
-    }
 
-    case 'I': {
-      const enum memmodel model = memmodel_base (INTVAL (op));
-      if (TARGET_ZTSO && model != MEMMODEL_SEQ_CST)
-       /* LR ops only have an annotation for SEQ_CST in the Ztso mapping.  */
-       break;
-      else if (model == MEMMODEL_SEQ_CST)
-       fputs (".aqrl", file);
-      else if (riscv_memmodel_needs_amo_acquire (model))
-       fputs (".aq", file);
+    case 'I':
+      if (!CONST_INT_P (op))
+       output_operand_lossage ("invalid operand for '%%%c'", letter);
+      else
+       {
+         const enum memmodel model = memmodel_base (INTVAL (op));
+         if (TARGET_ZTSO && model != MEMMODEL_SEQ_CST)
+           /* LR ops only have an annotation for SEQ_CST in the Ztso mapping.  */
+           break;
+         else if (model == MEMMODEL_SEQ_CST)
+           fputs (".aqrl", file);
+         else if (riscv_memmodel_needs_amo_acquire (model))
+           fputs (".aq", file);
+       }
       break;
-    }
 
-    case 'J': {
-      const enum memmodel model = memmodel_base (INTVAL (op));
-      if (TARGET_ZTSO && model == MEMMODEL_SEQ_CST)
-       /* SC ops only have an annotation for SEQ_CST in the Ztso mapping.  */
-       fputs (".rl", file);
-      else if (TARGET_ZTSO)
-       break;
-      else if (riscv_memmodel_needs_amo_release (model))
-       fputs (".rl", file);
+    case 'J':
+      if (!CONST_INT_P (op))
+       output_operand_lossage ("invalid operand for '%%%c'", letter);
+      else
+       {
+         const enum memmodel model = memmodel_base (INTVAL (op));
+         if (TARGET_ZTSO && model == MEMMODEL_SEQ_CST)
+           /* SC ops only have an annotation for SEQ_CST in the Ztso mapping.  */
+           fputs (".rl", file);
+         else if (TARGET_ZTSO)
+           break;
+         else if (riscv_memmodel_needs_amo_release (model))
+           fputs (".rl", file);
+       }
       break;
-    }
+
+    case 'L':
+      {
+       const char *ntl_hint = NULL;
+       switch (INTVAL (op))
+         {
+         case 0:
+           ntl_hint = "ntl.all";
+           break;
+         case 1:
+           ntl_hint = "ntl.pall";
+           break;
+         case 2:
+           ntl_hint = "ntl.p1";
+           break;
+         }
+
+      if (ntl_hint)
+       asm_fprintf (file, "%s\n\t", ntl_hint);
+      break;
+      }
 
     case 'i':
       if (code != REG)
@@ -6315,32 +6348,44 @@ riscv_print_operand (FILE *file, rtx op, int letter)
       break;
 
     case 'S':
-      {
-       rtx newop = GEN_INT (ctz_hwi (INTVAL (op)));
-       output_addr_const (file, newop);
-       break;
-      }
+      if (!CONST_INT_P (op))
+       output_operand_lossage ("invalid operand for '%%%c'", letter);
+      else
+       {
+         rtx newop = GEN_INT (ctz_hwi (INTVAL (op)));
+         output_addr_const (file, newop);
+       }
+      break;
     case 'T':
-      {
-       rtx newop = GEN_INT (ctz_hwi (~INTVAL (op)));
-       output_addr_const (file, newop);
-       break;
-      }
+      if (!CONST_INT_P (op))
+       output_operand_lossage ("invalid operand for '%%%c'", letter);
+      else
+       {
+         rtx newop = GEN_INT (ctz_hwi (~INTVAL (op)));
+         output_addr_const (file, newop);
+       }
+      break;
     case 'X':
-      {
-       int ival = INTVAL (op) + 1;
-       rtx newop = GEN_INT (ctz_hwi (ival) + 1);
-       output_addr_const (file, newop);
-       break;
-      }
+      if (!CONST_INT_P (op))
+       output_operand_lossage ("invalid operand for '%%%c'", letter);
+      else
+       {
+         int ival = INTVAL (op) + 1;
+         rtx newop = GEN_INT (ctz_hwi (ival) + 1);
+         output_addr_const (file, newop);
+       }
+      break;
     case 'Y':
-      {
-       unsigned int imm = (UINTVAL (op) & 63);
-       gcc_assert (imm <= 63);
-       rtx newop = GEN_INT (imm);
-       output_addr_const (file, newop);
-       break;
-      }
+      if (!CONST_INT_P (op))
+       output_operand_lossage ("invalid operand for '%%%c'", letter);
+      else
+       {
+         unsigned int imm = (UINTVAL (op) & 63);
+         gcc_assert (imm <= 63);
+         rtx newop = GEN_INT (imm);
+         output_addr_const (file, newop);
+       }
+      break;
     default:
       switch (code)
        {
diff --git a/gcc/testsuite/gcc.target/riscv/pr106544.c b/gcc/testsuite/gcc.target/riscv/pr106544.c
new file mode 100644 (file)
index 0000000..6e449ee
--- /dev/null
@@ -0,0 +1,6 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-flto"} } */
+void f(int a)
+{
+  __asm("#%A0" : : "r"(a)); /* { dg-error "invalid 'asm': invalid operand" } */
+}