public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Improve debug info for various cases where we drop location info on the floor (PR debug/55608)
@ 2012-12-06 19:14 Jakub Jelinek
  2012-12-07 19:36 ` Cary Coutant
  0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2012-12-06 19:14 UTC (permalink / raw)
  To: Jason Merrill, Cary Coutant; +Cc: gcc-patches, Tom Tromey

Hi!

This patch adds DW_AT_const_value or DW_AT_location for unused static vars
(thus, not really modified and their DECL_INITIAL can be used for
location/constant value info), and optimizes various cases using
DW_OP_GNU_implicit_pointer (e.g. DW_OP_addr <symbol> DW_OP_stack_value
where symbol is either an optimized away static var (but with DW_AT_location
or DW_AT_const_value on it's DIE), which would be otherwise dropped
to avoid referencing non-existent symbol, can be replaced with
DW_OP_GNU_implicit_pointer to that DIE.  Or, if symbol above is a constant string
literal, we can create DW_TAG_dwarf_procedure with DW_AT_location
DW_OP_implicit_value (and, surprisingly, GDB handles that out of the box).

In cc1plus .debug_loc grew with this by about 3% and .debug_info grew by
0.25%, in libstdc++ the changes were in the noise.

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

2012-12-06  Jakub Jelinek  <jakub@redhat.com>

	PR debug/55608
	* dwarf2out.c (tree_add_const_value_attribute): Call ggc_free (array)
	on failure.
	(resolve_one_addr): Fail if referenced STRING_CST hasn't been written.
	(string_cst_pool_decl): New function.
	(resolve_addr_in_expr): Optimize DWARF location expression
	DW_OP_addr DW_OP_stack_value where DW_OP_addr refers to some variable
	which doesn't live in memory, but has DW_AT_location or
	DW_AT_const_value, or refers to a string literal, into
	DW_OP_GNU_implicit_pointer.
	(resolve_addr): If removing DW_AT_location of a variable because
	it was DW_OP_addr of address of the variable, but the variable doesn't
	live in memory, try to emit const value attribute for the initializer.

--- gcc/dwarf2out.c.jj	2012-11-21 18:44:25.847837373 +0100
+++ gcc/dwarf2out.c	2012-12-06 17:22:14.348761149 +0100
@@ -15492,6 +15492,7 @@ tree_add_const_value_attribute (dw_die_r
 	      add_AT_vec (die, DW_AT_const_value, size, 1, array);
 	      return true;
 	    }
+	  ggc_free (array);
 	}
     }
   return false;
@@ -22370,6 +22371,10 @@ resolve_one_addr (rtx *addr, void *data
       if (!rtl || !MEM_P (rtl))
 	return 1;
       rtl = XEXP (rtl, 0);
+      if (GET_CODE (rtl) == SYMBOL_REF
+	  && SYMBOL_REF_DECL (rtl)
+	  && !TREE_ASM_WRITTEN (SYMBOL_REF_DECL (rtl)))
+	return 1;
       vec_safe_push (used_rtx_array, rtl);
       *addr = rtl;
       return 0;
@@ -22394,6 +22399,46 @@ resolve_one_addr (rtx *addr, void *data
   return 0;
 }
 
+/* For STRING_CST, return SYMBOL_REF of its constant pool entry,
+   if possible, and create DW_TAG_dwarf_procedure that can be referenced
+   from DW_OP_GNU_implicit_pointer if the string hasn't been seen yet.  */
+
+static rtx
+string_cst_pool_decl (tree t)
+{
+  rtx rtl = output_constant_def (t, 1);
+  unsigned char *array;
+  dw_loc_descr_ref l;
+  tree decl;
+  size_t len;
+  dw_die_ref ref;
+
+  if (!rtl || !MEM_P (rtl))
+    return NULL_RTX;
+  rtl = XEXP (rtl, 0);
+  if (GET_CODE (rtl) != SYMBOL_REF
+      || SYMBOL_REF_DECL (rtl) == NULL_TREE)
+    return NULL_RTX;
+
+  decl = SYMBOL_REF_DECL (rtl);
+  if (lookup_decl_die (decl))
+    return rtl;
+
+  len = TREE_STRING_LENGTH (t);
+  vec_safe_push (used_rtx_array, rtl);
+  ref = new_die (DW_TAG_dwarf_procedure, comp_unit_die (), decl);
+  array = (unsigned char *) ggc_alloc_atomic (len);
+  memcpy (array, TREE_STRING_POINTER (t), len);
+  l = new_loc_descr (DW_OP_implicit_value, len, 0);
+  l->dw_loc_oprnd2.val_class = dw_val_class_vec;
+  l->dw_loc_oprnd2.v.val_vec.length = len;
+  l->dw_loc_oprnd2.v.val_vec.elt_size = 1;
+  l->dw_loc_oprnd2.v.val_vec.array = array;
+  add_AT_loc (ref, DW_AT_location, l);
+  equate_decl_number_to_die (decl, ref);
+  return rtl;
+}
+
 /* Helper function for resolve_addr, handle one location
    expression, return false if at least one CONST_STRING or SYMBOL_REF in
    the location list couldn't be resolved.  */
@@ -22402,23 +22447,81 @@ static bool
 resolve_addr_in_expr (dw_loc_descr_ref loc)
 {
   dw_loc_descr_ref keep = NULL;
-  for (; loc; loc = loc->dw_loc_next)
+  bool start_of_dw_expr = true;
+  for (; loc; start_of_dw_expr = (loc->dw_loc_opc == DW_OP_piece
+				  || loc->dw_loc_opc == DW_OP_bit_piece),
+	      loc = loc->dw_loc_next)
     switch (loc->dw_loc_opc)
       {
       case DW_OP_addr:
 	if (resolve_one_addr (&loc->dw_loc_oprnd1.v.val_addr, NULL))
-	  return false;
+	  {
+	    if (start_of_dw_expr
+		&& loc->dw_loc_next
+		&& loc->dw_loc_next->dw_loc_opc == DW_OP_stack_value
+		&& !dwarf_strict)
+	      {
+		rtx rtl = loc->dw_loc_oprnd1.v.val_addr;
+		HOST_WIDE_INT offset = 0;
+		dw_die_ref ref = NULL;
+		tree decl;
+
+		if (GET_CODE (rtl) == CONST
+		    && GET_CODE (XEXP (rtl, 0)) == PLUS
+		    && CONST_INT_P (XEXP (XEXP (rtl, 0), 1)))
+		  {
+		    offset = INTVAL (XEXP (XEXP (rtl, 0), 1));
+		    rtl = XEXP (XEXP (rtl, 0), 0);
+		  }
+		if (GET_CODE (rtl) == CONST_STRING)
+		  {
+		    size_t len = strlen (XSTR (rtl, 0)) + 1;
+		    tree t = build_string (len, XSTR (rtl, 0));
+		    tree tlen = size_int (len - 1);
+
+		    TREE_TYPE (t)
+		      = build_array_type (char_type_node,
+					  build_index_type (tlen));
+		    rtl = string_cst_pool_decl (t);
+		    if (!rtl)
+		      return false;
+		  }
+		if (GET_CODE (rtl) == SYMBOL_REF
+		    && SYMBOL_REF_DECL (rtl))
+		  {
+		    decl = SYMBOL_REF_DECL (rtl);
+		    if (TREE_CODE (decl) == VAR_DECL
+			&& !DECL_EXTERNAL (decl))
+		      {
+			ref = lookup_decl_die (decl);
+			if (ref
+			    && (get_AT (ref, DW_AT_location)
+				|| get_AT (ref, DW_AT_const_value)))
+			  {
+			    loc->dw_loc_opc = DW_OP_GNU_implicit_pointer;
+			    loc->dw_loc_oprnd1.val_class
+			      = dw_val_class_die_ref;
+			    loc->dw_loc_oprnd1.val_entry = NULL;
+			    loc->dw_loc_oprnd1.v.val_die_ref.die = ref;
+			    loc->dw_loc_oprnd1.v.val_die_ref.external = 0;
+			    loc->dw_loc_next = loc->dw_loc_next->dw_loc_next;
+			    loc->dw_loc_oprnd2.v.val_int = offset;
+			    break;
+			  }
+		      }
+		  }
+	      }
+	    return false;
+	  }
 	break;
       case DW_OP_GNU_addr_index:
       case DW_OP_GNU_const_index:
-        {
-          if ((loc->dw_loc_opc == DW_OP_GNU_addr_index
-               || (loc->dw_loc_opc == DW_OP_GNU_const_index && loc->dtprel))
-              && resolve_one_addr (&loc->dw_loc_oprnd1.val_entry->addr.rtl,
-                                   NULL))
-            return false;
-        }
-       break;
+	if ((loc->dw_loc_opc == DW_OP_GNU_addr_index
+	     || (loc->dw_loc_opc == DW_OP_GNU_const_index && loc->dtprel))
+	    && resolve_one_addr (&loc->dw_loc_oprnd1.val_entry->addr.rtl,
+				 NULL))
+	  return false;
+	break;
       case DW_OP_const4u:
       case DW_OP_const8u:
 	if (loc->dtprel
@@ -22601,8 +22704,79 @@ resolve_addr (dw_die_ref die)
 	       || l->dw_loc_next != NULL)
 	      && !resolve_addr_in_expr (l))
 	    {
-              if (dwarf_split_debug_info)
-                remove_loc_list_addr_table_entries (l);
+	      if (dwarf_split_debug_info)
+		remove_loc_list_addr_table_entries (l);
+	      if (l != NULL
+		  && l->dw_loc_next == NULL
+		  && l->dw_loc_opc == DW_OP_addr
+		  && GET_CODE (l->dw_loc_oprnd1.v.val_addr) == SYMBOL_REF
+		  && SYMBOL_REF_DECL (l->dw_loc_oprnd1.v.val_addr)
+		  && a->dw_attr == DW_AT_location)
+		{
+		  tree decl = SYMBOL_REF_DECL (l->dw_loc_oprnd1.v.val_addr);
+		  remove_AT (die, a->dw_attr);
+		  ix--;
+		  if (TREE_CODE (decl) == VAR_DECL
+		      && lookup_decl_die (decl) == die
+		      && !DECL_EXTERNAL (decl)
+		      && TREE_STATIC (decl)
+		      && DECL_INITIAL (decl)
+		      && !DECL_P (DECL_INITIAL (decl))
+		      && !get_AT (die, DW_AT_const_value))
+		    {
+		      tree init = DECL_INITIAL (decl);
+		      HOST_WIDE_INT offset = 0;
+		      /* For variables that have been optimized away and thus
+			 don't have a memory location, see if we can emit
+			 DW_AT_const_value instead.  */
+		      if (tree_add_const_value_attribute (die, init))
+			break;
+		      if (dwarf_strict)
+			break;
+		      STRIP_NOPS (init);
+		      if (TREE_CODE (init) == POINTER_PLUS_EXPR
+			  && host_integerp (TREE_OPERAND (init, 1), 0))
+			{
+			  offset = tree_low_cst (TREE_OPERAND (init, 1), 0);
+			  init = TREE_OPERAND (init, 0);
+			  STRIP_NOPS (init);
+			}
+		      if (TREE_CODE (init) != ADDR_EXPR)
+			break;
+		      if ((TREE_CODE (TREE_OPERAND (init, 0)) == STRING_CST
+			   && !TREE_ASM_WRITTEN (TREE_OPERAND (init, 0)))
+			  || (TREE_CODE (TREE_OPERAND (init, 0)) == VAR_DECL
+			      && !DECL_EXTERNAL (TREE_OPERAND (init, 0))
+			      && TREE_OPERAND (init, 0) != decl))
+			{
+			  dw_die_ref ref;
+			  tree decl;
+
+			  if (TREE_CODE (TREE_OPERAND (init, 0)) == STRING_CST)
+			    {
+			      rtx rtl
+				= string_cst_pool_decl (TREE_OPERAND (init, 0));
+			      if (!rtl)
+				break;
+			      decl = SYMBOL_REF_DECL (rtl);
+			    }
+			  else
+			    decl = TREE_OPERAND (init, 0);
+			  ref = lookup_decl_die (decl);
+			  if (ref == NULL
+			      || (!get_AT (ref, DW_AT_location)
+				  && !get_AT (ref, DW_AT_const_value)))
+			    break;
+			  l = new_loc_descr (DW_OP_GNU_implicit_pointer, 0,
+					     offset);
+			  l->dw_loc_oprnd1.val_class = dw_val_class_die_ref;
+			  l->dw_loc_oprnd1.v.val_die_ref.die = ref;
+			  l->dw_loc_oprnd1.v.val_die_ref.external = 0;
+			  add_AT_loc (die, DW_AT_location, l);
+			}
+		    }
+		  break;
+		}
 	      remove_AT (die, a->dw_attr);
 	      ix--;
 	    }

	Jakub

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Improve debug info for various cases where we drop location info on the floor (PR debug/55608)
  2012-12-06 19:14 [PATCH] Improve debug info for various cases where we drop location info on the floor (PR debug/55608) Jakub Jelinek
@ 2012-12-07 19:36 ` Cary Coutant
  2013-03-20 12:19   ` Jakub Jelinek
  0 siblings, 1 reply; 6+ messages in thread
From: Cary Coutant @ 2012-12-07 19:36 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches, Tom Tromey

> This patch adds DW_AT_const_value or DW_AT_location for unused static vars
> (thus, not really modified and their DECL_INITIAL can be used for
> location/constant value info), and optimizes various cases using
> DW_OP_GNU_implicit_pointer (e.g. DW_OP_addr <symbol> DW_OP_stack_value
> where symbol is either an optimized away static var (but with DW_AT_location
> or DW_AT_const_value on it's DIE), which would be otherwise dropped
> to avoid referencing non-existent symbol, can be replaced with
> DW_OP_GNU_implicit_pointer to that DIE.  Or, if symbol above is a constant string
> literal, we can create DW_TAG_dwarf_procedure with DW_AT_location
> DW_OP_implicit_value (and, surprisingly, GDB handles that out of the box).
>
> In cc1plus .debug_loc grew with this by about 3% and .debug_info grew by
> 0.25%, in libstdc++ the changes were in the noise.

Based on my reading of PR 55608, this isn't really a regression, and
it's definitely not a regression from 4.7 to 4.8. While the patch
looks OK to me, the size increase in .debug_loc and the scope of the
change suggest that this would be more appropriate for when Stage 1
reopens.

-cary

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Improve debug info for various cases where we drop location info on the floor (PR debug/55608)
  2012-12-07 19:36 ` Cary Coutant
@ 2013-03-20 12:19   ` Jakub Jelinek
  2013-03-20 18:22     ` Cary Coutant
  0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2013-03-20 12:19 UTC (permalink / raw)
  To: Cary Coutant; +Cc: Jason Merrill, gcc-patches, Tom Tromey

On Fri, Dec 07, 2012 at 11:36:24AM -0800, Cary Coutant wrote:
> > This patch adds DW_AT_const_value or DW_AT_location for unused static vars
> > (thus, not really modified and their DECL_INITIAL can be used for
> > location/constant value info), and optimizes various cases using
> > DW_OP_GNU_implicit_pointer (e.g. DW_OP_addr <symbol> DW_OP_stack_value
> > where symbol is either an optimized away static var (but with DW_AT_location
> > or DW_AT_const_value on it's DIE), which would be otherwise dropped
> > to avoid referencing non-existent symbol, can be replaced with
> > DW_OP_GNU_implicit_pointer to that DIE.  Or, if symbol above is a constant string
> > literal, we can create DW_TAG_dwarf_procedure with DW_AT_location
> > DW_OP_implicit_value (and, surprisingly, GDB handles that out of the box).
> >
> > In cc1plus .debug_loc grew with this by about 3% and .debug_info grew by
> > 0.25%, in libstdc++ the changes were in the noise.
> 
> Based on my reading of PR 55608, this isn't really a regression, and
> it's definitely not a regression from 4.7 to 4.8. While the patch
> looks OK to me, the size increase in .debug_loc and the scope of the
> change suggest that this would be more appropriate for when Stage 1
> reopens.

Now that Stage 1 reopened, is this ok for 4.9?

	Jakub

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Improve debug info for various cases where we drop location info on the floor (PR debug/55608)
  2013-03-20 12:19   ` Jakub Jelinek
@ 2013-03-20 18:22     ` Cary Coutant
  2013-03-21 12:47       ` Jakub Jelinek
  0 siblings, 1 reply; 6+ messages in thread
From: Cary Coutant @ 2013-03-20 18:22 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches, Tom Tromey

> Now that Stage 1 reopened, is this ok for 4.9?

Looks OK to me; I just had a few comments on the code readability.

> +  if (lookup_decl_die (decl))
> +    return rtl;
> +
> +  len = TREE_STRING_LENGTH (t);
> +  vec_safe_push (used_rtx_array, rtl);
> +  ref = new_die (DW_TAG_dwarf_procedure, comp_unit_die (), decl);
> +  array = (unsigned char *) ggc_alloc_atomic (len);
> +  memcpy (array, TREE_STRING_POINTER (t), len);
> +  l = new_loc_descr (DW_OP_implicit_value, len, 0);
> +  l->dw_loc_oprnd2.val_class = dw_val_class_vec;
> +  l->dw_loc_oprnd2.v.val_vec.length = len;
> +  l->dw_loc_oprnd2.v.val_vec.elt_size = 1;
> +  l->dw_loc_oprnd2.v.val_vec.array = array;
> +  add_AT_loc (ref, DW_AT_location, l);
> +  equate_decl_number_to_die (decl, ref);
> +  return rtl;

This is just a suggestion -- rewrite the above as:

  if (!lookup_decl_die (decl))
    {
      ...
    }

  return rtl;

This makes it more clear that you're actually returning the same thing
in either case, and that the creation of the dwarf procedure is just
a side effect. When I read through this the first time, I was expecting
the fall-through path to return something different just because of the
structure.

> -  for (; loc; loc = loc->dw_loc_next)
> +  bool start_of_dw_expr = true;
> +  for (; loc; start_of_dw_expr = (loc->dw_loc_opc == DW_OP_piece
> +                                 || loc->dw_loc_opc == DW_OP_bit_piece),
> +             loc = loc->dw_loc_next)

Another suggestion to make this a little less unwieldy:

  for (dw_loc_descr_ref prev = NULL; loc; prev = loc, loc = loc->dw_loc_next)
    ...
           if ((prev == NULL
                || prev->dw_loc_opc == DW_OP_piece
                || prev->dw_loc_opc == DW_OP_bit_piece)
               && loc->dw_loc_next
               && loc->dw_loc_next->dw_loc_opc == DW_OP_stack_value
               && !dwarf_strict)
             {
               ...
             }

Can you replace the { ... } with a function call? And a comment about
what you're doing here.

> +             if (l != NULL
> +                 && l->dw_loc_next == NULL
> +                 && l->dw_loc_opc == DW_OP_addr
> +                 && GET_CODE (l->dw_loc_oprnd1.v.val_addr) == SYMBOL_REF
> +                 && SYMBOL_REF_DECL (l->dw_loc_oprnd1.v.val_addr)
> +                 && a->dw_attr == DW_AT_location)
> +               {
> +                 ...
> +               }

Likewise here.

-cary

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Improve debug info for various cases where we drop location info on the floor (PR debug/55608)
  2013-03-20 18:22     ` Cary Coutant
@ 2013-03-21 12:47       ` Jakub Jelinek
  2013-03-21 17:16         ` Cary Coutant
  0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2013-03-21 12:47 UTC (permalink / raw)
  To: Cary Coutant; +Cc: Jason Merrill, gcc-patches, Tom Tromey

On Wed, Mar 20, 2013 at 11:21:57AM -0700, Cary Coutant wrote:
> > +  if (lookup_decl_die (decl))
> > +    return rtl;
> > +
> > +  len = TREE_STRING_LENGTH (t);
> > +  vec_safe_push (used_rtx_array, rtl);
> > +  ref = new_die (DW_TAG_dwarf_procedure, comp_unit_die (), decl);
> > +  array = (unsigned char *) ggc_alloc_atomic (len);
> > +  memcpy (array, TREE_STRING_POINTER (t), len);
> > +  l = new_loc_descr (DW_OP_implicit_value, len, 0);
> > +  l->dw_loc_oprnd2.val_class = dw_val_class_vec;
> > +  l->dw_loc_oprnd2.v.val_vec.length = len;
> > +  l->dw_loc_oprnd2.v.val_vec.elt_size = 1;
> > +  l->dw_loc_oprnd2.v.val_vec.array = array;
> > +  add_AT_loc (ref, DW_AT_location, l);
> > +  equate_decl_number_to_die (decl, ref);
> > +  return rtl;
> 
> This is just a suggestion -- rewrite the above as:

The reason for writing it that way was to decrease the indentation level,
but you're right, in this particular case nothing needs to be even wrapped

> > -  for (; loc; loc = loc->dw_loc_next)
> > +  bool start_of_dw_expr = true;
> > +  for (; loc; start_of_dw_expr = (loc->dw_loc_opc == DW_OP_piece
> > +                                 || loc->dw_loc_opc == DW_OP_bit_piece),
> > +             loc = loc->dw_loc_next)
> 
> Another suggestion to make this a little less unwieldy:

Ok.

>   for (dw_loc_descr_ref prev = NULL; loc; prev = loc, loc = loc->dw_loc_next)
>     ...
>            if ((prev == NULL
>                 || prev->dw_loc_opc == DW_OP_piece
>                 || prev->dw_loc_opc == DW_OP_bit_piece)
>                && loc->dw_loc_next
>                && loc->dw_loc_next->dw_loc_opc == DW_OP_stack_value
>                && !dwarf_strict)
>              {
>                ...
>              }
> 
> Can you replace the { ... } with a function call? And a comment about
> what you're doing here.

Done.

> > +             if (l != NULL
> > +                 && l->dw_loc_next == NULL
> > +                 && l->dw_loc_opc == DW_OP_addr
> > +                 && GET_CODE (l->dw_loc_oprnd1.v.val_addr) == SYMBOL_REF
> > +                 && SYMBOL_REF_DECL (l->dw_loc_oprnd1.v.val_addr)
> > +                 && a->dw_attr == DW_AT_location)
> > +               {
> > +                 ...
> > +               }
> 
> Likewise here.

Done.

Bootstrapped/regtested on x86_64-linux and i686-linux again, tested also on
the testcase from the PR (gdb apparently still hasn't been fixed so its
issues are still there, but the output looks good from gcc and at least
about half of the cases handles gdb right or partly right; what gdb doesn't
handle (prints <optimized>) is DW_OP_GNU_implicit_pointer to DIE with
DW_AT_const_value, and it would be nice if it was able to print implicit
pointer arrays or strings rather than just individual array entries or
characters).

Ok for trunk?

2013-03-21  Jakub Jelinek  <jakub@redhat.com>

	PR debug/55608
	* dwarf2out.c (tree_add_const_value_attribute): Call ggc_free (array)
	on failure.
	(resolve_one_addr): Fail if referenced STRING_CST hasn't been written.
	(string_cst_pool_decl): New function.
	(optimize_one_addr_into_implicit_ptr): New function.
	(resolve_addr_in_expr): Optimize DWARF location expression
	DW_OP_addr DW_OP_stack_value where DW_OP_addr refers to some variable
	which doesn't live in memory, but has DW_AT_location or
	DW_AT_const_value, or refers to a string literal, into
	DW_OP_GNU_implicit_pointer.
	(optimize_location_into_implicit_ptr): New function.
	(resolve_addr): If removing DW_AT_location of a variable because
	it was DW_OP_addr of address of the variable, but the variable doesn't
	live in memory, try to emit const value attribute for the initializer.

--- gcc/dwarf2out.c.jj	2013-03-17 17:26:07.894986420 +0100
+++ gcc/dwarf2out.c	2013-03-21 09:54:03.471200903 +0100
@@ -15527,6 +15527,7 @@ tree_add_const_value_attribute (dw_die_r
 	      add_AT_vec (die, DW_AT_const_value, size, 1, array);
 	      return true;
 	    }
+	  ggc_free (array);
 	}
     }
   return false;
@@ -22494,6 +22495,10 @@ resolve_one_addr (rtx *addr, void *data
       if (!rtl || !MEM_P (rtl))
 	return 1;
       rtl = XEXP (rtl, 0);
+      if (GET_CODE (rtl) == SYMBOL_REF
+	  && SYMBOL_REF_DECL (rtl)
+	  && !TREE_ASM_WRITTEN (SYMBOL_REF_DECL (rtl)))
+	return 1;
       vec_safe_push (used_rtx_array, rtl);
       *addr = rtl;
       return 0;
@@ -22518,6 +22523,103 @@ resolve_one_addr (rtx *addr, void *data
   return 0;
 }
 
+/* For STRING_CST, return SYMBOL_REF of its constant pool entry,
+   if possible, and create DW_TAG_dwarf_procedure that can be referenced
+   from DW_OP_GNU_implicit_pointer if the string hasn't been seen yet.  */
+
+static rtx
+string_cst_pool_decl (tree t)
+{
+  rtx rtl = output_constant_def (t, 1);
+  unsigned char *array;
+  dw_loc_descr_ref l;
+  tree decl;
+  size_t len;
+  dw_die_ref ref;
+
+  if (!rtl || !MEM_P (rtl))
+    return NULL_RTX;
+  rtl = XEXP (rtl, 0);
+  if (GET_CODE (rtl) != SYMBOL_REF
+      || SYMBOL_REF_DECL (rtl) == NULL_TREE)
+    return NULL_RTX;
+
+  decl = SYMBOL_REF_DECL (rtl);
+  if (!lookup_decl_die (decl))
+    {
+      len = TREE_STRING_LENGTH (t);
+      vec_safe_push (used_rtx_array, rtl);
+      ref = new_die (DW_TAG_dwarf_procedure, comp_unit_die (), decl);
+      array = (unsigned char *) ggc_alloc_atomic (len);
+      memcpy (array, TREE_STRING_POINTER (t), len);
+      l = new_loc_descr (DW_OP_implicit_value, len, 0);
+      l->dw_loc_oprnd2.val_class = dw_val_class_vec;
+      l->dw_loc_oprnd2.v.val_vec.length = len;
+      l->dw_loc_oprnd2.v.val_vec.elt_size = 1;
+      l->dw_loc_oprnd2.v.val_vec.array = array;
+      add_AT_loc (ref, DW_AT_location, l);
+      equate_decl_number_to_die (decl, ref);
+    }
+  return rtl;
+}
+
+/* Helper function of resolve_addr_in_expr.  LOC is
+   a DW_OP_addr followed by DW_OP_stack_value, either at the start
+   of exprloc or after DW_OP_{,bit_}piece, and val_addr can't be
+   resolved.  Replace it (both DW_OP_addr and DW_OP_stack_value)
+   with DW_OP_GNU_implicit_pointer if possible
+   and return true, if unsuccesful, return false.  */
+
+static bool
+optimize_one_addr_into_implicit_ptr (dw_loc_descr_ref loc)
+{
+  rtx rtl = loc->dw_loc_oprnd1.v.val_addr;
+  HOST_WIDE_INT offset = 0;
+  dw_die_ref ref = NULL;
+  tree decl;
+
+  if (GET_CODE (rtl) == CONST
+      && GET_CODE (XEXP (rtl, 0)) == PLUS
+      && CONST_INT_P (XEXP (XEXP (rtl, 0), 1)))
+    {
+      offset = INTVAL (XEXP (XEXP (rtl, 0), 1));
+      rtl = XEXP (XEXP (rtl, 0), 0);
+    }
+  if (GET_CODE (rtl) == CONST_STRING)
+    {
+      size_t len = strlen (XSTR (rtl, 0)) + 1;
+      tree t = build_string (len, XSTR (rtl, 0));
+      tree tlen = size_int (len - 1);
+
+      TREE_TYPE (t)
+	= build_array_type (char_type_node, build_index_type (tlen));
+      rtl = string_cst_pool_decl (t);
+      if (!rtl)
+	return false;
+    }
+  if (GET_CODE (rtl) == SYMBOL_REF && SYMBOL_REF_DECL (rtl))
+    {
+      decl = SYMBOL_REF_DECL (rtl);
+      if (TREE_CODE (decl) == VAR_DECL && !DECL_EXTERNAL (decl))
+	{
+	  ref = lookup_decl_die (decl);
+	  if (ref && (get_AT (ref, DW_AT_location)
+		      || get_AT (ref, DW_AT_const_value)))
+	    {
+	      loc->dw_loc_opc = DW_OP_GNU_implicit_pointer;
+	      loc->dw_loc_oprnd1.val_class = dw_val_class_die_ref;
+	      loc->dw_loc_oprnd1.val_entry = NULL;
+	      loc->dw_loc_oprnd1.v.val_die_ref.die = ref;
+	      loc->dw_loc_oprnd1.v.val_die_ref.external = 0;
+	      loc->dw_loc_next = loc->dw_loc_next->dw_loc_next;
+	      loc->dw_loc_oprnd2.v.val_int = offset;
+	      return true;
+	    }
+	}
+    }
+  return false;
+}
+
 /* Helper function for resolve_addr, handle one location
    expression, return false if at least one CONST_STRING or SYMBOL_REF in
    the location list couldn't be resolved.  */
@@ -22526,23 +22628,31 @@ static bool
 resolve_addr_in_expr (dw_loc_descr_ref loc)
 {
   dw_loc_descr_ref keep = NULL;
-  for (; loc; loc = loc->dw_loc_next)
+  for (dw_loc_descr_ref prev = NULL; loc; prev = loc, loc = loc->dw_loc_next)
     switch (loc->dw_loc_opc)
       {
       case DW_OP_addr:
 	if (resolve_one_addr (&loc->dw_loc_oprnd1.v.val_addr, NULL))
-	  return false;
+	  {
+	    if ((prev == NULL
+		 || prev->dw_loc_opc == DW_OP_piece
+		 || prev->dw_loc_opc == DW_OP_bit_piece)
+		&& loc->dw_loc_next
+		&& loc->dw_loc_next->dw_loc_opc == DW_OP_stack_value
+		&& !dwarf_strict
+		&& optimize_one_addr_into_implicit_ptr (loc))
+	      break;
+	    return false;
+	  }
 	break;
       case DW_OP_GNU_addr_index:
       case DW_OP_GNU_const_index:
-        {
-          if ((loc->dw_loc_opc == DW_OP_GNU_addr_index
-               || (loc->dw_loc_opc == DW_OP_GNU_const_index && loc->dtprel))
-              && resolve_one_addr (&loc->dw_loc_oprnd1.val_entry->addr.rtl,
-                                   NULL))
-            return false;
-        }
-       break;
+	if ((loc->dw_loc_opc == DW_OP_GNU_addr_index
+	     || (loc->dw_loc_opc == DW_OP_GNU_const_index && loc->dtprel))
+	    && resolve_one_addr (&loc->dw_loc_oprnd1.val_entry->addr.rtl,
+				 NULL))
+	  return false;
+	break;
       case DW_OP_const4u:
       case DW_OP_const8u:
 	if (loc->dtprel
@@ -22637,6 +22747,80 @@ resolve_addr_in_expr (dw_loc_descr_ref l
   return true;
 }
 
+/* Helper function of resolve_addr.  DIE had DW_AT_location of
+   DW_OP_addr alone, which referred to DECL in DW_OP_addr's operand
+   and DW_OP_addr couldn't be resolved.  resolve_addr has already
+   removed the DW_AT_location attribute.  This function attempts to
+   add a new DW_AT_location attribute with DW_OP_GNU_implicit_pointer
+   to it or DW_AT_const_value attribute, if possible.  */
+
+static void
+optimize_location_into_implicit_ptr (dw_die_ref die, tree decl)
+{
+  if (TREE_CODE (decl) != VAR_DECL
+      || lookup_decl_die (decl) != die
+      || DECL_EXTERNAL (decl)
+      || !TREE_STATIC (decl)
+      || DECL_INITIAL (decl) == NULL_TREE
+      || DECL_P (DECL_INITIAL (decl))
+      || get_AT (die, DW_AT_const_value))
+    return;
+
+  tree init = DECL_INITIAL (decl);
+  HOST_WIDE_INT offset = 0;
+  /* For variables that have been optimized away and thus
+     don't have a memory location, see if we can emit
+     DW_AT_const_value instead.  */
+  if (tree_add_const_value_attribute (die, init))
+    return;
+  if (dwarf_strict)
+    return;
+  /* If init is ADDR_EXPR or POINTER_PLUS_EXPR of ADDR_EXPR,
+     and ADDR_EXPR refers to a decl that has DW_AT_location or
+     DW_AT_const_value (but isn't addressable, otherwise
+     resolving the original DW_OP_addr wouldn't fail), see if
+     we can add DW_OP_GNU_implicit_pointer.  */
+  STRIP_NOPS (init);
+  if (TREE_CODE (init) == POINTER_PLUS_EXPR
+      && host_integerp (TREE_OPERAND (init, 1), 0))
+    {
+      offset = tree_low_cst (TREE_OPERAND (init, 1), 0);
+      init = TREE_OPERAND (init, 0);
+      STRIP_NOPS (init);
+    }
+  if (TREE_CODE (init) != ADDR_EXPR)
+    return;
+  if ((TREE_CODE (TREE_OPERAND (init, 0)) == STRING_CST
+       && !TREE_ASM_WRITTEN (TREE_OPERAND (init, 0)))
+      || (TREE_CODE (TREE_OPERAND (init, 0)) == VAR_DECL
+	  && !DECL_EXTERNAL (TREE_OPERAND (init, 0))
+	  && TREE_OPERAND (init, 0) != decl))
+    {
+      dw_die_ref ref;
+      dw_loc_descr_ref l;
+
+      if (TREE_CODE (TREE_OPERAND (init, 0)) == STRING_CST)
+	{
+	  rtx rtl = string_cst_pool_decl (TREE_OPERAND (init, 0));
+	  if (!rtl)
+	    return;
+	  decl = SYMBOL_REF_DECL (rtl);
+	}
+      else
+	decl = TREE_OPERAND (init, 0);
+      ref = lookup_decl_die (decl);
+      if (ref == NULL
+	  || (!get_AT (ref, DW_AT_location)
+	      && !get_AT (ref, DW_AT_const_value)))
+	return;
+      l = new_loc_descr (DW_OP_GNU_implicit_pointer, 0, offset);
+      l->dw_loc_oprnd1.val_class = dw_val_class_die_ref;
+      l->dw_loc_oprnd1.v.val_die_ref.die = ref;
+      l->dw_loc_oprnd1.v.val_die_ref.external = 0;
+      add_AT_loc (die, DW_AT_location, l);
+    }
+}
+
 /* Resolve DW_OP_addr and DW_AT_const_value CONST_STRING arguments to
    an address in .rodata section if the string literal is emitted there,
    or remove the containing location list or replace DW_AT_const_value
@@ -22723,8 +22907,21 @@ resolve_addr (dw_die_ref die)
 	       || l->dw_loc_next != NULL)
 	      && !resolve_addr_in_expr (l))
 	    {
-              if (dwarf_split_debug_info)
-                remove_loc_list_addr_table_entries (l);
+	      if (dwarf_split_debug_info)
+		remove_loc_list_addr_table_entries (l);
+	      if (l != NULL
+		  && l->dw_loc_next == NULL
+		  && l->dw_loc_opc == DW_OP_addr
+		  && GET_CODE (l->dw_loc_oprnd1.v.val_addr) == SYMBOL_REF
+		  && SYMBOL_REF_DECL (l->dw_loc_oprnd1.v.val_addr)
+		  && a->dw_attr == DW_AT_location)
+		{
+		  tree decl = SYMBOL_REF_DECL (l->dw_loc_oprnd1.v.val_addr);
+		  remove_AT (die, a->dw_attr);
+		  ix--;
+		  optimize_location_into_implicit_ptr (die, decl);
+		  break;
+		}
 	      remove_AT (die, a->dw_attr);
 	      ix--;
 	    }


	Jakub

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Improve debug info for various cases where we drop location info on the floor (PR debug/55608)
  2013-03-21 12:47       ` Jakub Jelinek
@ 2013-03-21 17:16         ` Cary Coutant
  0 siblings, 0 replies; 6+ messages in thread
From: Cary Coutant @ 2013-03-21 17:16 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches, Tom Tromey

On Thu, Mar 21, 2013 at 5:47 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Bootstrapped/regtested on x86_64-linux and i686-linux again, tested also on
> the testcase from the PR (gdb apparently still hasn't been fixed so its
> issues are still there, but the output looks good from gcc and at least
> about half of the cases handles gdb right or partly right; what gdb doesn't
> handle (prints <optimized>) is DW_OP_GNU_implicit_pointer to DIE with
> DW_AT_const_value, and it would be nice if it was able to print implicit
> pointer arrays or strings rather than just individual array entries or
> characters).
>
> Ok for trunk?

Thanks, this is OK.

-cary

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-03-21 17:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-06 19:14 [PATCH] Improve debug info for various cases where we drop location info on the floor (PR debug/55608) Jakub Jelinek
2012-12-07 19:36 ` Cary Coutant
2013-03-20 12:19   ` Jakub Jelinek
2013-03-20 18:22     ` Cary Coutant
2013-03-21 12:47       ` Jakub Jelinek
2013-03-21 17:16         ` Cary Coutant

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