public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH v4] generate EH info for asm statements (PR93981)
@ 2020-12-01 23:23 J.W. Jagersma
  0 siblings, 0 replies; only message in thread
From: J.W. Jagersma @ 2020-12-01 23:23 UTC (permalink / raw)
  To: gcc-patches; +Cc: J.W. Jagersma

The following patch extends the generation of exception handling
information, so that it is possible to catch exceptions thrown from
asm statements, when -fnon-call-exceptions is enabled.  Parts of the
gcc code already suggested this should be possible, but it was never
fully implemented.  Both volatile and non-volatile asms, including
asm goto, are now considered potentially throwing.

A new '-' constraint modifier is introduced, that allows users to
specify which asm outputs must be clobbered in case an exception is
thrown.  This also applies to the jump path for asm goto.

Three new test cases are added.  The target-dependent test should pass
on platforms where throwing from a signal handler is allowed.  The only
platform I am aware of where that is the case is *-linux-gnu, so it is
set to XFAIL on all others.

gcc/
2020-12-01  Jan W. Jagersma  <jwjagersma@gmail.com>

	PR inline-asm/93981
	* cfgexpand.c (expand_asm_stmt): Expand throwing asms into jump
	insns.  Emit move insns across edges.
	* gimplify.c (gimplify_asm_expr): Assign '-' constrained
	outputs via a temporary.  Warn for other constraint modifiers
	if a copy must be made.
	* stmt.c (parse_output_constraint): Accept '-' modifier.
	(parse_input_constraint): Don't accept '-' modifier.
	* tree-cfg.c (make_edges_bb): Make EH edges for GIMPLE_ASM.
	* tree-eh.c (lower_eh_constructs_2): Add case for GIMPLE_ASM.
	(tree_could_trap_p): Return true for case ASM_EXPR.
	(stmt_could_throw_p): Likewise, for GIMPLE_ASM.
	* rtlanal.c (may_trap_p_1): Likewise, for ASM_OPERANDS.
	* doc/extend.texi: Document the effects of constraint modifiers
	on output semantics for throwing/jumping asms.
	* doc/md.texi: Document new '-' constraint modifier.

gcc/testsuite/
2020-12-01  Jan W. Jagersma  <jwjagersma@gmail.com>

	PR inline-asm/93981
	* g++.target/i386/pr93981.C: New test.
	* g++.dg/eh/pr93981-1.C: New test.
	* g++.dg/eh/pr93981-2.C: New test.
---
 gcc/cfgexpand.c                         |  12 +-
 gcc/doc/extend.texi                     |   9 ++
 gcc/doc/md.texi                         |  26 +++-
 gcc/gimplify.c                          |  41 ++++++-
 gcc/rtlanal.c                           |   4 +-
 gcc/stmt.c                              |  16 +--
 gcc/testsuite/g++.dg/eh/pr93981-1.C     |  18 +++
 gcc/testsuite/g++.dg/eh/pr93981-2.C     |  29 +++++
 gcc/testsuite/g++.target/i386/pr93981.C | 151 ++++++++++++++++++++++++
 gcc/tree-cfg.c                          |   2 +
 gcc/tree-eh.c                           |   7 +-
 11 files changed, 291 insertions(+), 24 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/eh/pr93981-1.C
 create mode 100644 gcc/testsuite/g++.dg/eh/pr93981-2.C
 create mode 100644 gcc/testsuite/g++.target/i386/pr93981.C

diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 7e0bdd58e85..a32e3637601 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -3496,7 +3496,7 @@ expand_asm_stmt (gasm *stmt)
   if (noutputs == 0 && nclobbers == 0)
     {
       /* No output operands: put in a raw ASM_OPERANDS rtx.  */
-      if (nlabels > 0)
+      if (stmt_ends_bb_p (stmt))
 	emit_jump_insn (body);
       else
 	emit_insn (body);
@@ -3504,7 +3504,7 @@ expand_asm_stmt (gasm *stmt)
   else if (noutputs == 1 && nclobbers == 0)
     {
       ASM_OPERANDS_OUTPUT_CONSTRAINT (body) = constraints[0];
-      if (nlabels > 0)
+      if (stmt_ends_bb_p (stmt))
 	emit_jump_insn (gen_rtx_SET (output_rvec[0], body));
       else 
 	emit_insn (gen_rtx_SET (output_rvec[0], body));
@@ -3570,7 +3570,7 @@ expand_asm_stmt (gasm *stmt)
 	  XVECEXP (body, 0, i++) = gen_rtx_CLOBBER (VOIDmode, clobbered_reg);
 	}
 
-      if (nlabels > 0)
+      if (stmt_ends_bb_p (stmt))
 	emit_jump_insn (body);
       else
 	emit_insn (body);
@@ -3585,9 +3585,7 @@ expand_asm_stmt (gasm *stmt)
     emit_insn (after_md_seq);
   if (after_rtl_seq)
     {
-      if (nlabels == 0)
-	emit_insn (after_rtl_seq);
-      else
+      if (stmt_ends_bb_p (stmt))
 	{
 	  edge e;
 	  edge_iterator ei;
@@ -3604,6 +3602,8 @@ expand_asm_stmt (gasm *stmt)
 	      insert_insn_on_edge (copy, e);
 	    }
 	}
+      else
+	emit_insn (after_rtl_seq);
     }
 
   free_temp_slots ();
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 23ede966bae..4e1da68a85e 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -10009,6 +10009,15 @@ ensures that modifying @var{a} does not affect the address referenced by
 @var{b}. Otherwise, the location of @var{b} 
 is undefined if @var{a} is modified before using @var{b}.
 
+If the @code{asm} throws an exception (which is possible when
+@option{-fnon-call-exceptions} are enabled) or jumps to a goto label, the
+operand's validity is determined by the constraint modifier used.  For example,
+the @samp{-} modifier may be used to specify that the output must be considered
+clobbered on the exception or jump paths.  If an operand is not always written
+to, but must remain valid on all code paths, the @samp{+} modifier should be
+used to prevent earlier assignments from being optimized away.  See
+@pxref{Modifiers} for details.
+
 @code{asm} supports operand modifiers on operands (for example @samp{%k2} 
 instead of simply @samp{%2}). Typically these qualifiers are hardware 
 dependent. The list of supported modifiers for x86 is found at 
diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
index da8c9a283dd..98ceb1a6d40 100644
--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -1607,6 +1607,9 @@ Here are constraint modifier characters.
 Means that this operand is written to by this instruction:
 the previous value is discarded and replaced by new data.
 
+The value of an operand with this modifier is undefined if the instruction
+does not write to it, eg. on the exception or jump path.
+
 @cindex @samp{+} in constraint
 @item +
 Means that this operand is both read and written by the instruction.
@@ -1617,8 +1620,23 @@ which are written by it.  @samp{=} identifies an operand which is only
 written; @samp{+} identifies an operand that is both read and written; all
 other operands are assumed to only be read.
 
-If you specify @samp{=} or @samp{+} in a constraint, you put it in the
-first character of the constraint string.
+The @samp{+} modifier also implies that the output remains valid when an
+exception is thrown; it retains its previous value if the instruction does
+not write to it.
+
+@cindex @samp{-} in constraint
+@item -
+Like @samp{=}, means that this operand is written by the instruction, and
+additionally specifies that it must be considered clobbered when an exception
+is thrown, or the instruction branches.  The compiler assigns the output via a
+temporary variable, which is only used on the fallthrough path.  This is
+generally only possible for operands that fit in a register.
+
+The @samp{-} modifier is implemented only for @code{asm} statements and is
+invalid for other instructions.
+
+If you specify @samp{=}, @samp{+} or @samp{-} in a constraint, you put it in
+the first character of the constraint string.
 
 @cindex @samp{&} in constraint
 @cindex earlyclobber operand
@@ -1643,8 +1661,8 @@ earlyclobber. See, for example, the @samp{mulsi3} insn of the ARM@.
 Furthermore, if the @dfn{earlyclobber} operand is also a read/write
 operand, then that operand is written only after it's used.
 
-@samp{&} does not obviate the need to write @samp{=} or @samp{+}.  As
-@dfn{earlyclobber} operands are always written, a read-only
+@samp{&} does not obviate the need to write @samp{=}, @samp{+} or @samp{-}.
+As @dfn{earlyclobber} operands are always written, a read-only
 @dfn{earlyclobber} operand is ill-formed and will be rejected by the
 compiler.
 
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 54cb66bd1dd..a574690bcf2 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -6329,10 +6329,38 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
 	  ret = tret;
 	}
 
+      /* Outputs constrained with '-' are assigned via a temporary
+	 so that they are not used on the exception or goto paths.  */
+      if (constraint[0] == '-')
+	{
+	  /* Turn it into a regular '=' constraint, since other
+	     parts of gcc do not recognize the '-' modifier.  */
+	  char *p = xstrdup (constraint);
+	  p[0] = '=';
+	  TREE_VALUE (TREE_PURPOSE (link)) = build_string (constraint_len, p);
+
+	  tree op = TREE_VALUE (link);
+	  if (! tree_could_throw_p (op)
+	      && is_gimple_reg_type (TREE_TYPE (op)))
+	    {
+	      tree tmp = create_tmp_reg (TREE_TYPE (op));
+	      tree assign = build2 (MODIFY_EXPR, TREE_TYPE (tmp), op, tmp);
+	      gimplify_and_add (assign, post_p);
+	      TREE_VALUE (link) = tmp;
+	    }
+	  else
+	    {
+	      error_at (EXPR_LOC_OR_LOC (op, input_location),
+			"cannot make temporary for operand %d"
+			" constrained with %<-%>",
+			i);
+	      ret = GS_ERROR;
+	    }
+	}
       /* If the constraint does not allow memory make sure we gimplify
-         it to a register if it is not already but its base is.  This
+	 it to a register if it is not already but its base is.  This
 	 happens for complex and vector components.  */
-      if (!allows_mem)
+      else if (!allows_mem)
 	{
 	  tree op = TREE_VALUE (link);
 	  if (! is_gimple_val (op)
@@ -6352,6 +6380,15 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
 
 	      TREE_VALUE (link) = tem;
 	      tret = GS_OK;
+
+	      /* If a copy must be made here, and the asm can alter control
+		 flow, emit a warning now.  The user must specify '-' in
+		 this case.  */
+	      if (cfun->can_throw_non_call_exceptions
+		  || ASM_LABELS (expr) != NULL)
+		warning_at (EXPR_LOC_OR_LOC (op, input_location), 0,
+			    "output %d is only valid on fallthrough path"
+			    " (use %<-%> constraint to suppress)", i);
 	    }
 	}
 
diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index 01130a10783..b3b5208ff93 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -2844,13 +2844,11 @@ may_trap_p_1 (const_rtx x, unsigned flags)
       return targetm.unspec_may_trap_p (x, flags);
 
     case UNSPEC_VOLATILE:
+    case ASM_OPERANDS:
     case ASM_INPUT:
     case TRAP_IF:
       return 1;
 
-    case ASM_OPERANDS:
-      return MEM_VOLATILE_P (x);
-
       /* Memory ref can trap unless it's a static var or a stack slot.  */
     case MEM:
       /* Recognize specific pattern of stack checking probes.  */
diff --git a/gcc/stmt.c b/gcc/stmt.c
index d81271a8160..dcd8ac93626 100644
--- a/gcc/stmt.c
+++ b/gcc/stmt.c
@@ -207,6 +207,8 @@ parse_output_constraint (const char **constraint_p, int operand_num,
   p = strchr (constraint, '=');
   if (!p)
     p = strchr (constraint, '+');
+  if (!p)
+    p = strchr (constraint, '-');
 
   /* If the string doesn't contain an `=', issue an error
      message.  */
@@ -220,8 +222,9 @@ parse_output_constraint (const char **constraint_p, int operand_num,
      from and written to.  */
   *is_inout = (*p == '+');
 
-  /* Canonicalize the output constraint so that it begins with `='.  */
-  if (p != constraint || *is_inout)
+  /* Canonicalize the output constraint so that it begins with the
+     modifier character.  */
+  if (p != constraint)
     {
       char *buf;
       size_t c_len = strlen (constraint);
@@ -236,9 +239,7 @@ parse_output_constraint (const char **constraint_p, int operand_num,
       strcpy (buf, constraint);
       /* Swap the first character and the `=' or `+'.  */
       buf[p - constraint] = buf[0];
-      /* Make sure the first character is an `='.  (Until we do this,
-	 it might be a `+'.)  */
-      buf[0] = '=';
+      buf[0] = *p;
       /* Replace the constraint with the canonicalized string.  */
       *constraint_p = ggc_alloc_string (buf, c_len);
       constraint = *constraint_p;
@@ -251,8 +252,9 @@ parse_output_constraint (const char **constraint_p, int operand_num,
 	{
 	case '+':
 	case '=':
+	case '-':
 	  error ("operand constraint contains incorrectly positioned "
-		 "%<+%> or %<=%>");
+		 "%<+%>, %<=%> or %<-%>");
 	  return false;
 
 	case '%':
@@ -335,7 +337,7 @@ parse_input_constraint (const char **constraint_p, int input_num,
   for (j = 0; j < c_len; j += CONSTRAINT_LEN (constraint[j], constraint+j))
     switch (constraint[j])
       {
-      case '+':  case '=':  case '&':
+      case '-': case '+':  case '=':  case '&':
 	if (constraint == orig_constraint)
 	  {
 	    error ("input operand constraint contains %qc", constraint[j]);
diff --git a/gcc/testsuite/g++.dg/eh/pr93981-1.C b/gcc/testsuite/g++.dg/eh/pr93981-1.C
new file mode 100644
index 00000000000..472b0ef299c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/eh/pr93981-1.C
@@ -0,0 +1,18 @@
+// PR inline-asm/93981
+// { dg-do compile }
+// { dg-options "-fnon-call-exceptions -O3" }
+
+void
+foo ()
+{
+  try
+    {
+      asm ("#try");
+    }
+  catch (...)
+    {
+      asm ("#catch");
+    }
+}
+
+// { dg-final { scan-assembler "#catch" } }
diff --git a/gcc/testsuite/g++.dg/eh/pr93981-2.C b/gcc/testsuite/g++.dg/eh/pr93981-2.C
new file mode 100644
index 00000000000..2140bb5f61c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/eh/pr93981-2.C
@@ -0,0 +1,29 @@
+// PR inline-asm/93981
+// { dg-do compile }
+// { dg-options "-fnon-call-exceptions" }
+
+int
+a ()
+{
+  int x = 0;
+  try { asm ("#%0" : "-m" (x)); } // no error
+  catch (...) { }
+  return x;
+}
+
+int
+b (int *x)
+{
+  try { asm ("#%0" : "-rm" (*x)); } // { dg-error "cannot make temporary" }
+  catch (...) { }
+  return *x;
+}
+
+int
+c ()
+{
+  struct { int a, b, c; } x = { 1, 2, 3 };
+  try { asm ("#%0" : "-m" (x)); } // { dg-error "cannot make temporary" }
+  catch (...) { }
+  return x.a + x.b + x.c;
+}
diff --git a/gcc/testsuite/g++.target/i386/pr93981.C b/gcc/testsuite/g++.target/i386/pr93981.C
new file mode 100644
index 00000000000..8f96734909d
--- /dev/null
+++ b/gcc/testsuite/g++.target/i386/pr93981.C
@@ -0,0 +1,151 @@
+// PR inline-asm/93981
+// { dg-do run }
+// { dg-options "-fnon-call-exceptions -O3 -masm=intel" }
+// { dg-xfail-if "" { ! *-linux-gnu } }
+// { dg-xfail-run-if "" { ! *-linux-gnu } }
+
+#include <signal.h>
+
+struct illegal_opcode { };
+
+extern "C" void
+sigill (int)
+{
+  throw illegal_opcode ( );
+}
+
+int
+f ()
+{
+  try
+    {
+      asm ("ud2");
+    }
+  catch (const illegal_opcode&)
+    {
+      return 0;
+    }
+  return 1;
+}
+
+int
+m0 ()
+{
+  int i = 0;
+  try
+    {
+      asm ("mov %0, 1; ud2" : "-m" (i));
+    }
+  catch (const illegal_opcode&) { }
+  return i;
+}
+
+int
+m1 ()
+{
+  int i = 1;
+  try
+    {
+      asm ("mov %0, 0; ud2" : "=m" (i));
+    }
+  catch (const illegal_opcode&) { }
+  return i;
+}
+
+int
+m2 ()
+{
+  int i = 0;
+  try
+    {
+      asm ("ud2" : "+m" (i));
+    }
+  catch (const illegal_opcode&) { }
+  return i;
+}
+
+int
+r0 ()
+{
+  int i = 0;
+  try
+    {
+      asm ("mov %0, 1; ud2" : "-r" (i));
+    }
+  catch (const illegal_opcode&) { }
+  return i;
+}
+
+int
+r1 ()
+{
+  int i = 1;
+  try
+    {
+      asm ("mov %0, 0; ud2" : "=r" (i));
+    }
+  catch (const illegal_opcode&) { }
+  return i;
+}
+
+int
+r2 ()
+{
+  int i = 0;
+  try
+    {
+      asm ("ud2" : "+r" (i));
+    }
+  catch (const illegal_opcode&) { }
+  return i;
+}
+
+int
+rm0 ()
+{
+  asm ("#%0" :: "r" (__builtin_frame_address (0)));
+  int i = 1;
+  try
+    {
+      asm ("mov %0, 0; ud2"
+	   : "+rm" (i)
+	   :
+	   : "eax", "ebx", "ecx", "edx", "edi", "esi"
+#ifdef __x86_64__
+	   , "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15"
+#endif
+	   );
+    }
+  catch (const illegal_opcode&) { }
+  return i;
+}
+
+int
+rm1 ()
+{
+  asm ("#%0" :: "r" (__builtin_frame_address (0)));
+  int i = 0;
+  try
+    {
+      asm ("mov %0, 1; ud2"
+	   : "-rm" (i)
+	   :
+	   : "eax", "ebx", "ecx", "edx", "edi", "esi"
+#ifdef __x86_64__
+	   , "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15"
+#endif
+	   );
+    }
+  catch (const illegal_opcode&) { }
+  return i;
+}
+
+int
+main ()
+{
+  struct sigaction sa = { };
+  sa.sa_handler = sigill;
+  sa.sa_flags = SA_NODEFER;
+  sigaction (SIGILL, &sa, 0);
+  return f () | m0 () | m1 () | m2 () | r0 () | r1 () | r2 () | rm0 () | rm1 ();
+}
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index f59a0c05200..b2ff62204bc 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -907,6 +907,8 @@ make_edges_bb (basic_block bb, struct omp_region **pcur_region, int *pomp_index)
       break;
 
     case GIMPLE_ASM:
+      if (stmt_can_throw_internal (cfun, last))
+	make_eh_edges (last);
       make_gimple_asm_edges (bb);
       fallthru = true;
       break;
diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
index c3314bbd78c..5dec2c1dd39 100644
--- a/gcc/tree-eh.c
+++ b/gcc/tree-eh.c
@@ -2069,6 +2069,9 @@ lower_eh_constructs_2 (struct leh_state *state, gimple_stmt_iterator *gsi)
 	  gimple_set_lhs (stmt, tmp);
 	  gsi_insert_after (gsi, s, GSI_SAME_STMT);
 	}
+
+      /* FALLTHRU */
+    case GIMPLE_ASM:
       /* Look for things that can throw exceptions, and record them.  */
       if (state->cur_region && stmt_could_throw_p (cfun, stmt))
 	{
@@ -2720,7 +2723,7 @@ tree_could_trap_p (tree expr)
       return !TREE_THIS_NOTRAP (expr);
 
     case ASM_EXPR:
-      return TREE_THIS_VOLATILE (expr);
+      return true;
 
     case CALL_EXPR:
       t = get_callee_fndecl (expr);
@@ -2924,7 +2927,7 @@ stmt_could_throw_p (function *fun, gimple *stmt)
     case GIMPLE_ASM:
       if (fun && !fun->can_throw_non_call_exceptions)
         return false;
-      return gimple_asm_volatile_p (as_a <gasm *> (stmt));
+      return true;
 
     default:
       return false;
-- 
2.29.2


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2020-12-01 23:24 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-01 23:23 [PATCH v4] generate EH info for asm statements (PR93981) J.W. Jagersma

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).