[PATCH] Fix wrong-debug with i?86/x86_64 _GLOBAL_OFFSET_TABLE_ (PR debug/82630)

[Jakub Jelinek <jakub at redhat dot com>]

Hi!

If all fails, when we can't prove that the PIC register is in some hard
register, we delegitimize something + foo@GOTOFF as (something -
_GLOBAL_OFFSET_TABLE_) + foo.  That is reasonable for the middle-end to
understand what is going on (it will never match in actual instructions
though), unfortunately when trying to emit that into .debug_info section
we run into the problem that .long _GLOBAL_OFFSET_TABLE_ etc. is not
actually assembled as address of _GLOBAL_OFFSET_TABLE_, but as
_GLOBAL_OFFSET_TABLE_-. (any time the assembler sees _GLOBAL_OFFSET_TABLE_
symbol by name, it adds the special relocation) and thus we get a bogus
expression.

I couldn't come up with a way to express this that wouldn't be even larger
than what we have, but if we actually not delegitimize it at all and let
it be emitted as
  something
  .byte DW_OP_addr
  .long foo@GOTOFF
  .byte DW_OP_plus
then it works fine and is even shorter than what we used to emit -
  something
  .byte DW_OP_addr
  .long _GLOBAL_OFFSET_TABLE_
  .byte DW_OP_minus
  .byte DW_OP_addr
  .long foo
  .byte DW_OP_plus
In order to achieve that, we need to allow selected UNSPECs through
into debug info, current trunk just gives up on all UNSPECs.

Fortunately, we already have a hook for rejecting some constants, so
by adding the rejection of all UNSPECs into the hook and on i386 overriding
that hook we achieve what we want.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-10-23  Jakub Jelinek  <jakub@redhat.com>

	PR debug/82630
	* target.def (const_not_ok_for_debug_p): Default to
	default_const_not_ok_for_debug_p instead of hook_bool_rtx_false.
	* targhooks.h (default_const_not_ok_for_debug_p): New declaration.
	* targhooks.c (default_const_not_ok_for_debug_p): New function.
	* dwarf2out.c (const_ok_for_output_1): Only reject UNSPECs for
	which targetm.const_not_ok_for_debug_p returned true.
	* config/arm/arm.c (arm_const_not_ok_for_debug_p): Return true
	for UNSPECs.
	* config/powerpcspe/powerpcspe.c (rs6000_const_not_ok_for_debug_p):
	Likewise.
	* config/rs6000/rs6000.c (rs6000_const_not_ok_for_debug_p): Likewise.
	* config/i386/i386.c (ix86_delegitimize_address_1): Don't delegitimize
	UNSPEC_GOTOFF with addend into addend - _GLOBAL_OFFSET_TABLE_ + symbol
	if !base_term_p.
	(ix86_const_not_ok_for_debug_p): New function.
	(i386_asm_output_addr_const_extra): Handle UNSPEC_GOTOFF.
	(TARGET_CONST_NOT_OK_FOR_DEBUG_P): Redefine.

	* g++.dg/guality/pr82630.C: New test.

--- gcc/target.def.jj	2017-10-10 11:54:13.000000000 +0200
+++ gcc/target.def	2017-10-20 14:07:06.463135128 +0200
@@ -2822,7 +2822,7 @@ DEFHOOK
  "This hook should return true if @var{x} should not be emitted into\n\
 debug sections.",
  bool, (rtx x),
- hook_bool_rtx_false)
+ default_const_not_ok_for_debug_p)
 
 /* Given an address RTX, say whether it is valid.  */
 DEFHOOK
--- gcc/targhooks.c.jj	2017-10-13 19:02:08.000000000 +0200
+++ gcc/targhooks.c	2017-10-20 14:26:07.945464025 +0200
@@ -177,6 +177,14 @@ default_legitimize_address_displacement
   return false;
 }
 
+bool
+default_const_not_ok_for_debug_p (rtx x)
+{
+  if (GET_CODE (x) == UNSPEC)
+    return true;
+  return false;
+}
+
 rtx
 default_expand_builtin_saveregs (void)
 {
--- gcc/targhooks.h.jj	2017-10-13 19:02:08.000000000 +0200
+++ gcc/targhooks.h	2017-10-20 14:26:07.945464025 +0200
@@ -26,6 +26,7 @@ extern void default_external_libcall (rt
 extern rtx default_legitimize_address (rtx, rtx, machine_mode);
 extern bool default_legitimize_address_displacement (rtx *, rtx *,
 						     machine_mode);
+extern bool default_const_not_ok_for_debug_p (rtx);
 
 extern int default_unspec_may_trap_p (const_rtx, unsigned);
 extern machine_mode default_promote_function_mode (const_tree, machine_mode,
--- gcc/dwarf2out.c.jj	2017-10-19 16:18:44.000000000 +0200
+++ gcc/dwarf2out.c	2017-10-20 14:39:49.432647598 +0200
@@ -13740,9 +13740,17 @@ expansion_failed (tree expr, rtx rtl, ch
 static bool
 const_ok_for_output_1 (rtx rtl)
 {
-  if (GET_CODE (rtl) == UNSPEC)
+  if (targetm.const_not_ok_for_debug_p (rtl))
     {
-      /* If delegitimize_address couldn't do anything with the UNSPEC, assume
+      if (GET_CODE (rtl) != UNSPEC)
+	{
+	  expansion_failed (NULL_TREE, rtl,
+			    "Expression rejected for debug by the backend.\n");
+	  return false;
+	}
+
+      /* If delegitimize_address couldn't do anything with the UNSPEC, and
+	 the target hook doesn't explicitly allow it in debug info, assume
 	 we can't express it in the debug info.  */
       /* Don't complain about TLS UNSPECs, those are just too hard to
 	 delegitimize.  Note this could be a non-decl SYMBOL_REF such as
@@ -13769,13 +13777,6 @@ const_ok_for_output_1 (rtx rtl)
       return false;
     }
 
-  if (targetm.const_not_ok_for_debug_p (rtl))
-    {
-      expansion_failed (NULL_TREE, rtl,
-			"Expression rejected for debug by the backend.\n");
-      return false;
-    }
-
   /* FIXME: Refer to PR60655. It is possible for simplification
      of rtl expressions in var tracking to produce such expressions.
      We should really identify / validate expressions
--- gcc/config/arm/arm.c.jj	2017-10-19 16:18:44.000000000 +0200
+++ gcc/config/arm/arm.c	2017-10-20 14:28:06.302046887 +0200
@@ -30391,6 +30391,8 @@ arm_const_not_ok_for_debug_p (rtx p)
   tree decl_op0 = NULL;
   tree decl_op1 = NULL;
 
+  if (GET_CODE (p) == UNSPEC)
+    return true;
   if (GET_CODE (p) == MINUS)
     {
       if (GET_CODE (XEXP (p, 1)) == SYMBOL_REF)
--- gcc/config/powerpcspe/powerpcspe.c.jj	2017-10-17 22:10:10.000000000 +0200
+++ gcc/config/powerpcspe/powerpcspe.c	2017-10-20 14:28:40.823633544 +0200
@@ -9536,6 +9536,8 @@ rs6000_delegitimize_address (rtx orig_x)
 static bool
 rs6000_const_not_ok_for_debug_p (rtx x)
 {
+  if (GET_CODE (x) == UNSPEC)
+    return true;
   if (GET_CODE (x) == SYMBOL_REF
       && CONSTANT_POOL_ADDRESS_P (x))
     {
--- gcc/config/rs6000/rs6000.c.jj	2017-10-17 22:10:10.000000000 +0200
+++ gcc/config/rs6000/rs6000.c	2017-10-20 14:29:01.066391168 +0200
@@ -8985,6 +8985,8 @@ rs6000_delegitimize_address (rtx orig_x)
 static bool
 rs6000_const_not_ok_for_debug_p (rtx x)
 {
+  if (GET_CODE (x) == UNSPEC)
+    return true;
   if (GET_CODE (x) == SYMBOL_REF
       && CONSTANT_POOL_ADDRESS_P (x))
     {
--- gcc/config/i386/i386.c.jj	2017-10-20 09:16:10.000000000 +0200
+++ gcc/config/i386/i386.c	2017-10-20 14:53:19.520953612 +0200
@@ -16657,13 +16657,17 @@ ix86_delegitimize_address_1 (rtx x, bool
 	 movl foo@GOTOFF(%ecx), %edx
 	 in which case we return (%ecx - %ebx) + foo
 	 or (%ecx - _GLOBAL_OFFSET_TABLE_) + foo if pseudo_pic_reg
-	 and reload has completed.  */
+	 and reload has completed.  Don't do the latter for debug,
+	 as _GLOBAL_OFFSET_TABLE_ can't be expressed in the assembly.  */
       if (pic_offset_table_rtx
 	  && (!reload_completed || !ix86_use_pseudo_pic_reg ()))
         result = gen_rtx_PLUS (Pmode, gen_rtx_MINUS (Pmode, copy_rtx (addend),
 						     pic_offset_table_rtx),
 			       result);
-      else if (pic_offset_table_rtx && !TARGET_MACHO && !TARGET_VXWORKS_RTP)
+      else if (base_term_p
+	       && pic_offset_table_rtx
+	       && !TARGET_MACHO
+	       && !TARGET_VXWORKS_RTP)
 	{
 	  rtx tmp = gen_rtx_SYMBOL_REF (Pmode, GOT_SYMBOL_NAME);
 	  tmp = gen_rtx_MINUS (Pmode, copy_rtx (addend), tmp);
@@ -16716,6 +16720,25 @@ ix86_find_base_term (rtx x)
 
   return ix86_delegitimize_address_1 (x, true);
 }
+
+/* Return true if X shouldn't be emitted into the debug info.
+   Disallow UNSPECs other than @gotoff - we can't emit _GLOBAL_OFFSET_TABLE_
+   symbol easily into the .debug_info section, so we need not to
+   delegitimize, but instead assemble as @gotoff.
+   Disallow _GLOBAL_OFFSET_TABLE_ SYMBOL_REF - the assembler magically
+   assembles that as _GLOBAL_OFFSET_TABLE_-. expression.  */
+
+static bool
+ix86_const_not_ok_for_debug_p (rtx x)
+{
+  if (GET_CODE (x) == UNSPEC && XINT (x, 1) != UNSPEC_GOTOFF)
+    return true;
+
+  if (SYMBOL_REF_P (x) && strcmp (XSTR (x, 0), GOT_SYMBOL_NAME) == 0)
+    return true;
+
+  return false;
+}
 
 static void
 put_condition_code (enum rtx_code code, machine_mode mode, bool reverse,
@@ -18032,6 +18055,10 @@ i386_asm_output_addr_const_extra (FILE *
   op = XVECEXP (x, 0, 0);
   switch (XINT (x, 1))
     {
+    case UNSPEC_GOTOFF:
+      output_addr_const (file, op);
+      fputs ("@gotoff", file);
+      break;
     case UNSPEC_GOTTPOFF:
       output_addr_const (file, op);
       /* FIXME: This might be @TPOFF in Sun ld.  */
@@ -49415,6 +49442,9 @@ ix86_run_selftests (void)
 #undef TARGET_DELEGITIMIZE_ADDRESS
 #define TARGET_DELEGITIMIZE_ADDRESS ix86_delegitimize_address
 
+#undef TARGET_CONST_NOT_OK_FOR_DEBUG_P
+#define TARGET_CONST_NOT_OK_FOR_DEBUG_P ix86_const_not_ok_for_debug_p
+
 #undef TARGET_MS_BITFIELD_LAYOUT_P
 #define TARGET_MS_BITFIELD_LAYOUT_P ix86_ms_bitfield_layout_p
 
--- gcc/testsuite/g++.dg/guality/pr82630.C.jj	2017-10-20 15:23:37.978128740 +0200
+++ gcc/testsuite/g++.dg/guality/pr82630.C	2017-10-20 15:30:39.608050350 +0200
@@ -0,0 +1,58 @@
+// PR debug/82630
+// { dg-do run }
+// { dg-additional-options "-fPIC" { target fpic } }
+
+struct C
+{
+  int &c;
+  long d;
+  __attribute__((always_inline)) C (int &x) : c(x), d() {}
+};
+int v;
+
+__attribute__((noipa)) void
+fn1 (const void *x)
+{
+  asm volatile ("" : : "g" (x) : "memory");
+}
+
+__attribute__((noipa)) void
+fn2 (C x)
+{
+  int a = x.c + x.d;
+  asm volatile ("" : : "g" (a) : "memory");
+}
+
+__attribute__((noipa)) void
+fn3 (void)
+{
+  asm volatile ("" : : : "memory");
+}
+
+__attribute__((noipa))
+#ifdef __i386__
+__attribute__((regparm (2)))
+#endif
+static void
+fn4 (int *x, const char *y, C z)
+{
+  fn2 (C (*x));
+  fn1 ("baz");
+  fn2 (z);	// { dg-final { gdb-test 41 "y\[0\]" "'f'" } }
+  fn1 ("baz");	// { dg-final { gdb-test 41 "y\[1\]" "'o'" } }
+}
+
+__attribute__((noipa)) void
+fn5 (int *x)
+{
+  fn4 (x, "foo", C (*x));
+  fn3 ();
+}
+
+int
+main ()
+{
+  int a = 10;
+  fn5 (&a);
+  return 0;
+}

	Jakub