diff mbox

Fix PR debug/66728

Message ID 87io88920k.fsf@e105548-lin.cambridge.arm.com
State New
Headers show

Commit Message

Richard Sandiford Aug. 21, 2015, 1:58 p.m. UTC 
This is yet another bug caused by rtx having modeless scalar integer
constants.  We need to use context to find the actual mode of a
CONST_INT or CONST_WIDE_INT.

Getting a mode is especially awkward here.  Decls have two modes
associated with them: TYPE_MODE (TREE_TYPE (decl)) and DECL_MODE (decl).
Promotion via things like promote_decl_mode can lead to the rtl having
a mode that is different from both of them.  Sometimes structure decls
have BLKmode but are assigned an integer-mode rtl (e.g. when passing
3-byte structures by value to functions).

I think in this case we're just going to have to assume that none of
these fancy mode changes happen for something that's big enough to
need a CONST_WIDE_INT.

loc_descriptor refuses to use CONST_INT for BLKmode decls (which aren't
actually integers at the source level).  That seems like the right
behaviour, so this patch does that for add_const_value_attribute too.
It asserts that the mode is otherwise sensible for both CONST_INT
and CONST_WIDE_INT.  Asserting for CONST_INT isn't strictly necessary
but means that the assumption will get much more coverage than asserting
only for CONST_WIDE_INT does.

Tested on x86_64-linux-gnu and aarch64-linux-gnu.  Also tested against
the gdb testsuite.  OK to install?

Thanks,
Richard

gcc/
	PR debug/66728
	* dwarf2out.c (loc_descriptor): Remove redundant GET_MODE of
	CONST_WIDE_INTs.  Handle BLKmode for CONST_WIDE_INT too.
	(add_const_value_attribute): Add a mode parameter.
	Check it for CONST_INT and CONST_WIDE_INT.  Use it to build
	wide_int values.
	(add_location_or_const_value_attribute): Update call.
	(tree_add_const_value_attribute): Likewise.

gcc/testsuite/
	PR debug/66728
	* gcc.dg/debug/dwarf2/pr66728.c: New test.

Comments

Jeff Law Aug. 21, 2015, 4:13 p.m. UTC | #1 
On 08/21/2015 07:58 AM, Richard Sandiford wrote:
> This is yet another bug caused by rtx having modeless scalar integer
> constants.  We need to use context to find the actual mode of a
> CONST_INT or CONST_WIDE_INT.
We've been bumping into these as long as I can remember.  I suspect I 
still have a great essay from Jim on why it's hard to fix on a hard 
drive from one of my old HP boxes from the early 90s ;-)


> gcc/
> 	PR debug/66728
> 	* dwarf2out.c (loc_descriptor): Remove redundant GET_MODE of
> 	CONST_WIDE_INTs.  Handle BLKmode for CONST_WIDE_INT too.
> 	(add_const_value_attribute): Add a mode parameter.
> 	Check it for CONST_INT and CONST_WIDE_INT.  Use it to build
> 	wide_int values.
> 	(add_location_or_const_value_attribute): Update call.
> 	(tree_add_const_value_attribute): Likewise.
I'll have to defer to Jason & Jakub on this.  Note Jakub is on PTO right 
now.

jeff
diff mbox

Patch

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index d9d3063..383d705 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -3259,7 +3259,7 @@  static HOST_WIDE_INT field_byte_offset (const_tree);
 static void add_AT_location_description	(dw_die_ref, enum dwarf_attribute,
 					 dw_loc_list_ref);
 static void add_data_member_location_attribute (dw_die_ref, tree);
-static bool add_const_value_attribute (dw_die_ref, rtx);
+static bool add_const_value_attribute (dw_die_ref, machine_mode, rtx);
 static void insert_int (HOST_WIDE_INT, unsigned, unsigned char *);
 static void insert_wide_int (const wide_int &, unsigned char *, int);
 static void insert_float (const_rtx, unsigned char *);
@@ -13795,10 +13795,9 @@  loc_descriptor (rtx rtl, machine_mode mode,
       break;
 
     case CONST_WIDE_INT:
-      if (mode == VOIDmode)
-	mode = GET_MODE (rtl);
-
-      if (mode != VOIDmode && (dwarf_version >= 4 || !dwarf_strict))
+      if (mode != VOIDmode
+	  && mode != BLKmode
+	  && (dwarf_version >= 4 || !dwarf_strict))
 	{
 	  loc_result = new_loc_descr (DW_OP_implicit_value,
 				      GET_MODE_SIZE (mode), 0);
@@ -15576,25 +15575,33 @@  insert_float (const_rtx rtl, unsigned char *array)
    constants do not necessarily get memory "homes".  */
 
 static bool
-add_const_value_attribute (dw_die_ref die, rtx rtl)
+add_const_value_attribute (dw_die_ref die, machine_mode mode, rtx rtl)
 {
   switch (GET_CODE (rtl))
     {
     case CONST_INT:
-      {
-	HOST_WIDE_INT val = INTVAL (rtl);
+      if (mode != BLKmode)
+	{
+	  gcc_checking_assert (SCALAR_INT_MODE_P (mode));
+	  HOST_WIDE_INT val = INTVAL (rtl);
 
-	if (val < 0)
-	  add_AT_int (die, DW_AT_const_value, val);
-	else
-	  add_AT_unsigned (die, DW_AT_const_value, (unsigned HOST_WIDE_INT) val);
-      }
-      return true;
+	  if (val < 0)
+	    add_AT_int (die, DW_AT_const_value, val);
+	  else
+	    add_AT_unsigned (die, DW_AT_const_value,
+			     (unsigned HOST_WIDE_INT) val);
+	  return true;
+	}
+      return false;
 
     case CONST_WIDE_INT:
-      add_AT_wide (die, DW_AT_const_value,
-		   std::make_pair (rtl, GET_MODE (rtl)));
-      return true;
+      if (mode != BLKmode)
+	{
+	  gcc_checking_assert (SCALAR_INT_MODE_P (mode));
+	  add_AT_wide (die, DW_AT_const_value, std::make_pair (rtl, mode));
+	  return true;
+	}
+      return false;
 
     case CONST_DOUBLE:
       /* Note that a CONST_DOUBLE rtx could represent either an integer or a
@@ -15671,7 +15678,7 @@  add_const_value_attribute (dw_die_ref die, rtx rtl)
 
     case CONST:
       if (CONSTANT_P (XEXP (rtl, 0)))
-	return add_const_value_attribute (die, XEXP (rtl, 0));
+	return add_const_value_attribute (die, mode, XEXP (rtl, 0));
       /* FALLTHROUGH */
     case SYMBOL_REF:
       if (!const_ok_for_output (rtl))
@@ -16171,7 +16178,7 @@  add_location_or_const_value_attribute (dw_die_ref die, tree decl, bool cache_p,
 
   rtl = rtl_for_decl_location (decl);
   if (rtl && (CONSTANT_P (rtl) || GET_CODE (rtl) == CONST_STRING)
-      && add_const_value_attribute (die, rtl))
+      && add_const_value_attribute (die, DECL_MODE (decl), rtl))
     return true;
 
   /* See if we have single element location list that is equivalent to
@@ -16192,7 +16199,7 @@  add_location_or_const_value_attribute (dw_die_ref die, tree decl, bool cache_p,
       if (GET_CODE (rtl) == EXPR_LIST)
 	rtl = XEXP (rtl, 0);
       if ((CONSTANT_P (rtl) || GET_CODE (rtl) == CONST_STRING)
-	  && add_const_value_attribute (die, rtl))
+	  && add_const_value_attribute (die, DECL_MODE (decl), rtl))
 	 return true;
     }
   /* If this decl is from BLOCK_NONLOCALIZED_VARS, we might need its
@@ -16395,7 +16402,7 @@  tree_add_const_value_attribute (dw_die_ref die, tree t)
 
   rtl = rtl_for_decl_init (init, type);
   if (rtl)
-    return add_const_value_attribute (die, rtl);
+    return add_const_value_attribute (die, TYPE_MODE (type), rtl);
   /* If the host and target are sane, try harder.  */
   else if (CHAR_BIT == 8 && BITS_PER_UNIT == 8
 	   && initializer_constant_valid_p (init, type))
diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/pr66728.c b/gcc/testsuite/gcc.dg/debug/dwarf2/pr66728.c
new file mode 100644
index 0000000..ba41e97
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/pr66728.c
@@ -0,0 +1,14 @@ 
+/* { dg-do compile { target { x86_64-*-* && lp64 } } } */
+/* { dg-options "-O -gdwarf -dA" } */
+
+__uint128_t
+test (void)
+{
+  static const __uint128_t foo = ((((__uint128_t) 0x22334455) << 96)
+				  | 0x99aabb);
+
+  return foo;
+}
+
+/* { dg-final { scan-assembler {\.quad\t0x99aabb\t# DW_AT_const_value} } } */
+/* { dg-final { scan-assembler {\.quad\t0x2233445500000000\t} } } */