diff mbox

Patch RFC: x86: Emit .cfi directives for BX register swap

Message ID mcra9hjiu9u.fsf@iant-glaptop.roam.corp.google.com
State New
Headers show

Commit Message

Ian Lance Taylor Nov. 5, 2013, 12:09 a.m. UTC 
This patch to the x86 backend adds .cfi directives for the BX register
swap in the atomic compare-and-exchange-doubleword insn.  If we don't do
this, then when compiling with -fnon-call-exceptions, if the cmpxchg8
memory address is invalid, and the SIGSEGV signal handler throws an
exception, the BX register value will not be restored when unwinding
past the cmpxchg8.  The effect is that BX will have some unpredictable
value, which is bad because BX is callee-saved.

This patch is not perfect, because not all systems support .cfi
directives.  Unfortunately as far as I know we don't have the framework
we need to emit the appropriate CFI opcodes for this insn.  It might be
possible to make it work by splitting the insn after register
allocation, creating separate insns to which to attach CFA_REGISTER
notes.  I have not tried this.  This patch is in any case an improvement
on the current situation for modern systems.

There is a test case for this in the libgo update I am going to commit
soon.  I don't have a non-Go test case.  It's not too hard to contruct a
test case that crosses shared library boundaries, because 32-bit mode
relies on %ebx, but it's somewhat painful to construct a test case that
does not involve shared libraries.

I don't think I need permission to commit this patch, but I will wait
for a day to hear any comments that people may have.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

Ian


2013-11-04  Ian Lance Taylor  <iant@google.com>

	* config/i386/sync.md (atomic_compare_and_swap<dwi>_doubleword):
	If possible, add .cfi directives to record change to bx.
	* config/i386/i386.c (ix86_emit_cfi): New function.
	* config/i386/i386-protos.h (ix86_emit_cfi): Declare.
diff mbox

Patch

Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 204359)
+++ config/i386/i386.c	(working copy)
@@ -17340,6 +17340,14 @@  ix86_split_idivmod (enum machine_mode mo
   emit_label (end_label);
 }
 
+/* Whether it is OK to emit CFI directives when emitting asm code.  */
+
+bool
+ix86_emit_cfi ()
+{
+  return dwarf2out_do_cfi_asm ();
+}
+
 #define LEA_MAX_STALL (3)
 #define LEA_SEARCH_THRESHOLD (LEA_MAX_STALL << 1)
 
Index: config/i386/sync.md
===================================================================
--- config/i386/sync.md	(revision 204359)
+++ config/i386/sync.md	(working copy)
@@ -430,10 +430,21 @@ 
   const char *xchg = "xchg{<imodesuffix>}\t%%<regprefix>bx, %5";
 
   if (swap)
-    output_asm_insn (xchg, operands);
+    {
+      output_asm_insn (xchg, operands);
+      if (ix86_emit_cfi ())
+	{
+	  output_asm_insn (".cfi_remember_state", operands);
+	  output_asm_insn (".cfi_register\t%%<regprefix>bx, %5", operands);
+	}
+    }
   output_asm_insn ("lock{%;} %K7cmpxchg<doublemodesuffix>b\t%2", operands);
   if (swap)
-    output_asm_insn (xchg, operands);
+    {
+      output_asm_insn (xchg, operands);
+      if (ix86_emit_cfi ())
+	output_asm_insn (".cfi_restore_state", operands);
+    }
 
   return "";
 })
Index: config/i386/i386-protos.h
===================================================================
--- config/i386/i386-protos.h	(revision 204359)
+++ config/i386/i386-protos.h	(working copy)
@@ -143,6 +143,7 @@  extern void ix86_split_lshr (rtx *, rtx,
 extern rtx ix86_find_base_term (rtx);
 extern bool ix86_check_movabs (rtx, int);
 extern void ix86_split_idivmod (enum machine_mode, rtx[], bool);
+extern bool ix86_emit_cfi ();
 
 extern rtx assign_386_stack_local (enum machine_mode, enum ix86_stack_slot);
 extern int ix86_attr_length_immediate_default (rtx, bool);