[VTA, PR41473] drop NULL locations from lists

[VTA, PR41473] drop NULL locations from lists

[Alexandre Oliva]

[-- Attachment #1: Type: text/plain, Size: 726 bytes --]

Entries in location lists whose RTL is NULL or not representable end up
causing us to waste debug info space emitting empty DW_AT_location.

This patch optimizes the debug information, omitting the useless
DW_AT_location.  This should work around the bug in Darwin's dsymutil.

While at that, I arranged for entries in location lists that are not
representable in debug info to not waste an entry in the output location
list.

The patch also fixes the handling of CONST_DOUBLEs and CONST_VECTORs
within CONCAT and CONCATN, that was currently disabled because mode was
specified as VOIDmode by the handlers of these RTL forms.  Even with
-gdwarf-4, we'd still have failed to emit them.

Ok to install if this passes regstrap?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vta-dwarf2out-omit-empty-loc-pr41473.patch --]
[-- Type: text/x-diff, Size: 6099 bytes --]

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/41473
	* dwarf2out.c (loc_descriptor): Infer mode from CONST_DOUBLEs
	and CONST_VECTORs.
	(dw_loc_list): Don't create entries without location.  Don't
	special-case the first node of the list.
	(loc_list_from_tree): Don't use DECL_RTL if loc_list is nonempty.

Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c.orig	2009-11-23 17:16:14.000000000 -0200
+++ gcc/dwarf2out.c	2009-11-23 17:52:38.000000000 -0200
@@ -13650,15 +13650,17 @@ loc_descriptor (rtx rtl, enum machine_mo
       break;
 
     case CONST_DOUBLE:
+      if (mode == VOIDmode)
+	mode = GET_MODE (rtl);
+
       if (mode != VOIDmode && (dwarf_version >= 4 || !dwarf_strict))
 	{
+	  gcc_assert (mode == GET_MODE (rtl));
+
 	  /* Note that a CONST_DOUBLE rtx could represent either an integer
 	     or a floating-point constant.  A CONST_DOUBLE is used whenever
 	     the constant requires more than one word in order to be
 	     adequately represented.  We output CONST_DOUBLEs as blocks.  */
-	  if (GET_MODE (rtl) != VOIDmode)
-	    mode = GET_MODE (rtl);
-
 	  loc_result = new_loc_descr (DW_OP_implicit_value,
 				      GET_MODE_SIZE (mode), 0);
 	  if (SCALAR_FLOAT_MODE_P (mode))
@@ -13684,6 +13686,9 @@ loc_descriptor (rtx rtl, enum machine_mo
       break;
 
     case CONST_VECTOR:
+      if (mode == VOIDmode)
+	mode = GET_MODE (rtl);
+
       if (mode != VOIDmode && (dwarf_version >= 4 || !dwarf_strict))
 	{
 	  unsigned int elt_size = GET_MODE_UNIT_SIZE (GET_MODE (rtl));
@@ -13692,7 +13697,7 @@ loc_descriptor (rtx rtl, enum machine_mo
 	  unsigned int i;
 	  unsigned char *p;
 
-	  mode = GET_MODE (rtl);
+	  gcc_assert (mode == GET_MODE (rtl));
 	  switch (GET_MODE_CLASS (mode))
 	    {
 	    case MODE_VECTOR_INT:
@@ -13928,20 +13933,21 @@ dw_loc_list_1 (tree loc, rtx varloc, int
   return descr;
 }
 
-/* Return dwarf representation of location list representing for
-   LOC_LIST of DECL.  WANT_ADDRESS has the same meaning as in
-   loc_list_from_tree function.  */
+/* Return the dwarf representation of the location list LOC_LIST of
+   DECL.  WANT_ADDRESS has the same meaning as in loc_list_from_tree
+   function.  */
 
 static dw_loc_list_ref
-dw_loc_list (var_loc_list * loc_list, tree decl, int want_address)
+dw_loc_list (var_loc_list *loc_list, tree decl, int want_address)
 {
   const char *endname, *secname;
-  dw_loc_list_ref list;
   rtx varloc;
   enum var_init_status initialized;
   struct var_loc_node *node;
   dw_loc_descr_ref descr;
   char label_id[MAX_ARTIFICIAL_LABEL_BYTES];
+  dw_loc_list_ref list = NULL;
+  dw_loc_list_ref *listp = &list;
 
   /* Now that we know what section we are using for a base,
      actually construct the list of locations.
@@ -13954,26 +13960,9 @@ dw_loc_list (var_loc_list * loc_list, tr
      This means we have to special case the last node, and generate
      a range of [last location start, end of function label].  */
 
-  node = loc_list->first;
   secname = secname_for_decl (decl);
 
-  if (NOTE_VAR_LOCATION_LOC (node->var_loc_note))
-    initialized = NOTE_VAR_LOCATION_STATUS (node->var_loc_note);
-  else
-    initialized = VAR_INIT_STATUS_INITIALIZED;
-  varloc = NOTE_VAR_LOCATION (node->var_loc_note);
-  descr = dw_loc_list_1 (decl, varloc, want_address, initialized);
-
-  if (loc_list && loc_list->first != loc_list->last)
-    list = new_loc_list (descr, node->label, node->next->label, secname, 1);
-  else
-    return single_element_loc_list (descr);
-  node = node->next;
-
-  if (!node)
-    return NULL;
-
-  for (; node->next; node = node->next)
+  for (node = loc_list->first; node->next; node = node->next)
     if (NOTE_VAR_LOCATION_LOC (node->var_loc_note) != NULL_RTX)
       {
 	/* The variable has a location between NODE->LABEL and
@@ -13981,28 +13970,48 @@ dw_loc_list (var_loc_list * loc_list, tr
 	initialized = NOTE_VAR_LOCATION_STATUS (node->var_loc_note);
 	varloc = NOTE_VAR_LOCATION (node->var_loc_note);
 	descr = dw_loc_list_1 (decl, varloc, want_address, initialized);
-	add_loc_descr_to_loc_list (&list, descr,
-				   node->label, node->next->label, secname);
+	if (descr)
+	  {
+	    if (list)
+	      add_loc_descr_to_loc_list (listp, descr,
+					 node->label, node->next->label,
+					 secname);
+	    else
+	      *listp = new_loc_list (descr, node->label, node->next->label,
+				     secname, 1);
+	    /* Speed up the next insertion.  */
+	    listp = &(*listp)->dw_loc_next;
+	  }
       }
 
   /* If the variable has a location at the last label
      it keeps its location until the end of function.  */
   if (NOTE_VAR_LOCATION_LOC (node->var_loc_note) != NULL_RTX)
     {
-      if (!current_function_decl)
-	endname = text_end_label;
-      else
-	{
-	  ASM_GENERATE_INTERNAL_LABEL (label_id, FUNC_END_LABEL,
-				       current_function_funcdef_no);
-	  endname = ggc_strdup (label_id);
-	}
-
       initialized = NOTE_VAR_LOCATION_STATUS (node->var_loc_note);
       varloc = NOTE_VAR_LOCATION (node->var_loc_note);
       descr = dw_loc_list_1 (decl, varloc, want_address, initialized);
-      add_loc_descr_to_loc_list (&list, descr, node->label, endname, secname);
+      if (descr)
+	{
+	  if (!current_function_decl)
+	    endname = text_end_label;
+	  else
+	    {
+	      ASM_GENERATE_INTERNAL_LABEL (label_id, FUNC_END_LABEL,
+					   current_function_funcdef_no);
+	      endname = ggc_strdup (label_id);
+	    }
+
+	  if (*listp)
+	    add_loc_descr_to_loc_list (listp, descr,
+				       node->label, endname,
+				       secname);
+	  else
+	    *listp = new_loc_list (descr, node->label, endname,
+				   secname, 1);
+	}
     }
+
   return list;
 }
 
@@ -14312,9 +14321,9 @@ loc_list_from_tree (tree loc, int want_a
 	rtx rtl;
 	var_loc_list *loc_list = lookup_decl_loc (loc);
 
-	if (loc_list && loc_list->first
-	    && (list_ret = dw_loc_list (loc_list, loc, want_address)))
+	if (loc_list && loc_list->first)
 	  {
+	    list_ret = dw_loc_list (loc_list, loc, want_address);
 	    have_address = want_address != 0;
 	    break;
 	  }

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

Re: [VTA, PR41473] drop NULL locations from lists

[Jakub Jelinek]

On Mon, Nov 23, 2009 at 06:11:57PM -0200, Alexandre Oliva wrote:
> Entries in location lists whose RTL is NULL or not representable end up
> causing us to waste debug info space emitting empty DW_AT_location.
> 
> This patch optimizes the debug information, omitting the useless
> DW_AT_location.  This should work around the bug in Darwin's dsymutil.
> 
> While at that, I arranged for entries in location lists that are not
> representable in debug info to not waste an entry in the output location
> list.
> 
> The patch also fixes the handling of CONST_DOUBLEs and CONST_VECTORs
> within CONCAT and CONCATN, that was currently disabled because mode was
> specified as VOIDmode by the handlers of these RTL forms.  Even with
> -gdwarf-4, we'd still have failed to emit them.
> 
> Ok to install if this passes regstrap?
> 

Are you sure the dropping of single_element_loc_list call in the
loc->first && loc->last == loc->first case is a good idea?
I believe that will force using location lists even when block form could be
used for DW_AT_location.  If loc->first->descr is NULL, the function should
just return NULL, otherwise it should create the special single element list
so that add_AT_location_description does the right thing.

	Jakub

Re: [VTA, PR41473] drop NULL locations from lists

[Jack Howarth]

On Mon, Nov 23, 2009 at 03:40:16PM -0500, Jakub Jelinek wrote:
> On Mon, Nov 23, 2009 at 06:11:57PM -0200, Alexandre Oliva wrote:
> > Entries in location lists whose RTL is NULL or not representable end up
> > causing us to waste debug info space emitting empty DW_AT_location.
> > 
> > This patch optimizes the debug information, omitting the useless
> > DW_AT_location.  This should work around the bug in Darwin's dsymutil.
> > 
> > While at that, I arranged for entries in location lists that are not
> > representable in debug info to not waste an entry in the output location
> > list.
> > 
> > The patch also fixes the handling of CONST_DOUBLEs and CONST_VECTORs
> > within CONCAT and CONCATN, that was currently disabled because mode was
> > specified as VOIDmode by the handlers of these RTL forms.  Even with
> > -gdwarf-4, we'd still have failed to emit them.
> > 
> > Ok to install if this passes regstrap?
> > 
> 
> Are you sure the dropping of single_element_loc_list call in the
> loc->first && loc->last == loc->first case is a good idea?
> I believe that will force using location lists even when block form could be
> used for DW_AT_location.  If loc->first->descr is NULL, the function should
> just return NULL, otherwise it should create the special single element list
> so that add_AT_location_description does the right thing.
> 
> 	Jakub

Jakub,
   I have tested this but on the gdb mailing list it was suggested that
in resolve_addr() we change...

a->dw_attr_val.v.val_loc = NULL;

to

a->dw_attr_val.v.val_loc = DW_OP_nop;

http://sourceware.org/ml/gdb/2009-11/msg00176.html

I haven't had a chance to test this patch on darwin yet...

Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	(revision 154462)
+++ gcc/dwarf2out.c	(working copy)
@@ -20991,7 +20991,7 @@
 	break;
       case dw_val_class_loc:
 	if (!resolve_addr_in_expr (AT_loc (a)))
-	  a->dw_attr_val.v.val_loc = NULL;
+	  a->dw_attr_val.v.val_loc = DW_OP_nop;
 	break;
       case dw_val_class_addr:
 	if (a->dw_attr == DW_AT_const_value
@@ -20999,7 +20999,7 @@
 	  {
 	    a->dw_attr = DW_AT_location;
 	    a->dw_attr_val.val_class = dw_val_class_loc;
-	    a->dw_attr_val.v.val_loc = NULL;
+	    a->dw_attr_val.v.val_loc = DW_OP_nop;
 	  }
 	break;
       default:

to verify that dsymutil tolerates the DW_OP_nop. However, if it works,
might that be a more workable solution?
              Jack

Re: [VTA, PR41473] drop NULL locations from lists

[Cary Coutant]

> Are you sure the dropping of single_element_loc_list call in the
> loc->first && loc->last == loc->first case is a good idea?
> I believe that will force using location lists even when block form could be
> used for DW_AT_location.  If loc->first->descr is NULL, the function should
> just return NULL, otherwise it should create the special single element list
> so that add_AT_location_description does the right thing.

I don't think that would mean the same thing: a single-element loc
list describes a variable whose value is known only for that one
range, where a block form location expression describes a variable
that can always be found at the given location.

-cary

Re: [VTA, PR41473] drop NULL locations from lists

[Cary Coutant]

>   I have tested this but on the gdb mailing list it was suggested that
> in resolve_addr() we change...
>
> a->dw_attr_val.v.val_loc = NULL;
>
> to
>
> a->dw_attr_val.v.val_loc = DW_OP_nop;
>
> http://sourceware.org/ml/gdb/2009-11/msg00176.html
>
> I haven't had a chance to test this patch on darwin yet...

What I suggested doesn't quite translate to this -- DW_OP_nop isn't
something you can assign to dw_attr_val.v.val_loc. You need to create
an expression containing that single opcode, and set
dw_attr_val.v.val_loc to point to that expression.

But Alexandre's patch, I think, is a better approach -- don't create
the empty expressions in the first place.

-cary

Re: [VTA, PR41473] drop NULL locations from lists

[Jack Howarth]

On Mon, Nov 23, 2009 at 01:22:44PM -0800, Cary Coutant wrote:
> >   I have tested this but on the gdb mailing list it was suggested that
> > in resolve_addr() we change...
> >
> > a->dw_attr_val.v.val_loc = NULL;
> >
> > to
> >
> > a->dw_attr_val.v.val_loc = DW_OP_nop;
> >
> > http://sourceware.org/ml/gdb/2009-11/msg00176.html
> >
> > I haven't had a chance to test this patch on darwin yet...
> 
> What I suggested doesn't quite translate to this -- DW_OP_nop isn't
> something you can assign to dw_attr_val.v.val_loc. You need to create
> an expression containing that single opcode, and set
> dw_attr_val.v.val_loc to point to that expression.
> 
> But Alexandre's patch, I think, is a better approach -- don't create
> the empty expressions in the first place.
> 
> -cary

Cary,
   What would you gain from having the empty expressions in the first
place? Wouldn't those provide nothing more than a just list of symbols that
have been optimized out existence from the original source code?
               Jack

Re: [VTA, PR41473] drop NULL locations from lists

[Cary Coutant]

Jakub wrote:
>> Are you sure the dropping of single_element_loc_list call in the
>> loc->first && loc->last == loc->first case is a good idea?
>> I believe that will force using location lists even when block form could be
>> used for DW_AT_location.  If loc->first->descr is NULL, the function should
>> just return NULL, otherwise it should create the special single element list
>> so that add_AT_location_description does the right thing.

And I replied:
> I don't think that would mean the same thing: a single-element loc
> list describes a variable whose value is known only for that one
> range, where a block form location expression describes a variable
> that can always be found at the given location.

Hmmm, it looks like add_AT_location_description() is already going to
convert single-element loc lists into a simple block-form location
expression. In the case of a single-element location list where the
range is constrained, is that really the right thing to do?

-cary

Re: [VTA, PR41473] drop NULL locations from lists

[Cary Coutant]

>> But Alexandre's patch, I think, is a better approach -- don't create
>> the empty expressions in the first place.
>
> Cary,
>   What would you gain from having the empty expressions in the first
> place? Wouldn't those provide nothing more than a just list of symbols that
> have been optimized out existence from the original source code?

I think it was Jan who suggested it might be worthwhile distinguishing
between (a) no location == no information provided by the compiler,
vs. (b) empty location == optimized away. I don't have a strong
preference either way, but it's clear that the Apple tools don't like
(b) (yet).

-cary

Re: [VTA, PR41473] drop NULL locations from lists

[Jack Howarth]

On Mon, Nov 23, 2009 at 06:11:57PM -0200, Alexandre Oliva wrote:
> Entries in location lists whose RTL is NULL or not representable end up
> causing us to waste debug info space emitting empty DW_AT_location.
> 
> This patch optimizes the debug information, omitting the useless
> DW_AT_location.  This should work around the bug in Darwin's dsymutil.
> 
> While at that, I arranged for entries in location lists that are not
> representable in debug info to not waste an entry in the output location
> list.
> 
> The patch also fixes the handling of CONST_DOUBLEs and CONST_VECTORs
> within CONCAT and CONCATN, that was currently disabled because mode was
> specified as VOIDmode by the handlers of these RTL forms.  Even with
> -gdwarf-4, we'd still have failed to emit them.
> 
> Ok to install if this passes regstrap?
> 

> for  gcc/ChangeLog
> from  Alexandre Oliva  <aoliva@redhat.com>
> 
> 	PR debug/41473
> 	* dwarf2out.c (loc_descriptor): Infer mode from CONST_DOUBLEs
> 	and CONST_VECTORs.
> 	(dw_loc_list): Don't create entries without location.  Don't
> 	special-case the first node of the list.
> 	(loc_list_from_tree): Don't use DECL_RTL if loc_list is nonempty.
> 
> Index: gcc/dwarf2out.c
> ===================================================================
> --- gcc/dwarf2out.c.orig	2009-11-23 17:16:14.000000000 -0200
> +++ gcc/dwarf2out.c	2009-11-23 17:52:38.000000000 -0200
> @@ -13650,15 +13650,17 @@ loc_descriptor (rtx rtl, enum machine_mo
>        break;
>  
>      case CONST_DOUBLE:
> +      if (mode == VOIDmode)
> +	mode = GET_MODE (rtl);
> +
>        if (mode != VOIDmode && (dwarf_version >= 4 || !dwarf_strict))
>  	{
> +	  gcc_assert (mode == GET_MODE (rtl));
> +
>  	  /* Note that a CONST_DOUBLE rtx could represent either an integer
>  	     or a floating-point constant.  A CONST_DOUBLE is used whenever
>  	     the constant requires more than one word in order to be
>  	     adequately represented.  We output CONST_DOUBLEs as blocks.  */
> -	  if (GET_MODE (rtl) != VOIDmode)
> -	    mode = GET_MODE (rtl);
> -
>  	  loc_result = new_loc_descr (DW_OP_implicit_value,
>  				      GET_MODE_SIZE (mode), 0);
>  	  if (SCALAR_FLOAT_MODE_P (mode))
> @@ -13684,6 +13686,9 @@ loc_descriptor (rtx rtl, enum machine_mo
>        break;
>  
>      case CONST_VECTOR:
> +      if (mode == VOIDmode)
> +	mode = GET_MODE (rtl);
> +
>        if (mode != VOIDmode && (dwarf_version >= 4 || !dwarf_strict))
>  	{
>  	  unsigned int elt_size = GET_MODE_UNIT_SIZE (GET_MODE (rtl));
> @@ -13692,7 +13697,7 @@ loc_descriptor (rtx rtl, enum machine_mo
>  	  unsigned int i;
>  	  unsigned char *p;
>  
> -	  mode = GET_MODE (rtl);
> +	  gcc_assert (mode == GET_MODE (rtl));
>  	  switch (GET_MODE_CLASS (mode))
>  	    {
>  	    case MODE_VECTOR_INT:
> @@ -13928,20 +13933,21 @@ dw_loc_list_1 (tree loc, rtx varloc, int
>    return descr;
>  }
>  
> -/* Return dwarf representation of location list representing for
> -   LOC_LIST of DECL.  WANT_ADDRESS has the same meaning as in
> -   loc_list_from_tree function.  */
> +/* Return the dwarf representation of the location list LOC_LIST of
> +   DECL.  WANT_ADDRESS has the same meaning as in loc_list_from_tree
> +   function.  */
>  
>  static dw_loc_list_ref
> -dw_loc_list (var_loc_list * loc_list, tree decl, int want_address)
> +dw_loc_list (var_loc_list *loc_list, tree decl, int want_address)
>  {
>    const char *endname, *secname;
> -  dw_loc_list_ref list;
>    rtx varloc;
>    enum var_init_status initialized;
>    struct var_loc_node *node;
>    dw_loc_descr_ref descr;
>    char label_id[MAX_ARTIFICIAL_LABEL_BYTES];
> +  dw_loc_list_ref list = NULL;
> +  dw_loc_list_ref *listp = &list;
>  
>    /* Now that we know what section we are using for a base,
>       actually construct the list of locations.
> @@ -13954,26 +13960,9 @@ dw_loc_list (var_loc_list * loc_list, tr
>       This means we have to special case the last node, and generate
>       a range of [last location start, end of function label].  */
>  
> -  node = loc_list->first;
>    secname = secname_for_decl (decl);
>  
> -  if (NOTE_VAR_LOCATION_LOC (node->var_loc_note))
> -    initialized = NOTE_VAR_LOCATION_STATUS (node->var_loc_note);
> -  else
> -    initialized = VAR_INIT_STATUS_INITIALIZED;
> -  varloc = NOTE_VAR_LOCATION (node->var_loc_note);
> -  descr = dw_loc_list_1 (decl, varloc, want_address, initialized);
> -
> -  if (loc_list && loc_list->first != loc_list->last)
> -    list = new_loc_list (descr, node->label, node->next->label, secname, 1);
> -  else
> -    return single_element_loc_list (descr);
> -  node = node->next;
> -
> -  if (!node)
> -    return NULL;
> -
> -  for (; node->next; node = node->next)
> +  for (node = loc_list->first; node->next; node = node->next)
>      if (NOTE_VAR_LOCATION_LOC (node->var_loc_note) != NULL_RTX)
>        {
>  	/* The variable has a location between NODE->LABEL and
> @@ -13981,28 +13970,48 @@ dw_loc_list (var_loc_list * loc_list, tr
>  	initialized = NOTE_VAR_LOCATION_STATUS (node->var_loc_note);
>  	varloc = NOTE_VAR_LOCATION (node->var_loc_note);
>  	descr = dw_loc_list_1 (decl, varloc, want_address, initialized);
> -	add_loc_descr_to_loc_list (&list, descr,
> -				   node->label, node->next->label, secname);
> +	if (descr)
> +	  {
> +	    if (list)
> +	      add_loc_descr_to_loc_list (listp, descr,
> +					 node->label, node->next->label,
> +					 secname);
> +	    else
> +	      *listp = new_loc_list (descr, node->label, node->next->label,
> +				     secname, 1);
> +	    /* Speed up the next insertion.  */
> +	    listp = &(*listp)->dw_loc_next;
> +	  }
>        }
>  
>    /* If the variable has a location at the last label
>       it keeps its location until the end of function.  */
>    if (NOTE_VAR_LOCATION_LOC (node->var_loc_note) != NULL_RTX)
>      {
> -      if (!current_function_decl)
> -	endname = text_end_label;
> -      else
> -	{
> -	  ASM_GENERATE_INTERNAL_LABEL (label_id, FUNC_END_LABEL,
> -				       current_function_funcdef_no);
> -	  endname = ggc_strdup (label_id);
> -	}
> -
>        initialized = NOTE_VAR_LOCATION_STATUS (node->var_loc_note);
>        varloc = NOTE_VAR_LOCATION (node->var_loc_note);
>        descr = dw_loc_list_1 (decl, varloc, want_address, initialized);
> -      add_loc_descr_to_loc_list (&list, descr, node->label, endname, secname);
> +      if (descr)
> +	{
> +	  if (!current_function_decl)
> +	    endname = text_end_label;
> +	  else
> +	    {
> +	      ASM_GENERATE_INTERNAL_LABEL (label_id, FUNC_END_LABEL,
> +					   current_function_funcdef_no);
> +	      endname = ggc_strdup (label_id);
> +	    }
> +
> +	  if (*listp)
> +	    add_loc_descr_to_loc_list (listp, descr,
> +				       node->label, endname,
> +				       secname);
> +	  else
> +	    *listp = new_loc_list (descr, node->label, endname,
> +				   secname, 1);
> +	}
>      }
> +
>    return list;
>  }
>  
> @@ -14312,9 +14321,9 @@ loc_list_from_tree (tree loc, int want_a
>  	rtx rtl;
>  	var_loc_list *loc_list = lookup_decl_loc (loc);
>  
> -	if (loc_list && loc_list->first
> -	    && (list_ret = dw_loc_list (loc_list, loc, want_address)))
> +	if (loc_list && loc_list->first)
>  	  {
> +	    list_ret = dw_loc_list (loc_list, loc, want_address);
>  	    have_address = want_address != 0;
>  	    break;
>  	  }

> 
> -- 
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist      Red Hat Brazil Compiler Engineer


Alexandre,
    It appears that we still must be missing some cases were
AT_location is being set to zero but this definitely is an improvement.
Before this patch, dsymutil asserted on...

libgcj.11.dylib	
libgcj-tools.11.dylib
libgfortran.3.dylib
libgomp.1.dylib
libobjc-gnu.2.dylib
libssp.0.dylib
libstdc++.6.dylib

The proposed patch with r154473 eliminates the asserts in dsymutil
for libgcj.11.dylib, libgcj-tools.11.dylib, libobjc-gnu.2.dylib and
libgomp.1.dylib so we are heading in the right direction. We still 
get...

Assertion failed: (orig_str), function FixReferences, file /SourceCache/dwarf_utilities/dwarf_utilities-70/source/DWARFdSYM.cpp, line 3641.
Abort

for libgfortran.3.dylib, libssp.0.dylib and libstdc++.6.dylib though.
          Jack

Re: [VTA, PR41473] drop NULL locations from lists

[Alexandre Oliva]

On Nov 23, 2009, Jack Howarth <howarth@bromo.med.uc.edu> wrote:

>     It appears that we still must be missing some cases were
> AT_location is being set to zero

Can you name any?  I'm using a cross compiler, so I need a testcase that
triggers the problem, its assembly with -dA comments and an indication
of where the problem is in it.

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

Re: [VTA, PR41473] drop NULL locations from lists

[Alexandre Oliva]

[-- Attachment #1: Type: text/plain, Size: 1934 bytes --]

On Nov 23, 2009, Cary Coutant <ccoutant@google.com> wrote:

> Jakub wrote:
>>> Are you sure the dropping of single_element_loc_list call in the
>>> loc-> first && loc->last == loc->first case is a good idea?

It wasn't.  I didn't realize it was needed for the block-form
optimization to work.  However, I think the optimization is a bit
excessive.  We don't want to say a variable is at a certain location
throughout its lifetime just because it happens to be there on a narrow
range of PCs.

> And I replied:
>> I don't think that would mean the same thing: a single-element loc
>> list describes a variable whose value is known only for that one
>> range, where a block form location expression describes a variable
>> that can always be found at the given location.

> Hmmm, it looks like add_AT_location_description() is already going to
> convert single-element loc lists into a simple block-form location
> expression. In the case of a single-element location list where the
> range is constrained, is that really the right thing to do?

If there aren't any other entries, we could probably assume they're
equivalent, i.e., that the variable is set when we enter its scope, and
remains live till the end.  This may not always hold, though: if the
variable is set later in the scope, we lose.

Nevertheless, I've (partially) restored this functionality in the patch
below.  It retains var_loc_notes with NULL locs until the time of
converting lists to the internal dwarf representation, and then it only
applies the optimization if there's really no more than a single entry
in the incoming location list.

I've also fixed an assertion check I ran into on
x86_64-linux-gnu/libgcc: a VOIDmode CONST_DOUBLE failed the mode ==
GET_MODE newly-introduced assertion, so I realized it to accept incoming
non-VOID modes with VOIDmode consts.

This one has passed bootstrap compare on x86_64-linux-gnu already,
building target libs now.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vta-dwarf2out-omit-empty-loc-pr41473.patch --]
[-- Type: text/x-diff, Size: 12347 bytes --]

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/41473
	* dwarf2out.c (add_var_loc_to_decl): Don't drop initial empty
	locations.
	(new_loc_list): Drop gensym arg.  Move generation of ll_symbol...
	(gen_llsym): ... here.  New function.
	(add_loc_descr_to_loc_list): Removed.
	(loc_descriptor): Infer mode from CONST_DOUBLEs and CONST_VECTORs.
	(single_element_loc_list): Removed.
	(dw_loc_list): Don't create entries without a location.  Don't
	special-case the first node of the list, only single nodes.
	(single_element_loc_list_p): Simplify.
	(loc_list_from_tree): Don't use DECL_RTL if loc_list is nonempty.
	(add_location_or_const_value_attribute): Test var loc for NULL.
	(convert_cfa_to_fb_loc_list): Adjust calls to new new_loc_list,
	call gen_llsym if needed.

Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c.orig	2009-11-23 18:12:26.000000000 -0200
+++ gcc/dwarf2out.c	2009-11-23 23:41:40.000000000 -0200
@@ -6172,10 +6172,7 @@ static void gen_generic_params_dies (tre
 static void splice_child_die (dw_die_ref, dw_die_ref);
 static int file_info_cmp (const void *, const void *);
 static dw_loc_list_ref new_loc_list (dw_loc_descr_ref, const char *,
-				     const char *, const char *, unsigned);
-static void add_loc_descr_to_loc_list (dw_loc_list_ref *, dw_loc_descr_ref,
-				       const char *, const char *,
-				       const char *);
+				     const char *, const char *);
 static void output_loc_list (dw_loc_list_ref);
 static char *gen_internal_sym (const char *);
 
@@ -7788,8 +7785,7 @@ add_var_loc_to_decl (tree decl, struct v
 	  temp->last = loc;
 	}
     }
-  /* Do not add empty location to the beginning of the list.  */
-  else if (NOTE_VAR_LOCATION_LOC (loc->var_loc_note) != NULL_RTX)
+  else
     {
       temp->first = loc;
       temp->last = loc;
@@ -10297,12 +10293,11 @@ output_die_symbol (dw_die_ref die)
 }
 
 /* Return a new location list, given the begin and end range, and the
-   expression. gensym tells us whether to generate a new internal symbol for
-   this location list node, which is done for the head of the list only.  */
+   expression.  */
 
 static inline dw_loc_list_ref
 new_loc_list (dw_loc_descr_ref expr, const char *begin, const char *end,
-	      const char *section, unsigned int gensym)
+	      const char *section)
 {
   dw_loc_list_ref retlist = GGC_CNEW (dw_loc_list_node);
 
@@ -10310,27 +10305,18 @@ new_loc_list (dw_loc_descr_ref expr, con
   retlist->end = end;
   retlist->expr = expr;
   retlist->section = section;
-  if (gensym)
-    retlist->ll_symbol = gen_internal_sym ("LLST");
 
   return retlist;
 }
 
-/* Add a location description expression to a location list.  */
+/* Generate a new internal symbol for this location list node, if it
+   hasn't got one yet.  */
 
 static inline void
-add_loc_descr_to_loc_list (dw_loc_list_ref *list_head, dw_loc_descr_ref descr,
-			   const char *begin, const char *end,
-			   const char *section)
+gen_llsym (dw_loc_list_ref list)
 {
-  dw_loc_list_ref *d;
-
-  /* Find the end of the chain.  */
-  for (d = list_head; (*d) != NULL; d = &(*d)->dw_loc_next)
-    ;
-
-  /* Add a new location list node to the list.  */
-  *d = new_loc_list (descr, begin, end, section, 0);
+  gcc_assert (!list->ll_symbol);
+  list->ll_symbol = gen_internal_sym ("LLST");
 }
 
 /* Output the location list given to us.  */
@@ -13650,15 +13636,17 @@ loc_descriptor (rtx rtl, enum machine_mo
       break;
 
     case CONST_DOUBLE:
+      if (mode == VOIDmode)
+	mode = GET_MODE (rtl);
+
       if (mode != VOIDmode && (dwarf_version >= 4 || !dwarf_strict))
 	{
+	  gcc_assert (mode == GET_MODE (rtl) || VOIDmode == GET_MODE (rtl));
+
 	  /* Note that a CONST_DOUBLE rtx could represent either an integer
 	     or a floating-point constant.  A CONST_DOUBLE is used whenever
 	     the constant requires more than one word in order to be
 	     adequately represented.  We output CONST_DOUBLEs as blocks.  */
-	  if (GET_MODE (rtl) != VOIDmode)
-	    mode = GET_MODE (rtl);
-
 	  loc_result = new_loc_descr (DW_OP_implicit_value,
 				      GET_MODE_SIZE (mode), 0);
 	  if (SCALAR_FLOAT_MODE_P (mode))
@@ -13684,6 +13672,9 @@ loc_descriptor (rtx rtl, enum machine_mo
       break;
 
     case CONST_VECTOR:
+      if (mode == VOIDmode)
+	mode = GET_MODE (rtl);
+
       if (mode != VOIDmode && (dwarf_version >= 4 || !dwarf_strict))
 	{
 	  unsigned int elt_size = GET_MODE_UNIT_SIZE (GET_MODE (rtl));
@@ -13692,7 +13683,7 @@ loc_descriptor (rtx rtl, enum machine_mo
 	  unsigned int i;
 	  unsigned char *p;
 
-	  mode = GET_MODE (rtl);
+	  gcc_assert (mode == GET_MODE (rtl) || VOIDmode == GET_MODE (rtl));
 	  switch (GET_MODE_CLASS (mode))
 	    {
 	    case MODE_VECTOR_INT:
@@ -13839,14 +13830,6 @@ decl_by_reference_p (tree decl)
 	  && DECL_BY_REFERENCE (decl));
 }
 
-/* Return single element location list containing loc descr REF.  */
-
-static dw_loc_list_ref
-single_element_loc_list (dw_loc_descr_ref ref)
-{
-  return new_loc_list (ref, NULL, NULL, NULL, 0);
-}
-
 /* Helper function for dw_loc_list.  Compute proper Dwarf location descriptor
    for VARLOC.  */
 
@@ -13928,20 +13911,21 @@ dw_loc_list_1 (tree loc, rtx varloc, int
   return descr;
 }
 
-/* Return dwarf representation of location list representing for
-   LOC_LIST of DECL.  WANT_ADDRESS has the same meaning as in
-   loc_list_from_tree function.  */
+/* Return the dwarf representation of the location list LOC_LIST of
+   DECL.  WANT_ADDRESS has the same meaning as in loc_list_from_tree
+   function.  */
 
 static dw_loc_list_ref
-dw_loc_list (var_loc_list * loc_list, tree decl, int want_address)
+dw_loc_list (var_loc_list *loc_list, tree decl, int want_address)
 {
   const char *endname, *secname;
-  dw_loc_list_ref list;
   rtx varloc;
   enum var_init_status initialized;
   struct var_loc_node *node;
   dw_loc_descr_ref descr;
   char label_id[MAX_ARTIFICIAL_LABEL_BYTES];
+  dw_loc_list_ref list = NULL;
+  dw_loc_list_ref *listp = &list;
 
   /* Now that we know what section we are using for a base,
      actually construct the list of locations.
@@ -13954,26 +13938,9 @@ dw_loc_list (var_loc_list * loc_list, tr
      This means we have to special case the last node, and generate
      a range of [last location start, end of function label].  */
 
-  node = loc_list->first;
   secname = secname_for_decl (decl);
 
-  if (NOTE_VAR_LOCATION_LOC (node->var_loc_note))
-    initialized = NOTE_VAR_LOCATION_STATUS (node->var_loc_note);
-  else
-    initialized = VAR_INIT_STATUS_INITIALIZED;
-  varloc = NOTE_VAR_LOCATION (node->var_loc_note);
-  descr = dw_loc_list_1 (decl, varloc, want_address, initialized);
-
-  if (loc_list && loc_list->first != loc_list->last)
-    list = new_loc_list (descr, node->label, node->next->label, secname, 1);
-  else
-    return single_element_loc_list (descr);
-  node = node->next;
-
-  if (!node)
-    return NULL;
-
-  for (; node->next; node = node->next)
+  for (node = loc_list->first; node->next; node = node->next)
     if (NOTE_VAR_LOCATION_LOC (node->var_loc_note) != NULL_RTX)
       {
 	/* The variable has a location between NODE->LABEL and
@@ -13981,28 +13948,46 @@ dw_loc_list (var_loc_list * loc_list, tr
 	initialized = NOTE_VAR_LOCATION_STATUS (node->var_loc_note);
 	varloc = NOTE_VAR_LOCATION (node->var_loc_note);
 	descr = dw_loc_list_1 (decl, varloc, want_address, initialized);
-	add_loc_descr_to_loc_list (&list, descr,
-				   node->label, node->next->label, secname);
+	if (descr)
+	  {
+	    *listp = new_loc_list (descr, node->label, node->next->label,
+				   secname);
+	    listp = &(*listp)->dw_loc_next;
+	  }
       }
 
   /* If the variable has a location at the last label
      it keeps its location until the end of function.  */
   if (NOTE_VAR_LOCATION_LOC (node->var_loc_note) != NULL_RTX)
     {
-      if (!current_function_decl)
-	endname = text_end_label;
-      else
-	{
-	  ASM_GENERATE_INTERNAL_LABEL (label_id, FUNC_END_LABEL,
-				       current_function_funcdef_no);
-	  endname = ggc_strdup (label_id);
-	}
-
       initialized = NOTE_VAR_LOCATION_STATUS (node->var_loc_note);
       varloc = NOTE_VAR_LOCATION (node->var_loc_note);
       descr = dw_loc_list_1 (decl, varloc, want_address, initialized);
-      add_loc_descr_to_loc_list (&list, descr, node->label, endname, secname);
+      if (descr)
+	{
+	  if (!current_function_decl)
+	    endname = text_end_label;
+	  else
+	    {
+	      ASM_GENERATE_INTERNAL_LABEL (label_id, FUNC_END_LABEL,
+					   current_function_funcdef_no);
+	      endname = ggc_strdup (label_id);
+	    }
+
+	  *listp = new_loc_list (descr, node->label, endname, secname);
+	  listp = &(*listp)->dw_loc_next;
+	}
     }
+
+  /* Try to avoid the overhead of a location list emitting a location
+     expression instead, but only if we didn't have more than one
+     location entry in the first place.  If some entries were not
+     representable, we don't want to pretend a single entry that was
+     applies to the entire scope in which the variable is
+     available.  */
+  if (list && loc_list->first->next)
+    gen_llsym (list);
+
   return list;
 }
 
@@ -14012,7 +13997,8 @@ dw_loc_list (var_loc_list * loc_list, tr
 static bool
 single_element_loc_list_p (dw_loc_list_ref list)
 {
-  return (!list->dw_loc_next && !list->begin && !list->end);
+  gcc_assert (!list->dw_loc_next || list->ll_symbol);
+  return !list->ll_symbol;
 }
 
 /* To each location in list LIST add loc descr REF.  */
@@ -14312,9 +14298,9 @@ loc_list_from_tree (tree loc, int want_a
 	rtx rtl;
 	var_loc_list *loc_list = lookup_decl_loc (loc);
 
-	if (loc_list && loc_list->first
-	    && (list_ret = dw_loc_list (loc_list, loc, want_address)))
+	if (loc_list && loc_list->first)
 	  {
+	    list_ret = dw_loc_list (loc_list, loc, want_address);
 	    have_address = want_address != 0;
 	    break;
 	  }
@@ -14725,7 +14711,7 @@ loc_list_from_tree (tree loc, int want_a
 	add_loc_descr_to_each (list_ret, new_loc_descr (op, size, 0));
     }
   if (ret)
-    list_ret = single_element_loc_list (ret);
+    list_ret = new_loc_list (ret, NULL, NULL, NULL);
 
   return list_ret;
 }
@@ -15719,17 +15705,19 @@ add_location_or_const_value_attribute (d
      a constant value.  That way we are better to use add_const_value_attribute
      rather than expanding constant value equivalent.  */
   loc_list = lookup_decl_loc (decl);
-  if (loc_list && loc_list->first && loc_list->first == loc_list->last)
+  if (loc_list && loc_list->first && loc_list->first == loc_list->last
+      && loc_list->first->var_loc_note
+      && NOTE_VAR_LOCATION (loc_list->first->var_loc_note)
+      && NOTE_VAR_LOCATION_LOC (loc_list->first->var_loc_note))
     {
       enum var_init_status status;
       struct var_loc_node *node;
 
       node = loc_list->first;
       status = NOTE_VAR_LOCATION_STATUS (node->var_loc_note);
-      rtl = NOTE_VAR_LOCATION (node->var_loc_note);
-      if (GET_CODE (rtl) == VAR_LOCATION
-	  && GET_CODE (XEXP (rtl, 1)) != PARALLEL)
-	rtl = XEXP (XEXP (rtl, 1), 0);
+      rtl = NOTE_VAR_LOCATION_LOC (node->var_loc_note);
+      if (GET_CODE (rtl) != PARALLEL)
+	rtl = XEXP (rtl, 0);
       if ((CONSTANT_P (rtl) || GET_CODE (rtl) == CONST_STRING)
 	  && add_const_value_attribute (die, rtl))
 	 return true;
@@ -16016,8 +16004,7 @@ convert_cfa_to_fb_loc_list (HOST_WIDE_IN
 	if (!cfa_equal_p (&last_cfa, &next_cfa))
 	  {
 	    *list_tail = new_loc_list (build_cfa_loc (&last_cfa, offset),
-				       start_label, last_label, section,
-				       list == NULL);
+				       start_label, last_label, section);
 
 	    list_tail = &(*list_tail)->dw_loc_next;
 	    last_cfa = next_cfa;
@@ -16038,14 +16025,16 @@ convert_cfa_to_fb_loc_list (HOST_WIDE_IN
   if (!cfa_equal_p (&last_cfa, &next_cfa))
     {
       *list_tail = new_loc_list (build_cfa_loc (&last_cfa, offset),
-				 start_label, last_label, section,
-				 list == NULL);
+				 start_label, last_label, section);
       list_tail = &(*list_tail)->dw_loc_next;
       start_label = last_label;
     }
+
   *list_tail = new_loc_list (build_cfa_loc (&next_cfa, offset),
-			     start_label, fde->dw_fde_end, section,
-			     list == NULL);
+			     start_label, fde->dw_fde_end, section);
+
+  if (list && list->dw_loc_next)
+    gen_llsym (list);
 
   return list;
 }

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

Re: [VTA, PR41473] drop NULL locations from lists

[Jack Howarth]

On Tue, Nov 24, 2009 at 12:30:44AM -0200, Alexandre Oliva wrote:
> On Nov 23, 2009, Jack Howarth <howarth@bromo.med.uc.edu> wrote:
> 
> >     It appears that we still must be missing some cases were
> > AT_location is being set to zero
> 
> Can you name any?  I'm using a cross compiler, so I need a testcase that
> triggers the problem, its assembly with -dA comments and an indication
> of where the problem is in it.
> 
> -- 
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist      Red Hat Brazil Compiler Engineer

Alexandre,
    I am uncertain if the stock dwarfdump will provide the necessary
information to find any residual zero locations. I have uploaded
an archive of the remaining shared libs that cause dsymutil to
assert under radar://7415099 and have asked the darwin linker 
maintainers to look at those. I'll let you know as soon as I hear
back if this is still the same issue with zero locations or something
different. Remember that they have been working from the limited
testcase in PR41473 and haven't tested full shared libs from FSF
gcc yet with their patched dsymutil. I specific it may still be
the zero locations problem though because dsymutil is stil asserting
on the same line...

Assertion failed: (orig_str), function FixReferences, file /SourceCache/dwarf_utilities/dwarf_utilities-70/source/DWARFdSYM.cpp, line 3641.
Abort

        Jack

Re: [VTA, PR41473] drop NULL locations from lists

[Jack Howarth]

On Tue, Nov 24, 2009 at 12:30:44AM -0200, Alexandre Oliva wrote:
> On Nov 23, 2009, Jack Howarth <howarth@bromo.med.uc.edu> wrote:
> 
> >     It appears that we still must be missing some cases were
> > AT_location is being set to zero
> 
> Can you name any?  I'm using a cross compiler, so I need a testcase that
> triggers the problem, its assembly with -dA comments and an indication
> of where the problem is in it.
> 
> -- 
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist      Red Hat Brazil Compiler Engineer

Alexandre,
   I have some comments from the dsymutil maintainer regarding the ssp.s
testcase that I added to PR41473...

--------------------------------------------------------------------

On Nov 24, 2009, at 7:48 AM, Jack Howarth wrote:

> Greg,
>  I have narrowed down the offending source code in libssp.0.dylib
> which still asserts dsymutil in FSF gcc trunk when built with...
>
> http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01329.html
>
> (that is supposed to eliminate the NULL locations in the
> dwarf output). The offending code from ssp.c as assembly
> to attached at...
>
> http://gcc.gnu.org/bugzilla/attachment.cgi?id=19109
>
> When compiled as..
>
> as -arch x86_64 ssp.s -o ssp.o
>
> and linked as
>
> gcc -arch x86_64 -dynamiclib ssp.o -o test.dylib
>
> , the resulting test.dylib asserts dsymutil. I will
> update the ssp.s later tonight to be built with -dA
> so that it is easier to read. In the meanwhile,
> can you tell me if your patched dsymutil can process
> this file?

It does!


> Or is this some new issue with zero
> AT_locations beyond the previous block forms?

There are still zero length blocks in the resulting .o file which is what is crashing dsymutil:

% dwarfdump --verbose --show-form ssp.o
....
0x00000269:         TAG_variable [12]
0x0000026a:          AT_name[FORM_string] ( "__progname" )
0x00000275:          AT_decl_file[FORM_data1] ( 0x01 ( "../../../gcc-4.5-20091123/libssp/ssp.c" ) )
0x00000276:          AT_decl_line[FORM_data1] ( 0x66 ( 102 ) )
0x00000277:          AT_type[FORM_ref4] ( cu + 0x00000355 => {0x00000355} ( const char[] ) )
0x0000027b:          AT_location[FORM_block1] ( <0x0> EMPTY )

> Thanks
> in advance for any information. It would be helpful
> if we can provide Alexandre Oliva with any additional
> details on why this is asserting dsymutil since he 
> is quite willing to make the effort to work around the
> limitations in the current darwin dsymutils for
> gcc 4.5.

Unfortunately the current dwarfdump won't show these zero length blocks, but you can detect them by looking for a "AT_location" attribute with 
+"FORM_blockXXX" attributes that have location lists displayed beneath them:

0x00000269:         TAG_variable [12]
0x0000026a:          AT_name[FORM_string] ( "__progname" )
0x00000275:          AT_decl_file[FORM_data1] ( 0x01 ( "../../../gcc-4.5-20091123/libssp/ssp.c" ) )
0x00000276:          AT_decl_line[FORM_data1] ( 0x66 ( 102 ) )
0x00000277:          AT_type[FORM_ref4] ( cu + 0x00000355 => {0x00000355} ( const char[] ) )
0x0000027b:          AT_location[FORM_block1] (
                        0x0000000000000000 - 0x0000000000000001: breg7 +8
                        0x0000000000000001 - 0x0000000000000006: breg7 +16
                        0x0000000000000006 - 0x0000000000000111: breg6 +16 )

Usually when you have a location with a FORM_block1, the entire location will be displayed on the same line. There is a good example for "fd"
+in ssp.o:

0x0000027c:         TAG_variable [12]
0x0000027d:          AT_name[FORM_string] ( "fd" )
0x00000280:          AT_decl_file[FORM_data1] ( 0x01 ( "../../../gcc-4.5-20091123/libssp/ssp.c" ) )
0x00000281:          AT_decl_line[FORM_data1] ( 0x68 ( 104 ) )
0x00000282:          AT_type[FORM_ref4] ( cu + 0x00000108 => {0x00000108} ( int ) )
0x00000286:          AT_location[FORM_block1] ( <0x1> 5d  ( reg13 ) )


Note how the block shows the length of the block as in between less than/greater than chars above.

The following command will allow you to use the existing dwarfdump to find any remaining zero length block issues:

% /usr/bin/dwarfdump --verbose --show-form ssp.o | grep -C5 'AT_location.*FORM_block[^<]+$'
0x00000269:         TAG_variable [12]
0x0000026a:          AT_name[FORM_string] ( "__progname" )
0x00000275:          AT_decl_file[FORM_data1] ( 0x01 ( "../../../gcc-4.5-20091123/libssp/ssp.c" ) )
0x00000276:          AT_decl_line[FORM_data1] ( 0x66 ( 102 ) )
0x00000277:          AT_type[FORM_ref4] ( cu + 0x00000355 => {0x00000355} ( const char[] ) )
0x0000027b:          AT_location[FORM_block1] (
                        0x0000000000000000 - 0x0000000000000001: breg7 +8
                        0x0000000000000001 - 0x0000000000000006: breg7 +16
                        0x0000000000000006 - 0x0000000000000111: breg6 +16 )





>              Jack
> ps I plan on working my way through all of the
> remaining shared libraries in FSF gcc that still
> assert dsymutil with the proposed patch and identify
> the specific object files containing the offending
> code. I should add that I was very surprised last
> night to see libgcj not assert dsymutil during the
> build with the first version of the proposed patch
> but to assert dsymutil with the second revision.
> The weird bit was that when I manually ran dsymutil
> on the libgcj outside of the build it didn't assert.
> Odd.

It all comes down to luck of the draw with these binaries, as dsymutil might not 
assert depending on what gets erroneously written where...
dsymutil is incorrectly putting data into the middle of the DWARF debug 
info and might cause an assert if we change the abbreviation code for a DIE (debug info entry).

-------------------------------------------------------------------------------------------------------------

Oddly, I see the section from '/usr/bin/dwarfdump --verbose --show-form ssp.o' that is supposed to
have the zero AT_location...

0x00000269:         TAG_variable [12]  
0x0000026a:          AT_name[FORM_string] ( "__progname" )
0x00000275:          AT_decl_file[FORM_data1] ( 0x01 ( "../../../gcc-4.5-20091123/libssp/ssp.c" ) )
0x00000276:          AT_decl_line[FORM_data1] ( 0x66 ( 102 ) )
0x00000277:          AT_type[FORM_ref4] ( cu + 0x00000355 => {0x00000355} ( const char[] ) )
0x0000027b:          AT_location[FORM_block1] ( 
                        0x0000000000000000 - 0x0000000000000001: breg7 +8
                        0x0000000000000001 - 0x0000000000000006: breg7 +16
                        0x0000000000000006 - 0x0000000000000111: breg6 +16 )

that he mentions but...

 /usr/bin/dwarfdump --verbose --show-form ssp.o | grep -C5 'AT_location.*FORM_block[^<]+$'

produces no output. I am looking at these binaries under Leopard instead of SL so
it could be some oddity from that combination. In any case, I will add the dwarfdump
to the PR.
               Jack

Re: [VTA, PR41473] drop NULL locations from lists

[Jack Howarth]

On Tue, Nov 24, 2009 at 11:48:38AM -0800, Cary Coutant wrote:
> > Unfortunately the current dwarfdump won't show these zero length blocks, but you can detect them by looking for a "AT_location" attribute with
> > +"FORM_blockXXX" attributes that have location lists displayed beneath them:
> >
> > 0x00000269:         TAG_variable [12]
> > 0x0000026a:          AT_name[FORM_string] ( "__progname" )
> > 0x00000275:          AT_decl_file[FORM_data1] ( 0x01 ( "../../../gcc-4.5-20091123/libssp/ssp.c" ) )
> > 0x00000276:          AT_decl_line[FORM_data1] ( 0x66 ( 102 ) )
> > 0x00000277:          AT_type[FORM_ref4] ( cu + 0x00000355 => {0x00000355} ( const char[] ) )
> > 0x0000027b:          AT_location[FORM_block1] (
> >                        0x0000000000000000 - 0x0000000000000001: breg7 +8
> >                        0x0000000000000001 - 0x0000000000000006: breg7 +16
> >                        0x0000000000000006 - 0x0000000000000111: breg6 +16 )
> 
> I'm puzzled by this. When an AT_location attribute points to a
> location list, it should be using DW_FORM_data4 or DW_FORM_data8.
> DW_FORM_block1 indicates a single location expression contained in the
> block. This looks like a bug in dwarfdump, as I don't see how a
> DW_FORM_block1 could lead to a location list. What does readelf -wi
> show?
> 
> -cary

Cary,
   How could readelf show anything as these are MACH-O binaries?

file ssp.o
ssp.o: Mach-O 64-bit object x86_64

file test.dylib
test.dylib: Mach-O 64-bit dynamically linked shared library x86_64

Also, I should note that the fixed unreleased dsymutil was changed
to handle zero AT_locations for for any block form (DW_FORM_block1,
DW_FORM_block2, DW_FORM_block4, DW_FORM_block).
                    Jack

Re: [VTA, PR41473] drop NULL locations from lists

[Cary Coutant]

> Unfortunately the current dwarfdump won't show these zero length blocks, but you can detect them by looking for a "AT_location" attribute with
> +"FORM_blockXXX" attributes that have location lists displayed beneath them:
>
> 0x00000269:         TAG_variable [12]
> 0x0000026a:          AT_name[FORM_string] ( "__progname" )
> 0x00000275:          AT_decl_file[FORM_data1] ( 0x01 ( "../../../gcc-4.5-20091123/libssp/ssp.c" ) )
> 0x00000276:          AT_decl_line[FORM_data1] ( 0x66 ( 102 ) )
> 0x00000277:          AT_type[FORM_ref4] ( cu + 0x00000355 => {0x00000355} ( const char[] ) )
> 0x0000027b:          AT_location[FORM_block1] (
>                        0x0000000000000000 - 0x0000000000000001: breg7 +8
>                        0x0000000000000001 - 0x0000000000000006: breg7 +16
>                        0x0000000000000006 - 0x0000000000000111: breg6 +16 )

I'm puzzled by this. When an AT_location attribute points to a
location list, it should be using DW_FORM_data4 or DW_FORM_data8.
DW_FORM_block1 indicates a single location expression contained in the
block. This looks like a bug in dwarfdump, as I don't see how a
DW_FORM_block1 could lead to a location list. What does readelf -wi
show?

-cary

Re: [VTA, PR41473] drop NULL locations from lists

[Jack Howarth]

On Tue, Nov 24, 2009 at 11:48:38AM -0800, Cary Coutant wrote:
> > Unfortunately the current dwarfdump won't show these zero length blocks, but you can detect them by looking for a "AT_location" attribute with
> > +"FORM_blockXXX" attributes that have location lists displayed beneath them:
> >
> > 0x00000269:         TAG_variable [12]
> > 0x0000026a:          AT_name[FORM_string] ( "__progname" )
> > 0x00000275:          AT_decl_file[FORM_data1] ( 0x01 ( "../../../gcc-4.5-20091123/libssp/ssp.c" ) )
> > 0x00000276:          AT_decl_line[FORM_data1] ( 0x66 ( 102 ) )
> > 0x00000277:          AT_type[FORM_ref4] ( cu + 0x00000355 => {0x00000355} ( const char[] ) )
> > 0x0000027b:          AT_location[FORM_block1] (
> >                        0x0000000000000000 - 0x0000000000000001: breg7 +8
> >                        0x0000000000000001 - 0x0000000000000006: breg7 +16
> >                        0x0000000000000006 - 0x0000000000000111: breg6 +16 )
> 
> I'm puzzled by this. When an AT_location attribute points to a
> location list, it should be using DW_FORM_data4 or DW_FORM_data8.
> DW_FORM_block1 indicates a single location expression contained in the
> block. This looks like a bug in dwarfdump, as I don't see how a
> DW_FORM_block1 could lead to a location list. What does readelf -wi
> show?
> 
> -cary

Cary,
    The response from the dsymutil maintainer was...

---------------------------------------------------------------------------

As I said, "with the current dwarfdump". The new dwarfdump disassebles the DWARF correctly:

0x00000269:         TAG_variable [12]
0x0000026a:          AT_name[FORM_string] ( "__progname" )
0x00000275:          AT_decl_file[FORM_data1] ( 0x01 ( "../../../gcc-4.5-20091123/libssp/ssp.c" ) )
0x00000276:          AT_decl_line[FORM_data1] ( 0x66 ( 102 ) )
0x00000277:          AT_type[FORM_ref4] ( cu + 0x00000355 => {0x00000355} ( const char[] ) )
0x0000027b:          AT_location[FORM_block1] ( <0x0> EMPTY )


I gave you a hack to tell when you are seeing this issue -- the dwarfdump command with the grep -- and the grep was looking for the discrepancy    
+Cary Coutant was pointing out! :-)

---------------------------------------------------------------------------

So with the current dwarfdump (which doesn't understand zero AT_locations)
we can really only be used to identify where the offending zero AT_locations
are. Does the information above from the new dwarfdump help explain why
Alexandre's proposed patch (http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01329.html)
left thati particular zero AT_location in the emitted dwarf code?
               Jack

Re: [VTA, PR41473] drop NULL locations from lists

[Tristan Gingold]

On Nov 24, 2009, at 9:03 PM, Jack Howarth wrote:

> On Tue, Nov 24, 2009 at 11:48:38AM -0800, Cary Coutant wrote:
>>> Unfortunately the current dwarfdump won't show these zero length blocks, but you can detect them by looking for a "AT_location" attribute with
>>> +"FORM_blockXXX" attributes that have location lists displayed beneath them:
>>> 
>>> 0x00000269:         TAG_variable [12]
>>> 0x0000026a:          AT_name[FORM_string] ( "__progname" )
>>> 0x00000275:          AT_decl_file[FORM_data1] ( 0x01 ( "../../../gcc-4.5-20091123/libssp/ssp.c" ) )
>>> 0x00000276:          AT_decl_line[FORM_data1] ( 0x66 ( 102 ) )
>>> 0x00000277:          AT_type[FORM_ref4] ( cu + 0x00000355 => {0x00000355} ( const char[] ) )
>>> 0x0000027b:          AT_location[FORM_block1] (
>>>                       0x0000000000000000 - 0x0000000000000001: breg7 +8
>>>                       0x0000000000000001 - 0x0000000000000006: breg7 +16
>>>                       0x0000000000000006 - 0x0000000000000111: breg6 +16 )
>> 
>> I'm puzzled by this. When an AT_location attribute points to a
>> location list, it should be using DW_FORM_data4 or DW_FORM_data8.
>> DW_FORM_block1 indicates a single location expression contained in the
>> block. This looks like a bug in dwarfdump, as I don't see how a
>> DW_FORM_block1 could lead to a location list. What does readelf -wi
>> show?
>> 
>> -cary
> 
> Cary,
>  How could readelf show anything as these are MACH-O binaries?

You can try objdump -Wi instead!

Re: [VTA, PR41473] drop NULL locations from lists

[Jack Howarth]

On Wed, Nov 25, 2009 at 09:33:11AM +0100, Tristan Gingold wrote:
> 
> On Nov 24, 2009, at 9:03 PM, Jack Howarth wrote:
> 
> > On Tue, Nov 24, 2009 at 11:48:38AM -0800, Cary Coutant wrote:
> >>> Unfortunately the current dwarfdump won't show these zero length blocks, but you can detect them by looking for a "AT_location" attribute with
> >>> +"FORM_blockXXX" attributes that have location lists displayed beneath them:
> >>> 
> >>> 0x00000269:         TAG_variable [12]
> >>> 0x0000026a:          AT_name[FORM_string] ( "__progname" )
> >>> 0x00000275:          AT_decl_file[FORM_data1] ( 0x01 ( "../../../gcc-4.5-20091123/libssp/ssp.c" ) )
> >>> 0x00000276:          AT_decl_line[FORM_data1] ( 0x66 ( 102 ) )
> >>> 0x00000277:          AT_type[FORM_ref4] ( cu + 0x00000355 => {0x00000355} ( const char[] ) )
> >>> 0x0000027b:          AT_location[FORM_block1] (
> >>>                       0x0000000000000000 - 0x0000000000000001: breg7 +8
> >>>                       0x0000000000000001 - 0x0000000000000006: breg7 +16
> >>>                       0x0000000000000006 - 0x0000000000000111: breg6 +16 )
> >> 
> >> I'm puzzled by this. When an AT_location attribute points to a
> >> location list, it should be using DW_FORM_data4 or DW_FORM_data8.
> >> DW_FORM_block1 indicates a single location expression contained in the
> >> block. This looks like a bug in dwarfdump, as I don't see how a
> >> DW_FORM_block1 could lead to a location list. What does readelf -wi
> >> show?
> >> 
> >> -cary
> > 
> > Cary,
> >  How could readelf show anything as these are MACH-O binaries?
> 
> You can try objdump -Wi instead!


Using binutils 2.17-3 from fink, I am not having any luck dumping the dwarf
on darwin. For i386 or x86_64 object files...

objdump --target=mach-o-le --architecture=i386 -Wi foobar.o

or

objdump --target=mach-o-le --architecture=x86_64 -Wi foobar.o

only produces...

BFD header file version 2.17
mach-o-be
 (header big endian, data big endian)
  i386
  powerpc:common
  rs6000:6000
mach-o-le
 (header little endian, data little endian)
  i386
  powerpc:common
  rs6000:6000
mach-o-fat
 (header big endian, data big endian)
  i386
  powerpc:common
  rs6000:6000
pef
 (header big endian, data big endian)
  i386
  powerpc:common
  rs6000:6000
pef-xlib
 (header big endian, data big endian)
sym
 (header big endian, data big endian)
  i386
  powerpc:common
  rs6000:6000
srec
 (header endianness unknown, data endianness unknown)
  i386
  powerpc:common
  rs6000:6000
symbolsrec
 (header endianness unknown, data endianness unknown)
  i386
  powerpc:common
  rs6000:6000
tekhex
 (header endianness unknown, data endianness unknown)
  i386
  powerpc:common
  rs6000:6000
binary
 (header endianness unknown, data endianness unknown)
  i386
  powerpc:common
  rs6000:6000
ihex
 (header endianness unknown, data endianness unknown)
  i386
  powerpc:common
  rs6000:6000

               mach-o-be mach-o-le mach-o-fat pef pef-xlib sym srec symbolsrec 
          i386 mach-o-be mach-o-le mach-o-fat pef -------- sym srec symbolsrec 
powerpc:common mach-o-be mach-o-le mach-o-fat pef -------- sym srec symbolsrec 
   rs6000:6000 mach-o-be mach-o-le mach-o-fat pef -------- sym srec symbolsrec 

               tekhex binary ihex 
          i386 tekhex binary ihex 
powerpc:common tekhex binary ihex 
   rs6000:6000 tekhex binary ihex 

Why do we need this anyway? Isn't their sufficient information in the
generated assembly in PR41473 now with -dA? I certainly see the occurances
of the offending...

.byte	0x0	# DW_AT_location

in those assembly files.
                Jack

Re: [VTA, PR41473] drop NULL locations from lists

[Jack Howarth]

On Tue, Nov 24, 2009 at 12:30:44AM -0200, Alexandre Oliva wrote:
> On Nov 23, 2009, Jack Howarth <howarth@bromo.med.uc.edu> wrote:
> 
> >     It appears that we still must be missing some cases were
> > AT_location is being set to zero
> 
> Can you name any?  I'm using a cross compiler, so I need a testcase that
> triggers the problem, its assembly with -dA comments and an indication
> of where the problem is in it.
> 
> -- 
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist      Red Hat Brazil Compiler Engineer

Alexandre,
   I've uploaded assembly files (generated with -dA) as well as the
preprocessed source for all the object files in the FSF gcc trunk
build with your proosed patch...

http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01329.html

that still emit zero AT_LOCATIONS and cause dsymutil to assert.
These all seem to be of the form...

.byte	0x0	# DW_AT_location

So far, looking through the same files in gcc trunk built with
your patch on x86_64 Fedora 10 linux, I have been unable to
find the same 0x0 DW_AT_locations being emitted.
   Let me know if I can provide any other information to try
to debug this further. Hopefully from all of the potential
test cases, there must be a pattern suggesting why these
additional zero AT_locations are being emitted on darwin.
                    Jack

Re: [VTA, PR41473] drop NULL locations from lists

[Jack Howarth]

On Tue, Nov 24, 2009 at 12:30:44AM -0200, Alexandre Oliva wrote:
> On Nov 23, 2009, Jack Howarth <howarth@bromo.med.uc.edu> wrote:
> 
> >     It appears that we still must be missing some cases were
> > AT_location is being set to zero
> 
> Can you name any?  I'm using a cross compiler, so I need a testcase that
> triggers the problem, its assembly with -dA comments and an indication
> of where the problem is in it.
> 
> -- 
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist      Red Hat Brazil Compiler Engineer

Alexandre,
   I should clarify as well that not every single testcase added to
PR41473 currently causes dsymutil to assert but they all can potentially
do that. As the dsymutil maintainer noted about the behavior of the 
current dsymutil with zero AT_locaations...

> It all comes down to luck of the draw with these binaries, as dsymutil might not 
> assert depending on what gets erroneously written where...
> dsymutil is incorrectly putting data into the middle of the DWARF debug 
> info and might cause an assert if we change the abbreviation code for a DIE (debug info entry).

So to resolve this issue for darwin9 and earlier, we will need to suppress
the emission of any zero AT_locations for DW block forms.
                     Jack

Re: [VTA, PR41473] drop NULL locations from lists

[Alexandre Oliva]

[-- Attachment #1: Type: text/plain, Size: 1161 bytes --]

On Nov 24, 2009, Jack Howarth <howarth@bromo.med.uc.edu> wrote:

> I am uncertain if the stock dwarfdump will provide the necessary
> information to find any residual zero locations.

That doesn't matter, I don't think I'd be able to use it in a
cross-compilation setting anyway.

On Nov 25, 2009, Jack Howarth <howarth@bromo.med.uc.edu> wrote:

> I've uploaded assembly files (generated with -dA) as well as the
> preprocessed source for all the object files in the FSF gcc trunk
> build with your proosed patch...

> http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01329.html

> that still emit zero AT_LOCATIONS and cause dsymutil to assert.
> These all seem to be of the form...

> .byte	0x0	# DW_AT_location

Thanks.  I picked up one of the testcases you posted and found out where
that location information comes from: it was a DWARF2-representable
location expression that referenced a symbol that, in the end, wasn't
emitted.

We used to simply zero out the location information.  With the patch
below, we'll remove attributes and location list entries that fail to
resolve.

Will you please give it a try and let me know whether any other issues
remain?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vta-dwarf2out-drop-unresolved-pr41473.patch --]
[-- Type: text/x-diff, Size: 2176 bytes --]

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/41473
	* dwarf2out.c (AT_loc_list_ptr): New.
	(resolve_addr): Remove unresolved attributes and loc_list entries.

Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c.orig	2009-11-26 06:40:48.000000000 -0200
+++ gcc/dwarf2out.c	2009-11-26 07:04:41.000000000 -0200
@@ -7206,6 +7206,13 @@ AT_loc_list (dw_attr_ref a)
   return a->dw_attr_val.v.val_loc_list;
 }
 
+static inline dw_loc_list_ref *
+AT_loc_list_ptr (dw_attr_ref a)
+{
+  gcc_assert (a && AT_class (a) == dw_val_class_loc_list);
+  return &a->dw_attr_val.v.val_loc_list;
+}
+
 /* Add an address constant attribute value to a DIE.  */
 
 static inline void
@@ -20961,28 +20968,48 @@ resolve_addr (dw_die_ref die)
 {
   dw_die_ref c;
   dw_attr_ref a;
-  dw_loc_list_ref curr;
+  dw_loc_list_ref *curr;
   unsigned ix;
 
   for (ix = 0; VEC_iterate (dw_attr_node, die->die_attr, ix, a); ix++)
     switch (AT_class (a))
       {
       case dw_val_class_loc_list:
-	for (curr = AT_loc_list (a); curr != NULL; curr = curr->dw_loc_next)
-	  if (!resolve_addr_in_expr (curr->expr))
-	    curr->expr = NULL;
+	curr = AT_loc_list_ptr (a);
+	while (*curr)
+	  {
+	    if (!resolve_addr_in_expr ((*curr)->expr))
+	      {
+		dw_loc_list_ref next = (*curr)->dw_loc_next;
+		if (next && (*curr)->ll_symbol)
+		  {
+		    gcc_assert (!next->ll_symbol);
+		    next->ll_symbol = (*curr)->ll_symbol;
+		  }
+		*curr = next;
+	      }
+	    else
+	      curr = &(*curr)->dw_loc_next;
+	  }
+	if (!AT_loc_list (a))
+	  {
+	    remove_AT (die, a->dw_attr);
+	    ix--;
+	  }
 	break;
       case dw_val_class_loc:
 	if (!resolve_addr_in_expr (AT_loc (a)))
-	  a->dw_attr_val.v.val_loc = NULL;
+	  {
+	    remove_AT (die, a->dw_attr);
+	    ix--;
+	  }
 	break;
       case dw_val_class_addr:
 	if (a->dw_attr == DW_AT_const_value
 	    && resolve_one_addr (&a->dw_attr_val.v.val_addr, NULL))
 	  {
-	    a->dw_attr = DW_AT_location;
-	    a->dw_attr_val.val_class = dw_val_class_loc;
-	    a->dw_attr_val.v.val_loc = NULL;
+	    remove_AT (die, a->dw_attr);
+	    ix--;
 	  }
 	break;
       default:

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

Re: [VTA, PR41473] drop NULL locations from lists

[Jack Howarth]

On Thu, Nov 26, 2009 at 07:13:36AM -0200, Alexandre Oliva wrote:
> On Nov 24, 2009, Jack Howarth <howarth@bromo.med.uc.edu> wrote:
> 
> > I am uncertain if the stock dwarfdump will provide the necessary
> > information to find any residual zero locations.
> 
> That doesn't matter, I don't think I'd be able to use it in a
> cross-compilation setting anyway.
> 
> On Nov 25, 2009, Jack Howarth <howarth@bromo.med.uc.edu> wrote:
> 
> > I've uploaded assembly files (generated with -dA) as well as the
> > preprocessed source for all the object files in the FSF gcc trunk
> > build with your proosed patch...
> 
> > http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01329.html
> 
> > that still emit zero AT_LOCATIONS and cause dsymutil to assert.
> > These all seem to be of the form...
> 
> > .byte	0x0	# DW_AT_location
> 
> Thanks.  I picked up one of the testcases you posted and found out where
> that location information comes from: it was a DWARF2-representable
> location expression that referenced a symbol that, in the end, wasn't
> emitted.
> 
> We used to simply zero out the location information.  With the patch
> below, we'll remove attributes and location list entries that fail to
> resolve.
> 
> Will you please give it a try and let me know whether any other issues
> remain?
> 

> for  gcc/ChangeLog
> from  Alexandre Oliva  <aoliva@redhat.com>
> 
> 	PR debug/41473
> 	* dwarf2out.c (AT_loc_list_ptr): New.
> 	(resolve_addr): Remove unresolved attributes and loc_list entries.
> 
> Index: gcc/dwarf2out.c
> ===================================================================
> --- gcc/dwarf2out.c.orig	2009-11-26 06:40:48.000000000 -0200
> +++ gcc/dwarf2out.c	2009-11-26 07:04:41.000000000 -0200
> @@ -7206,6 +7206,13 @@ AT_loc_list (dw_attr_ref a)
>    return a->dw_attr_val.v.val_loc_list;
>  }
>  
> +static inline dw_loc_list_ref *
> +AT_loc_list_ptr (dw_attr_ref a)
> +{
> +  gcc_assert (a && AT_class (a) == dw_val_class_loc_list);
> +  return &a->dw_attr_val.v.val_loc_list;
> +}
> +
>  /* Add an address constant attribute value to a DIE.  */
>  
>  static inline void
> @@ -20961,28 +20968,48 @@ resolve_addr (dw_die_ref die)
>  {
>    dw_die_ref c;
>    dw_attr_ref a;
> -  dw_loc_list_ref curr;
> +  dw_loc_list_ref *curr;
>    unsigned ix;
>  
>    for (ix = 0; VEC_iterate (dw_attr_node, die->die_attr, ix, a); ix++)
>      switch (AT_class (a))
>        {
>        case dw_val_class_loc_list:
> -	for (curr = AT_loc_list (a); curr != NULL; curr = curr->dw_loc_next)
> -	  if (!resolve_addr_in_expr (curr->expr))
> -	    curr->expr = NULL;
> +	curr = AT_loc_list_ptr (a);
> +	while (*curr)
> +	  {
> +	    if (!resolve_addr_in_expr ((*curr)->expr))
> +	      {
> +		dw_loc_list_ref next = (*curr)->dw_loc_next;
> +		if (next && (*curr)->ll_symbol)
> +		  {
> +		    gcc_assert (!next->ll_symbol);
> +		    next->ll_symbol = (*curr)->ll_symbol;
> +		  }
> +		*curr = next;
> +	      }
> +	    else
> +	      curr = &(*curr)->dw_loc_next;
> +	  }
> +	if (!AT_loc_list (a))
> +	  {
> +	    remove_AT (die, a->dw_attr);
> +	    ix--;
> +	  }
>  	break;
>        case dw_val_class_loc:
>  	if (!resolve_addr_in_expr (AT_loc (a)))
> -	  a->dw_attr_val.v.val_loc = NULL;
> +	  {
> +	    remove_AT (die, a->dw_attr);
> +	    ix--;
> +	  }
>  	break;
>        case dw_val_class_addr:
>  	if (a->dw_attr == DW_AT_const_value
>  	    && resolve_one_addr (&a->dw_attr_val.v.val_addr, NULL))
>  	  {
> -	    a->dw_attr = DW_AT_location;
> -	    a->dw_attr_val.val_class = dw_val_class_loc;
> -	    a->dw_attr_val.v.val_loc = NULL;
> +	    remove_AT (die, a->dw_attr);
> +	    ix--;
>  	  }
>  	break;
>        default:

> 
> -- 
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist      Red Hat Brazil Compiler Engineer

Alexandre,
   I am testing now, but could you confirm that this patch is a
stand-alone fix for the NULL DW_AT_location problem on darwin or
should it be used in concert with your previous patch...

http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01329.html

If so, I believe that patch has suffered bit rot and not longer
cleanly applies to gcc trunk.
               Jack

Re: [VTA, PR41473] drop NULL locations from lists

[Jack Howarth]

On Thu, Nov 26, 2009 at 07:13:36AM -0200, Alexandre Oliva wrote:
> On Nov 24, 2009, Jack Howarth <howarth@bromo.med.uc.edu> wrote:
> 
> > I am uncertain if the stock dwarfdump will provide the necessary
> > information to find any residual zero locations.
> 
> That doesn't matter, I don't think I'd be able to use it in a
> cross-compilation setting anyway.
> 
> On Nov 25, 2009, Jack Howarth <howarth@bromo.med.uc.edu> wrote:
> 
> > I've uploaded assembly files (generated with -dA) as well as the
> > preprocessed source for all the object files in the FSF gcc trunk
> > build with your proosed patch...
> 
> > http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01329.html
> 
> > that still emit zero AT_LOCATIONS and cause dsymutil to assert.
> > These all seem to be of the form...
> 
> > .byte	0x0	# DW_AT_location
> 
> Thanks.  I picked up one of the testcases you posted and found out where
> that location information comes from: it was a DWARF2-representable
> location expression that referenced a symbol that, in the end, wasn't
> emitted.
> 
> We used to simply zero out the location information.  With the patch
> below, we'll remove attributes and location list entries that fail to
> resolve.
> 
> Will you please give it a try and let me know whether any other issues
> remain?
> 

> for  gcc/ChangeLog
> from  Alexandre Oliva  <aoliva@redhat.com>
> 
> 	PR debug/41473
> 	* dwarf2out.c (AT_loc_list_ptr): New.
> 	(resolve_addr): Remove unresolved attributes and loc_list entries.
> 
> Index: gcc/dwarf2out.c
> ===================================================================
> --- gcc/dwarf2out.c.orig	2009-11-26 06:40:48.000000000 -0200
> +++ gcc/dwarf2out.c	2009-11-26 07:04:41.000000000 -0200
> @@ -7206,6 +7206,13 @@ AT_loc_list (dw_attr_ref a)
>    return a->dw_attr_val.v.val_loc_list;
>  }
>  
> +static inline dw_loc_list_ref *
> +AT_loc_list_ptr (dw_attr_ref a)
> +{
> +  gcc_assert (a && AT_class (a) == dw_val_class_loc_list);
> +  return &a->dw_attr_val.v.val_loc_list;
> +}
> +
>  /* Add an address constant attribute value to a DIE.  */
>  
>  static inline void
> @@ -20961,28 +20968,48 @@ resolve_addr (dw_die_ref die)
>  {
>    dw_die_ref c;
>    dw_attr_ref a;
> -  dw_loc_list_ref curr;
> +  dw_loc_list_ref *curr;
>    unsigned ix;
>  
>    for (ix = 0; VEC_iterate (dw_attr_node, die->die_attr, ix, a); ix++)
>      switch (AT_class (a))
>        {
>        case dw_val_class_loc_list:
> -	for (curr = AT_loc_list (a); curr != NULL; curr = curr->dw_loc_next)
> -	  if (!resolve_addr_in_expr (curr->expr))
> -	    curr->expr = NULL;
> +	curr = AT_loc_list_ptr (a);
> +	while (*curr)
> +	  {
> +	    if (!resolve_addr_in_expr ((*curr)->expr))
> +	      {
> +		dw_loc_list_ref next = (*curr)->dw_loc_next;
> +		if (next && (*curr)->ll_symbol)
> +		  {
> +		    gcc_assert (!next->ll_symbol);
> +		    next->ll_symbol = (*curr)->ll_symbol;
> +		  }
> +		*curr = next;
> +	      }
> +	    else
> +	      curr = &(*curr)->dw_loc_next;
> +	  }
> +	if (!AT_loc_list (a))
> +	  {
> +	    remove_AT (die, a->dw_attr);
> +	    ix--;
> +	  }
>  	break;
>        case dw_val_class_loc:
>  	if (!resolve_addr_in_expr (AT_loc (a)))
> -	  a->dw_attr_val.v.val_loc = NULL;
> +	  {
> +	    remove_AT (die, a->dw_attr);
> +	    ix--;
> +	  }
>  	break;
>        case dw_val_class_addr:
>  	if (a->dw_attr == DW_AT_const_value
>  	    && resolve_one_addr (&a->dw_attr_val.v.val_addr, NULL))
>  	  {
> -	    a->dw_attr = DW_AT_location;
> -	    a->dw_attr_val.val_class = dw_val_class_loc;
> -	    a->dw_attr_val.v.val_loc = NULL;
> +	    remove_AT (die, a->dw_attr);
> +	    ix--;
>  	  }
>  	break;
>        default:

> 
> -- 
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist      Red Hat Brazil Compiler Engineer


Alexandre,
   This patch alone is insufficient to eliminate all of the asserts in
dsymutil but if I apply it in combination with the previous patch...

http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01329.html

all of the asserts in dsymutil are eliminated during the build. So
we definitely need both patches to solve the problem on darwin.
Thanks for fixing this issue.
                    Jack

Re: [VTA, PR41473] drop NULL locations from lists

[Jack Howarth]

On Thu, Nov 26, 2009 at 07:13:36AM -0200, Alexandre Oliva wrote:
> On Nov 24, 2009, Jack Howarth <howarth@bromo.med.uc.edu> wrote:
> 
> > I am uncertain if the stock dwarfdump will provide the necessary
> > information to find any residual zero locations.
> 
> That doesn't matter, I don't think I'd be able to use it in a
> cross-compilation setting anyway.
> 
> On Nov 25, 2009, Jack Howarth <howarth@bromo.med.uc.edu> wrote:
> 
> > I've uploaded assembly files (generated with -dA) as well as the
> > preprocessed source for all the object files in the FSF gcc trunk
> > build with your proosed patch...
> 
> > http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01329.html
> 
> > that still emit zero AT_LOCATIONS and cause dsymutil to assert.
> > These all seem to be of the form...
> 
> > .byte	0x0	# DW_AT_location
> 
> Thanks.  I picked up one of the testcases you posted and found out where
> that location information comes from: it was a DWARF2-representable
> location expression that referenced a symbol that, in the end, wasn't
> emitted.
> 
> We used to simply zero out the location information.  With the patch
> below, we'll remove attributes and location list entries that fail to
> resolve.
> 
> Will you please give it a try and let me know whether any other issues
> remain?
> 

> for  gcc/ChangeLog
> from  Alexandre Oliva  <aoliva@redhat.com>
> 
> 	PR debug/41473
> 	* dwarf2out.c (AT_loc_list_ptr): New.
> 	(resolve_addr): Remove unresolved attributes and loc_list entries.
> 
> Index: gcc/dwarf2out.c
> ===================================================================
> --- gcc/dwarf2out.c.orig	2009-11-26 06:40:48.000000000 -0200
> +++ gcc/dwarf2out.c	2009-11-26 07:04:41.000000000 -0200
> @@ -7206,6 +7206,13 @@ AT_loc_list (dw_attr_ref a)
>    return a->dw_attr_val.v.val_loc_list;
>  }
>  
> +static inline dw_loc_list_ref *
> +AT_loc_list_ptr (dw_attr_ref a)
> +{
> +  gcc_assert (a && AT_class (a) == dw_val_class_loc_list);
> +  return &a->dw_attr_val.v.val_loc_list;
> +}
> +
>  /* Add an address constant attribute value to a DIE.  */
>  
>  static inline void
> @@ -20961,28 +20968,48 @@ resolve_addr (dw_die_ref die)
>  {
>    dw_die_ref c;
>    dw_attr_ref a;
> -  dw_loc_list_ref curr;
> +  dw_loc_list_ref *curr;
>    unsigned ix;
>  
>    for (ix = 0; VEC_iterate (dw_attr_node, die->die_attr, ix, a); ix++)
>      switch (AT_class (a))
>        {
>        case dw_val_class_loc_list:
> -	for (curr = AT_loc_list (a); curr != NULL; curr = curr->dw_loc_next)
> -	  if (!resolve_addr_in_expr (curr->expr))
> -	    curr->expr = NULL;
> +	curr = AT_loc_list_ptr (a);
> +	while (*curr)
> +	  {
> +	    if (!resolve_addr_in_expr ((*curr)->expr))
> +	      {
> +		dw_loc_list_ref next = (*curr)->dw_loc_next;
> +		if (next && (*curr)->ll_symbol)
> +		  {
> +		    gcc_assert (!next->ll_symbol);
> +		    next->ll_symbol = (*curr)->ll_symbol;
> +		  }
> +		*curr = next;
> +	      }
> +	    else
> +	      curr = &(*curr)->dw_loc_next;
> +	  }
> +	if (!AT_loc_list (a))
> +	  {
> +	    remove_AT (die, a->dw_attr);
> +	    ix--;
> +	  }
>  	break;
>        case dw_val_class_loc:
>  	if (!resolve_addr_in_expr (AT_loc (a)))
> -	  a->dw_attr_val.v.val_loc = NULL;
> +	  {
> +	    remove_AT (die, a->dw_attr);
> +	    ix--;
> +	  }
>  	break;
>        case dw_val_class_addr:
>  	if (a->dw_attr == DW_AT_const_value
>  	    && resolve_one_addr (&a->dw_attr_val.v.val_addr, NULL))
>  	  {
> -	    a->dw_attr = DW_AT_location;
> -	    a->dw_attr_val.val_class = dw_val_class_loc;
> -	    a->dw_attr_val.v.val_loc = NULL;
> +	    remove_AT (die, a->dw_attr);
> +	    ix--;
>  	  }
>  	break;
>        default:

> 
> -- 
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist      Red Hat Brazil Compiler Engineer


Alexandre,
    A couple other users,  Dominique d'Humieres and Andreas Tobler,
have tried the combination of the two patches...

http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01329.html
http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01452.html

and it appears to be eliminating the dsymutil asserts for everyone.
Can we get this into gcc trunk before it leaves stage 3? It should
be okay afterwards since PR41473 does represent a regresssion from
gcc 4.4.2 on darwin. I just wanted to make certain. These patches
have been a huge help in allowing me to debug PR41991 in gcc trunk
since without them either the debug info in the dSYM will be missing
or corrupted. Thanks in advance.
                Jack

[PING]: [VTA, PR41473] drop NULL locations from lists

[Jack Howarth]

Alexandre,
     Can we get this patch which, when coupled with...

http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01452.html

, completely eliminates the crashes in dsymutil on darwin
into gcc trunk? Thanks in advance.
                 Jack


On Tue, Nov 24, 2009 at 12:39:12AM -0200, Alexandre Oliva wrote:
> On Nov 23, 2009, Cary Coutant <ccoutant@google.com> wrote:
> 
> > Jakub wrote:
> >>> Are you sure the dropping of single_element_loc_list call in the
> >>> loc-> first && loc->last == loc->first case is a good idea?
> 
> It wasn't.  I didn't realize it was needed for the block-form
> optimization to work.  However, I think the optimization is a bit
> excessive.  We don't want to say a variable is at a certain location
> throughout its lifetime just because it happens to be there on a narrow
> range of PCs.
> 
> > And I replied:
> >> I don't think that would mean the same thing: a single-element loc
> >> list describes a variable whose value is known only for that one
> >> range, where a block form location expression describes a variable
> >> that can always be found at the given location.
> 
> > Hmmm, it looks like add_AT_location_description() is already going to
> > convert single-element loc lists into a simple block-form location
> > expression. In the case of a single-element location list where the
> > range is constrained, is that really the right thing to do?
> 
> If there aren't any other entries, we could probably assume they're
> equivalent, i.e., that the variable is set when we enter its scope, and
> remains live till the end.  This may not always hold, though: if the
> variable is set later in the scope, we lose.
> 
> Nevertheless, I've (partially) restored this functionality in the patch
> below.  It retains var_loc_notes with NULL locs until the time of
> converting lists to the internal dwarf representation, and then it only
> applies the optimization if there's really no more than a single entry
> in the incoming location list.
> 
> I've also fixed an assertion check I ran into on
> x86_64-linux-gnu/libgcc: a VOIDmode CONST_DOUBLE failed the mode ==
> GET_MODE newly-introduced assertion, so I realized it to accept incoming
> non-VOID modes with VOIDmode consts.
> 
> This one has passed bootstrap compare on x86_64-linux-gnu already,
> building target libs now.
> 

> for  gcc/ChangeLog
> from  Alexandre Oliva  <aoliva@redhat.com>
> 
> 	PR debug/41473
> 	* dwarf2out.c (add_var_loc_to_decl): Don't drop initial empty
> 	locations.
> 	(new_loc_list): Drop gensym arg.  Move generation of ll_symbol...
> 	(gen_llsym): ... here.  New function.
> 	(add_loc_descr_to_loc_list): Removed.
> 	(loc_descriptor): Infer mode from CONST_DOUBLEs and CONST_VECTORs.
> 	(single_element_loc_list): Removed.
> 	(dw_loc_list): Don't create entries without a location.  Don't
> 	special-case the first node of the list, only single nodes.
> 	(single_element_loc_list_p): Simplify.
> 	(loc_list_from_tree): Don't use DECL_RTL if loc_list is nonempty.
> 	(add_location_or_const_value_attribute): Test var loc for NULL.
> 	(convert_cfa_to_fb_loc_list): Adjust calls to new new_loc_list,
> 	call gen_llsym if needed.
> 
> Index: gcc/dwarf2out.c
> ===================================================================
> --- gcc/dwarf2out.c.orig	2009-11-23 18:12:26.000000000 -0200
> +++ gcc/dwarf2out.c	2009-11-23 23:41:40.000000000 -0200
> @@ -6172,10 +6172,7 @@ static void gen_generic_params_dies (tre
>  static void splice_child_die (dw_die_ref, dw_die_ref);
>  static int file_info_cmp (const void *, const void *);
>  static dw_loc_list_ref new_loc_list (dw_loc_descr_ref, const char *,
> -				     const char *, const char *, unsigned);
> -static void add_loc_descr_to_loc_list (dw_loc_list_ref *, dw_loc_descr_ref,
> -				       const char *, const char *,
> -				       const char *);
> +				     const char *, const char *);
>  static void output_loc_list (dw_loc_list_ref);
>  static char *gen_internal_sym (const char *);
>  
> @@ -7788,8 +7785,7 @@ add_var_loc_to_decl (tree decl, struct v
>  	  temp->last = loc;
>  	}
>      }
> -  /* Do not add empty location to the beginning of the list.  */
> -  else if (NOTE_VAR_LOCATION_LOC (loc->var_loc_note) != NULL_RTX)
> +  else
>      {
>        temp->first = loc;
>        temp->last = loc;
> @@ -10297,12 +10293,11 @@ output_die_symbol (dw_die_ref die)
>  }
>  
>  /* Return a new location list, given the begin and end range, and the
> -   expression. gensym tells us whether to generate a new internal symbol for
> -   this location list node, which is done for the head of the list only.  */
> +   expression.  */
>  
>  static inline dw_loc_list_ref
>  new_loc_list (dw_loc_descr_ref expr, const char *begin, const char *end,
> -	      const char *section, unsigned int gensym)
> +	      const char *section)
>  {
>    dw_loc_list_ref retlist = GGC_CNEW (dw_loc_list_node);
>  
> @@ -10310,27 +10305,18 @@ new_loc_list (dw_loc_descr_ref expr, con
>    retlist->end = end;
>    retlist->expr = expr;
>    retlist->section = section;
> -  if (gensym)
> -    retlist->ll_symbol = gen_internal_sym ("LLST");
>  
>    return retlist;
>  }
>  
> -/* Add a location description expression to a location list.  */
> +/* Generate a new internal symbol for this location list node, if it
> +   hasn't got one yet.  */
>  
>  static inline void
> -add_loc_descr_to_loc_list (dw_loc_list_ref *list_head, dw_loc_descr_ref descr,
> -			   const char *begin, const char *end,
> -			   const char *section)
> +gen_llsym (dw_loc_list_ref list)
>  {
> -  dw_loc_list_ref *d;
> -
> -  /* Find the end of the chain.  */
> -  for (d = list_head; (*d) != NULL; d = &(*d)->dw_loc_next)
> -    ;
> -
> -  /* Add a new location list node to the list.  */
> -  *d = new_loc_list (descr, begin, end, section, 0);
> +  gcc_assert (!list->ll_symbol);
> +  list->ll_symbol = gen_internal_sym ("LLST");
>  }
>  
>  /* Output the location list given to us.  */
> @@ -13650,15 +13636,17 @@ loc_descriptor (rtx rtl, enum machine_mo
>        break;
>  
>      case CONST_DOUBLE:
> +      if (mode == VOIDmode)
> +	mode = GET_MODE (rtl);
> +
>        if (mode != VOIDmode && (dwarf_version >= 4 || !dwarf_strict))
>  	{
> +	  gcc_assert (mode == GET_MODE (rtl) || VOIDmode == GET_MODE (rtl));
> +
>  	  /* Note that a CONST_DOUBLE rtx could represent either an integer
>  	     or a floating-point constant.  A CONST_DOUBLE is used whenever
>  	     the constant requires more than one word in order to be
>  	     adequately represented.  We output CONST_DOUBLEs as blocks.  */
> -	  if (GET_MODE (rtl) != VOIDmode)
> -	    mode = GET_MODE (rtl);
> -
>  	  loc_result = new_loc_descr (DW_OP_implicit_value,
>  				      GET_MODE_SIZE (mode), 0);
>  	  if (SCALAR_FLOAT_MODE_P (mode))
> @@ -13684,6 +13672,9 @@ loc_descriptor (rtx rtl, enum machine_mo
>        break;
>  
>      case CONST_VECTOR:
> +      if (mode == VOIDmode)
> +	mode = GET_MODE (rtl);
> +
>        if (mode != VOIDmode && (dwarf_version >= 4 || !dwarf_strict))
>  	{
>  	  unsigned int elt_size = GET_MODE_UNIT_SIZE (GET_MODE (rtl));
> @@ -13692,7 +13683,7 @@ loc_descriptor (rtx rtl, enum machine_mo
>  	  unsigned int i;
>  	  unsigned char *p;
>  
> -	  mode = GET_MODE (rtl);
> +	  gcc_assert (mode == GET_MODE (rtl) || VOIDmode == GET_MODE (rtl));
>  	  switch (GET_MODE_CLASS (mode))
>  	    {
>  	    case MODE_VECTOR_INT:
> @@ -13839,14 +13830,6 @@ decl_by_reference_p (tree decl)
>  	  && DECL_BY_REFERENCE (decl));
>  }
>  
> -/* Return single element location list containing loc descr REF.  */
> -
> -static dw_loc_list_ref
> -single_element_loc_list (dw_loc_descr_ref ref)
> -{
> -  return new_loc_list (ref, NULL, NULL, NULL, 0);
> -}
> -
>  /* Helper function for dw_loc_list.  Compute proper Dwarf location descriptor
>     for VARLOC.  */
>  
> @@ -13928,20 +13911,21 @@ dw_loc_list_1 (tree loc, rtx varloc, int
>    return descr;
>  }
>  
> -/* Return dwarf representation of location list representing for
> -   LOC_LIST of DECL.  WANT_ADDRESS has the same meaning as in
> -   loc_list_from_tree function.  */
> +/* Return the dwarf representation of the location list LOC_LIST of
> +   DECL.  WANT_ADDRESS has the same meaning as in loc_list_from_tree
> +   function.  */
>  
>  static dw_loc_list_ref
> -dw_loc_list (var_loc_list * loc_list, tree decl, int want_address)
> +dw_loc_list (var_loc_list *loc_list, tree decl, int want_address)
>  {
>    const char *endname, *secname;
> -  dw_loc_list_ref list;
>    rtx varloc;
>    enum var_init_status initialized;
>    struct var_loc_node *node;
>    dw_loc_descr_ref descr;
>    char label_id[MAX_ARTIFICIAL_LABEL_BYTES];
> +  dw_loc_list_ref list = NULL;
> +  dw_loc_list_ref *listp = &list;
>  
>    /* Now that we know what section we are using for a base,
>       actually construct the list of locations.
> @@ -13954,26 +13938,9 @@ dw_loc_list (var_loc_list * loc_list, tr
>       This means we have to special case the last node, and generate
>       a range of [last location start, end of function label].  */
>  
> -  node = loc_list->first;
>    secname = secname_for_decl (decl);
>  
> -  if (NOTE_VAR_LOCATION_LOC (node->var_loc_note))
> -    initialized = NOTE_VAR_LOCATION_STATUS (node->var_loc_note);
> -  else
> -    initialized = VAR_INIT_STATUS_INITIALIZED;
> -  varloc = NOTE_VAR_LOCATION (node->var_loc_note);
> -  descr = dw_loc_list_1 (decl, varloc, want_address, initialized);
> -
> -  if (loc_list && loc_list->first != loc_list->last)
> -    list = new_loc_list (descr, node->label, node->next->label, secname, 1);
> -  else
> -    return single_element_loc_list (descr);
> -  node = node->next;
> -
> -  if (!node)
> -    return NULL;
> -
> -  for (; node->next; node = node->next)
> +  for (node = loc_list->first; node->next; node = node->next)
>      if (NOTE_VAR_LOCATION_LOC (node->var_loc_note) != NULL_RTX)
>        {
>  	/* The variable has a location between NODE->LABEL and
> @@ -13981,28 +13948,46 @@ dw_loc_list (var_loc_list * loc_list, tr
>  	initialized = NOTE_VAR_LOCATION_STATUS (node->var_loc_note);
>  	varloc = NOTE_VAR_LOCATION (node->var_loc_note);
>  	descr = dw_loc_list_1 (decl, varloc, want_address, initialized);
> -	add_loc_descr_to_loc_list (&list, descr,
> -				   node->label, node->next->label, secname);
> +	if (descr)
> +	  {
> +	    *listp = new_loc_list (descr, node->label, node->next->label,
> +				   secname);
> +	    listp = &(*listp)->dw_loc_next;
> +	  }
>        }
>  
>    /* If the variable has a location at the last label
>       it keeps its location until the end of function.  */
>    if (NOTE_VAR_LOCATION_LOC (node->var_loc_note) != NULL_RTX)
>      {
> -      if (!current_function_decl)
> -	endname = text_end_label;
> -      else
> -	{
> -	  ASM_GENERATE_INTERNAL_LABEL (label_id, FUNC_END_LABEL,
> -				       current_function_funcdef_no);
> -	  endname = ggc_strdup (label_id);
> -	}
> -
>        initialized = NOTE_VAR_LOCATION_STATUS (node->var_loc_note);
>        varloc = NOTE_VAR_LOCATION (node->var_loc_note);
>        descr = dw_loc_list_1 (decl, varloc, want_address, initialized);
> -      add_loc_descr_to_loc_list (&list, descr, node->label, endname, secname);
> +      if (descr)
> +	{
> +	  if (!current_function_decl)
> +	    endname = text_end_label;
> +	  else
> +	    {
> +	      ASM_GENERATE_INTERNAL_LABEL (label_id, FUNC_END_LABEL,
> +					   current_function_funcdef_no);
> +	      endname = ggc_strdup (label_id);
> +	    }
> +
> +	  *listp = new_loc_list (descr, node->label, endname, secname);
> +	  listp = &(*listp)->dw_loc_next;
> +	}
>      }
> +
> +  /* Try to avoid the overhead of a location list emitting a location
> +     expression instead, but only if we didn't have more than one
> +     location entry in the first place.  If some entries were not
> +     representable, we don't want to pretend a single entry that was
> +     applies to the entire scope in which the variable is
> +     available.  */
> +  if (list && loc_list->first->next)
> +    gen_llsym (list);
> +
>    return list;
>  }
>  
> @@ -14012,7 +13997,8 @@ dw_loc_list (var_loc_list * loc_list, tr
>  static bool
>  single_element_loc_list_p (dw_loc_list_ref list)
>  {
> -  return (!list->dw_loc_next && !list->begin && !list->end);
> +  gcc_assert (!list->dw_loc_next || list->ll_symbol);
> +  return !list->ll_symbol;
>  }
>  
>  /* To each location in list LIST add loc descr REF.  */
> @@ -14312,9 +14298,9 @@ loc_list_from_tree (tree loc, int want_a
>  	rtx rtl;
>  	var_loc_list *loc_list = lookup_decl_loc (loc);
>  
> -	if (loc_list && loc_list->first
> -	    && (list_ret = dw_loc_list (loc_list, loc, want_address)))
> +	if (loc_list && loc_list->first)
>  	  {
> +	    list_ret = dw_loc_list (loc_list, loc, want_address);
>  	    have_address = want_address != 0;
>  	    break;
>  	  }
> @@ -14725,7 +14711,7 @@ loc_list_from_tree (tree loc, int want_a
>  	add_loc_descr_to_each (list_ret, new_loc_descr (op, size, 0));
>      }
>    if (ret)
> -    list_ret = single_element_loc_list (ret);
> +    list_ret = new_loc_list (ret, NULL, NULL, NULL);
>  
>    return list_ret;
>  }
> @@ -15719,17 +15705,19 @@ add_location_or_const_value_attribute (d
>       a constant value.  That way we are better to use add_const_value_attribute
>       rather than expanding constant value equivalent.  */
>    loc_list = lookup_decl_loc (decl);
> -  if (loc_list && loc_list->first && loc_list->first == loc_list->last)
> +  if (loc_list && loc_list->first && loc_list->first == loc_list->last
> +      && loc_list->first->var_loc_note
> +      && NOTE_VAR_LOCATION (loc_list->first->var_loc_note)
> +      && NOTE_VAR_LOCATION_LOC (loc_list->first->var_loc_note))
>      {
>        enum var_init_status status;
>        struct var_loc_node *node;
>  
>        node = loc_list->first;
>        status = NOTE_VAR_LOCATION_STATUS (node->var_loc_note);
> -      rtl = NOTE_VAR_LOCATION (node->var_loc_note);
> -      if (GET_CODE (rtl) == VAR_LOCATION
> -	  && GET_CODE (XEXP (rtl, 1)) != PARALLEL)
> -	rtl = XEXP (XEXP (rtl, 1), 0);
> +      rtl = NOTE_VAR_LOCATION_LOC (node->var_loc_note);
> +      if (GET_CODE (rtl) != PARALLEL)
> +	rtl = XEXP (rtl, 0);
>        if ((CONSTANT_P (rtl) || GET_CODE (rtl) == CONST_STRING)
>  	  && add_const_value_attribute (die, rtl))
>  	 return true;
> @@ -16016,8 +16004,7 @@ convert_cfa_to_fb_loc_list (HOST_WIDE_IN
>  	if (!cfa_equal_p (&last_cfa, &next_cfa))
>  	  {
>  	    *list_tail = new_loc_list (build_cfa_loc (&last_cfa, offset),
> -				       start_label, last_label, section,
> -				       list == NULL);
> +				       start_label, last_label, section);
>  
>  	    list_tail = &(*list_tail)->dw_loc_next;
>  	    last_cfa = next_cfa;
> @@ -16038,14 +16025,16 @@ convert_cfa_to_fb_loc_list (HOST_WIDE_IN
>    if (!cfa_equal_p (&last_cfa, &next_cfa))
>      {
>        *list_tail = new_loc_list (build_cfa_loc (&last_cfa, offset),
> -				 start_label, last_label, section,
> -				 list == NULL);
> +				 start_label, last_label, section);
>        list_tail = &(*list_tail)->dw_loc_next;
>        start_label = last_label;
>      }
> +
>    *list_tail = new_loc_list (build_cfa_loc (&next_cfa, offset),
> -			     start_label, fde->dw_fde_end, section,
> -			     list == NULL);
> +			     start_label, fde->dw_fde_end, section);
> +
> +  if (list && list->dw_loc_next)
> +    gen_llsym (list);
>  
>    return list;
>  }

> 
> -- 
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist      Red Hat Brazil Compiler Engineer

PING Pt2: [VTA, PR41473] drop NULL locations from list

[Jack Howarth]

Alexandre,
    Can we get this patch which when coupled with...

http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01329.html

completely eliminates the dsymutil crashes on darwin into
gcc trunk? Thanks in advance.
                          Jack


On Thu, Nov 26, 2009 at 07:13:36AM -0200, Alexandre Oliva wrote:
> On Nov 24, 2009, Jack Howarth <howarth@bromo.med.uc.edu> wrote:
> 
> > I am uncertain if the stock dwarfdump will provide the necessary
> > information to find any residual zero locations.
> 
> That doesn't matter, I don't think I'd be able to use it in a
> cross-compilation setting anyway.
> 
> On Nov 25, 2009, Jack Howarth <howarth@bromo.med.uc.edu> wrote:
> 
> > I've uploaded assembly files (generated with -dA) as well as the
> > preprocessed source for all the object files in the FSF gcc trunk
> > build with your proosed patch...
> 
> > http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01329.html
> 
> > that still emit zero AT_LOCATIONS and cause dsymutil to assert.
> > These all seem to be of the form...
> 
> > .byte	0x0	# DW_AT_location
> 
> Thanks.  I picked up one of the testcases you posted and found out where
> that location information comes from: it was a DWARF2-representable
> location expression that referenced a symbol that, in the end, wasn't
> emitted.
> 
> We used to simply zero out the location information.  With the patch
> below, we'll remove attributes and location list entries that fail to
> resolve.
> 
> Will you please give it a try and let me know whether any other issues
> remain?
> 

> for  gcc/ChangeLog
> from  Alexandre Oliva  <aoliva@redhat.com>
> 
> 	PR debug/41473
> 	* dwarf2out.c (AT_loc_list_ptr): New.
> 	(resolve_addr): Remove unresolved attributes and loc_list entries.
> 
> Index: gcc/dwarf2out.c
> ===================================================================
> --- gcc/dwarf2out.c.orig	2009-11-26 06:40:48.000000000 -0200
> +++ gcc/dwarf2out.c	2009-11-26 07:04:41.000000000 -0200
> @@ -7206,6 +7206,13 @@ AT_loc_list (dw_attr_ref a)
>    return a->dw_attr_val.v.val_loc_list;
>  }
>  
> +static inline dw_loc_list_ref *
> +AT_loc_list_ptr (dw_attr_ref a)
> +{
> +  gcc_assert (a && AT_class (a) == dw_val_class_loc_list);
> +  return &a->dw_attr_val.v.val_loc_list;
> +}
> +
>  /* Add an address constant attribute value to a DIE.  */
>  
>  static inline void
> @@ -20961,28 +20968,48 @@ resolve_addr (dw_die_ref die)
>  {
>    dw_die_ref c;
>    dw_attr_ref a;
> -  dw_loc_list_ref curr;
> +  dw_loc_list_ref *curr;
>    unsigned ix;
>  
>    for (ix = 0; VEC_iterate (dw_attr_node, die->die_attr, ix, a); ix++)
>      switch (AT_class (a))
>        {
>        case dw_val_class_loc_list:
> -	for (curr = AT_loc_list (a); curr != NULL; curr = curr->dw_loc_next)
> -	  if (!resolve_addr_in_expr (curr->expr))
> -	    curr->expr = NULL;
> +	curr = AT_loc_list_ptr (a);
> +	while (*curr)
> +	  {
> +	    if (!resolve_addr_in_expr ((*curr)->expr))
> +	      {
> +		dw_loc_list_ref next = (*curr)->dw_loc_next;
> +		if (next && (*curr)->ll_symbol)
> +		  {
> +		    gcc_assert (!next->ll_symbol);
> +		    next->ll_symbol = (*curr)->ll_symbol;
> +		  }
> +		*curr = next;
> +	      }
> +	    else
> +	      curr = &(*curr)->dw_loc_next;
> +	  }
> +	if (!AT_loc_list (a))
> +	  {
> +	    remove_AT (die, a->dw_attr);
> +	    ix--;
> +	  }
>  	break;
>        case dw_val_class_loc:
>  	if (!resolve_addr_in_expr (AT_loc (a)))
> -	  a->dw_attr_val.v.val_loc = NULL;
> +	  {
> +	    remove_AT (die, a->dw_attr);
> +	    ix--;
> +	  }
>  	break;
>        case dw_val_class_addr:
>  	if (a->dw_attr == DW_AT_const_value
>  	    && resolve_one_addr (&a->dw_attr_val.v.val_addr, NULL))
>  	  {
> -	    a->dw_attr = DW_AT_location;
> -	    a->dw_attr_val.val_class = dw_val_class_loc;
> -	    a->dw_attr_val.v.val_loc = NULL;
> +	    remove_AT (die, a->dw_attr);
> +	    ix--;
>  	  }
>  	break;
>        default:

> 
> -- 
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist      Red Hat Brazil Compiler Engineer

Re: [VTA, PR41473] drop NULL locations from lists

[Jack Howarth]

Alexandre,
   Now that that libtool update has been committed to gcc trunk, r155012,
we have eliminated the previous issues with libtool convenience archives
deleting debug code for parts of the gcc build. At this point, the only
outstanding issue with debugging on darwin is the crashes in dsymutil
which your proposed patches...

http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01329.html
http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01452.html

eliminate. Can we please get those into gcc 4.5 before the branch occurs?
They have been extremely helpful in debugging a crash in gcj (PR 41991).

Thanks in advance for helping to get those patches into gcc trunk.
                    Jack

[PINGx2]: [VTA, PR41473] drop NULL locations from lists

[Jack Howarth]

   Can someone please review and commit this patch so
that darwin can retain full debug code in FSF gcc?
      Thanks in advance.
               Jack

On Tue, Dec 01, 2009 at 03:14:24PM -0500, Jack Howarth wrote:
> Alexandre,
>      Can we get this patch which, when coupled with...
> 
> http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01452.html
> 
> , completely eliminates the crashes in dsymutil on darwin
> into gcc trunk? Thanks in advance.
>                  Jack
> 
> 
> On Tue, Nov 24, 2009 at 12:39:12AM -0200, Alexandre Oliva wrote:
> > On Nov 23, 2009, Cary Coutant <ccoutant@google.com> wrote:
> > 
> > > Jakub wrote:
> > >>> Are you sure the dropping of single_element_loc_list call in the
> > >>> loc-> first && loc->last == loc->first case is a good idea?
> > 
> > It wasn't.  I didn't realize it was needed for the block-form
> > optimization to work.  However, I think the optimization is a bit
> > excessive.  We don't want to say a variable is at a certain location
> > throughout its lifetime just because it happens to be there on a narrow
> > range of PCs.
> > 
> > > And I replied:
> > >> I don't think that would mean the same thing: a single-element loc
> > >> list describes a variable whose value is known only for that one
> > >> range, where a block form location expression describes a variable
> > >> that can always be found at the given location.
> > 
> > > Hmmm, it looks like add_AT_location_description() is already going to
> > > convert single-element loc lists into a simple block-form location
> > > expression. In the case of a single-element location list where the
> > > range is constrained, is that really the right thing to do?
> > 
> > If there aren't any other entries, we could probably assume they're
> > equivalent, i.e., that the variable is set when we enter its scope, and
> > remains live till the end.  This may not always hold, though: if the
> > variable is set later in the scope, we lose.
> > 
> > Nevertheless, I've (partially) restored this functionality in the patch
> > below.  It retains var_loc_notes with NULL locs until the time of
> > converting lists to the internal dwarf representation, and then it only
> > applies the optimization if there's really no more than a single entry
> > in the incoming location list.
> > 
> > I've also fixed an assertion check I ran into on
> > x86_64-linux-gnu/libgcc: a VOIDmode CONST_DOUBLE failed the mode ==
> > GET_MODE newly-introduced assertion, so I realized it to accept incoming
> > non-VOID modes with VOIDmode consts.
> > 
> > This one has passed bootstrap compare on x86_64-linux-gnu already,
> > building target libs now.
> > 
> 
> > for  gcc/ChangeLog
> > from  Alexandre Oliva  <aoliva@redhat.com>
> > 
> > 	PR debug/41473
> > 	* dwarf2out.c (add_var_loc_to_decl): Don't drop initial empty
> > 	locations.
> > 	(new_loc_list): Drop gensym arg.  Move generation of ll_symbol...
> > 	(gen_llsym): ... here.  New function.
> > 	(add_loc_descr_to_loc_list): Removed.
> > 	(loc_descriptor): Infer mode from CONST_DOUBLEs and CONST_VECTORs.
> > 	(single_element_loc_list): Removed.
> > 	(dw_loc_list): Don't create entries without a location.  Don't
> > 	special-case the first node of the list, only single nodes.
> > 	(single_element_loc_list_p): Simplify.
> > 	(loc_list_from_tree): Don't use DECL_RTL if loc_list is nonempty.
> > 	(add_location_or_const_value_attribute): Test var loc for NULL.
> > 	(convert_cfa_to_fb_loc_list): Adjust calls to new new_loc_list,
> > 	call gen_llsym if needed.
> > 
> > Index: gcc/dwarf2out.c
> > ===================================================================
> > --- gcc/dwarf2out.c.orig	2009-11-23 18:12:26.000000000 -0200
> > +++ gcc/dwarf2out.c	2009-11-23 23:41:40.000000000 -0200
> > @@ -6172,10 +6172,7 @@ static void gen_generic_params_dies (tre
> >  static void splice_child_die (dw_die_ref, dw_die_ref);
> >  static int file_info_cmp (const void *, const void *);
> >  static dw_loc_list_ref new_loc_list (dw_loc_descr_ref, const char *,
> > -				     const char *, const char *, unsigned);
> > -static void add_loc_descr_to_loc_list (dw_loc_list_ref *, dw_loc_descr_ref,
> > -				       const char *, const char *,
> > -				       const char *);
> > +				     const char *, const char *);
> >  static void output_loc_list (dw_loc_list_ref);
> >  static char *gen_internal_sym (const char *);
> >  
> > @@ -7788,8 +7785,7 @@ add_var_loc_to_decl (tree decl, struct v
> >  	  temp->last = loc;
> >  	}
> >      }
> > -  /* Do not add empty location to the beginning of the list.  */
> > -  else if (NOTE_VAR_LOCATION_LOC (loc->var_loc_note) != NULL_RTX)
> > +  else
> >      {
> >        temp->first = loc;
> >        temp->last = loc;
> > @@ -10297,12 +10293,11 @@ output_die_symbol (dw_die_ref die)
> >  }
> >  
> >  /* Return a new location list, given the begin and end range, and the
> > -   expression. gensym tells us whether to generate a new internal symbol for
> > -   this location list node, which is done for the head of the list only.  */
> > +   expression.  */
> >  
> >  static inline dw_loc_list_ref
> >  new_loc_list (dw_loc_descr_ref expr, const char *begin, const char *end,
> > -	      const char *section, unsigned int gensym)
> > +	      const char *section)
> >  {
> >    dw_loc_list_ref retlist = GGC_CNEW (dw_loc_list_node);
> >  
> > @@ -10310,27 +10305,18 @@ new_loc_list (dw_loc_descr_ref expr, con
> >    retlist->end = end;
> >    retlist->expr = expr;
> >    retlist->section = section;
> > -  if (gensym)
> > -    retlist->ll_symbol = gen_internal_sym ("LLST");
> >  
> >    return retlist;
> >  }
> >  
> > -/* Add a location description expression to a location list.  */
> > +/* Generate a new internal symbol for this location list node, if it
> > +   hasn't got one yet.  */
> >  
> >  static inline void
> > -add_loc_descr_to_loc_list (dw_loc_list_ref *list_head, dw_loc_descr_ref descr,
> > -			   const char *begin, const char *end,
> > -			   const char *section)
> > +gen_llsym (dw_loc_list_ref list)
> >  {
> > -  dw_loc_list_ref *d;
> > -
> > -  /* Find the end of the chain.  */
> > -  for (d = list_head; (*d) != NULL; d = &(*d)->dw_loc_next)
> > -    ;
> > -
> > -  /* Add a new location list node to the list.  */
> > -  *d = new_loc_list (descr, begin, end, section, 0);
> > +  gcc_assert (!list->ll_symbol);
> > +  list->ll_symbol = gen_internal_sym ("LLST");
> >  }
> >  
> >  /* Output the location list given to us.  */
> > @@ -13650,15 +13636,17 @@ loc_descriptor (rtx rtl, enum machine_mo
> >        break;
> >  
> >      case CONST_DOUBLE:
> > +      if (mode == VOIDmode)
> > +	mode = GET_MODE (rtl);
> > +
> >        if (mode != VOIDmode && (dwarf_version >= 4 || !dwarf_strict))
> >  	{
> > +	  gcc_assert (mode == GET_MODE (rtl) || VOIDmode == GET_MODE (rtl));
> > +
> >  	  /* Note that a CONST_DOUBLE rtx could represent either an integer
> >  	     or a floating-point constant.  A CONST_DOUBLE is used whenever
> >  	     the constant requires more than one word in order to be
> >  	     adequately represented.  We output CONST_DOUBLEs as blocks.  */
> > -	  if (GET_MODE (rtl) != VOIDmode)
> > -	    mode = GET_MODE (rtl);
> > -
> >  	  loc_result = new_loc_descr (DW_OP_implicit_value,
> >  				      GET_MODE_SIZE (mode), 0);
> >  	  if (SCALAR_FLOAT_MODE_P (mode))
> > @@ -13684,6 +13672,9 @@ loc_descriptor (rtx rtl, enum machine_mo
> >        break;
> >  
> >      case CONST_VECTOR:
> > +      if (mode == VOIDmode)
> > +	mode = GET_MODE (rtl);
> > +
> >        if (mode != VOIDmode && (dwarf_version >= 4 || !dwarf_strict))
> >  	{
> >  	  unsigned int elt_size = GET_MODE_UNIT_SIZE (GET_MODE (rtl));
> > @@ -13692,7 +13683,7 @@ loc_descriptor (rtx rtl, enum machine_mo
> >  	  unsigned int i;
> >  	  unsigned char *p;
> >  
> > -	  mode = GET_MODE (rtl);
> > +	  gcc_assert (mode == GET_MODE (rtl) || VOIDmode == GET_MODE (rtl));
> >  	  switch (GET_MODE_CLASS (mode))
> >  	    {
> >  	    case MODE_VECTOR_INT:
> > @@ -13839,14 +13830,6 @@ decl_by_reference_p (tree decl)
> >  	  && DECL_BY_REFERENCE (decl));
> >  }
> >  
> > -/* Return single element location list containing loc descr REF.  */
> > -
> > -static dw_loc_list_ref
> > -single_element_loc_list (dw_loc_descr_ref ref)
> > -{
> > -  return new_loc_list (ref, NULL, NULL, NULL, 0);
> > -}
> > -
> >  /* Helper function for dw_loc_list.  Compute proper Dwarf location descriptor
> >     for VARLOC.  */
> >  
> > @@ -13928,20 +13911,21 @@ dw_loc_list_1 (tree loc, rtx varloc, int
> >    return descr;
> >  }
> >  
> > -/* Return dwarf representation of location list representing for
> > -   LOC_LIST of DECL.  WANT_ADDRESS has the same meaning as in
> > -   loc_list_from_tree function.  */
> > +/* Return the dwarf representation of the location list LOC_LIST of
> > +   DECL.  WANT_ADDRESS has the same meaning as in loc_list_from_tree
> > +   function.  */
> >  
> >  static dw_loc_list_ref
> > -dw_loc_list (var_loc_list * loc_list, tree decl, int want_address)
> > +dw_loc_list (var_loc_list *loc_list, tree decl, int want_address)
> >  {
> >    const char *endname, *secname;
> > -  dw_loc_list_ref list;
> >    rtx varloc;
> >    enum var_init_status initialized;
> >    struct var_loc_node *node;
> >    dw_loc_descr_ref descr;
> >    char label_id[MAX_ARTIFICIAL_LABEL_BYTES];
> > +  dw_loc_list_ref list = NULL;
> > +  dw_loc_list_ref *listp = &list;
> >  
> >    /* Now that we know what section we are using for a base,
> >       actually construct the list of locations.
> > @@ -13954,26 +13938,9 @@ dw_loc_list (var_loc_list * loc_list, tr
> >       This means we have to special case the last node, and generate
> >       a range of [last location start, end of function label].  */
> >  
> > -  node = loc_list->first;
> >    secname = secname_for_decl (decl);
> >  
> > -  if (NOTE_VAR_LOCATION_LOC (node->var_loc_note))
> > -    initialized = NOTE_VAR_LOCATION_STATUS (node->var_loc_note);
> > -  else
> > -    initialized = VAR_INIT_STATUS_INITIALIZED;
> > -  varloc = NOTE_VAR_LOCATION (node->var_loc_note);
> > -  descr = dw_loc_list_1 (decl, varloc, want_address, initialized);
> > -
> > -  if (loc_list && loc_list->first != loc_list->last)
> > -    list = new_loc_list (descr, node->label, node->next->label, secname, 1);
> > -  else
> > -    return single_element_loc_list (descr);
> > -  node = node->next;
> > -
> > -  if (!node)
> > -    return NULL;
> > -
> > -  for (; node->next; node = node->next)
> > +  for (node = loc_list->first; node->next; node = node->next)
> >      if (NOTE_VAR_LOCATION_LOC (node->var_loc_note) != NULL_RTX)
> >        {
> >  	/* The variable has a location between NODE->LABEL and
> > @@ -13981,28 +13948,46 @@ dw_loc_list (var_loc_list * loc_list, tr
> >  	initialized = NOTE_VAR_LOCATION_STATUS (node->var_loc_note);
> >  	varloc = NOTE_VAR_LOCATION (node->var_loc_note);
> >  	descr = dw_loc_list_1 (decl, varloc, want_address, initialized);
> > -	add_loc_descr_to_loc_list (&list, descr,
> > -				   node->label, node->next->label, secname);
> > +	if (descr)
> > +	  {
> > +	    *listp = new_loc_list (descr, node->label, node->next->label,
> > +				   secname);
> > +	    listp = &(*listp)->dw_loc_next;
> > +	  }
> >        }
> >  
> >    /* If the variable has a location at the last label
> >       it keeps its location until the end of function.  */
> >    if (NOTE_VAR_LOCATION_LOC (node->var_loc_note) != NULL_RTX)
> >      {
> > -      if (!current_function_decl)
> > -	endname = text_end_label;
> > -      else
> > -	{
> > -	  ASM_GENERATE_INTERNAL_LABEL (label_id, FUNC_END_LABEL,
> > -				       current_function_funcdef_no);
> > -	  endname = ggc_strdup (label_id);
> > -	}
> > -
> >        initialized = NOTE_VAR_LOCATION_STATUS (node->var_loc_note);
> >        varloc = NOTE_VAR_LOCATION (node->var_loc_note);
> >        descr = dw_loc_list_1 (decl, varloc, want_address, initialized);
> > -      add_loc_descr_to_loc_list (&list, descr, node->label, endname, secname);
> > +      if (descr)
> > +	{
> > +	  if (!current_function_decl)
> > +	    endname = text_end_label;
> > +	  else
> > +	    {
> > +	      ASM_GENERATE_INTERNAL_LABEL (label_id, FUNC_END_LABEL,
> > +					   current_function_funcdef_no);
> > +	      endname = ggc_strdup (label_id);
> > +	    }
> > +
> > +	  *listp = new_loc_list (descr, node->label, endname, secname);
> > +	  listp = &(*listp)->dw_loc_next;
> > +	}
> >      }
> > +
> > +  /* Try to avoid the overhead of a location list emitting a location
> > +     expression instead, but only if we didn't have more than one
> > +     location entry in the first place.  If some entries were not
> > +     representable, we don't want to pretend a single entry that was
> > +     applies to the entire scope in which the variable is
> > +     available.  */
> > +  if (list && loc_list->first->next)
> > +    gen_llsym (list);
> > +
> >    return list;
> >  }
> >  
> > @@ -14012,7 +13997,8 @@ dw_loc_list (var_loc_list * loc_list, tr
> >  static bool
> >  single_element_loc_list_p (dw_loc_list_ref list)
> >  {
> > -  return (!list->dw_loc_next && !list->begin && !list->end);
> > +  gcc_assert (!list->dw_loc_next || list->ll_symbol);
> > +  return !list->ll_symbol;
> >  }
> >  
> >  /* To each location in list LIST add loc descr REF.  */
> > @@ -14312,9 +14298,9 @@ loc_list_from_tree (tree loc, int want_a
> >  	rtx rtl;
> >  	var_loc_list *loc_list = lookup_decl_loc (loc);
> >  
> > -	if (loc_list && loc_list->first
> > -	    && (list_ret = dw_loc_list (loc_list, loc, want_address)))
> > +	if (loc_list && loc_list->first)
> >  	  {
> > +	    list_ret = dw_loc_list (loc_list, loc, want_address);
> >  	    have_address = want_address != 0;
> >  	    break;
> >  	  }
> > @@ -14725,7 +14711,7 @@ loc_list_from_tree (tree loc, int want_a
> >  	add_loc_descr_to_each (list_ret, new_loc_descr (op, size, 0));
> >      }
> >    if (ret)
> > -    list_ret = single_element_loc_list (ret);
> > +    list_ret = new_loc_list (ret, NULL, NULL, NULL);
> >  
> >    return list_ret;
> >  }
> > @@ -15719,17 +15705,19 @@ add_location_or_const_value_attribute (d
> >       a constant value.  That way we are better to use add_const_value_attribute
> >       rather than expanding constant value equivalent.  */
> >    loc_list = lookup_decl_loc (decl);
> > -  if (loc_list && loc_list->first && loc_list->first == loc_list->last)
> > +  if (loc_list && loc_list->first && loc_list->first == loc_list->last
> > +      && loc_list->first->var_loc_note
> > +      && NOTE_VAR_LOCATION (loc_list->first->var_loc_note)
> > +      && NOTE_VAR_LOCATION_LOC (loc_list->first->var_loc_note))
> >      {
> >        enum var_init_status status;
> >        struct var_loc_node *node;
> >  
> >        node = loc_list->first;
> >        status = NOTE_VAR_LOCATION_STATUS (node->var_loc_note);
> > -      rtl = NOTE_VAR_LOCATION (node->var_loc_note);
> > -      if (GET_CODE (rtl) == VAR_LOCATION
> > -	  && GET_CODE (XEXP (rtl, 1)) != PARALLEL)
> > -	rtl = XEXP (XEXP (rtl, 1), 0);
> > +      rtl = NOTE_VAR_LOCATION_LOC (node->var_loc_note);
> > +      if (GET_CODE (rtl) != PARALLEL)
> > +	rtl = XEXP (rtl, 0);
> >        if ((CONSTANT_P (rtl) || GET_CODE (rtl) == CONST_STRING)
> >  	  && add_const_value_attribute (die, rtl))
> >  	 return true;
> > @@ -16016,8 +16004,7 @@ convert_cfa_to_fb_loc_list (HOST_WIDE_IN
> >  	if (!cfa_equal_p (&last_cfa, &next_cfa))
> >  	  {
> >  	    *list_tail = new_loc_list (build_cfa_loc (&last_cfa, offset),
> > -				       start_label, last_label, section,
> > -				       list == NULL);
> > +				       start_label, last_label, section);
> >  
> >  	    list_tail = &(*list_tail)->dw_loc_next;
> >  	    last_cfa = next_cfa;
> > @@ -16038,14 +16025,16 @@ convert_cfa_to_fb_loc_list (HOST_WIDE_IN
> >    if (!cfa_equal_p (&last_cfa, &next_cfa))
> >      {
> >        *list_tail = new_loc_list (build_cfa_loc (&last_cfa, offset),
> > -				 start_label, last_label, section,
> > -				 list == NULL);
> > +				 start_label, last_label, section);
> >        list_tail = &(*list_tail)->dw_loc_next;
> >        start_label = last_label;
> >      }
> > +
> >    *list_tail = new_loc_list (build_cfa_loc (&next_cfa, offset),
> > -			     start_label, fde->dw_fde_end, section,
> > -			     list == NULL);
> > +			     start_label, fde->dw_fde_end, section);
> > +
> > +  if (list && list->dw_loc_next)
> > +    gen_llsym (list);
> >  
> >    return list;
> >  }
> 
> > 
> > -- 
> > Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> > You must be the change you wish to see in the world. -- Gandhi
> > Be Free! -- http://FSFLA.org/   FSF Latin America board member
> > Free Software Evangelist      Red Hat Brazil Compiler Engineer

PINGx2 Pt2: [VTA, PR41473] drop NULL locations from list

[Jack Howarth]

   Can someone please review and commit this patch so
that darwin can retain full debug code in FSF gcc?
      Thanks in advance.
               Jack

On Tue, Dec 01, 2009 at 03:16:59PM -0500, Jack Howarth wrote:
> Alexandre,
>     Can we get this patch which when coupled with...
> 
> http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01329.html
> 
> completely eliminates the dsymutil crashes on darwin into
> gcc trunk? Thanks in advance.
>                           Jack
> 
> 
> On Thu, Nov 26, 2009 at 07:13:36AM -0200, Alexandre Oliva wrote:
> > On Nov 24, 2009, Jack Howarth <howarth@bromo.med.uc.edu> wrote:
> > 
> > > I am uncertain if the stock dwarfdump will provide the necessary
> > > information to find any residual zero locations.
> > 
> > That doesn't matter, I don't think I'd be able to use it in a
> > cross-compilation setting anyway.
> > 
> > On Nov 25, 2009, Jack Howarth <howarth@bromo.med.uc.edu> wrote:
> > 
> > > I've uploaded assembly files (generated with -dA) as well as the
> > > preprocessed source for all the object files in the FSF gcc trunk
> > > build with your proosed patch...
> > 
> > > http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01329.html
> > 
> > > that still emit zero AT_LOCATIONS and cause dsymutil to assert.
> > > These all seem to be of the form...
> > 
> > > .byte	0x0	# DW_AT_location
> > 
> > Thanks.  I picked up one of the testcases you posted and found out where
> > that location information comes from: it was a DWARF2-representable
> > location expression that referenced a symbol that, in the end, wasn't
> > emitted.
> > 
> > We used to simply zero out the location information.  With the patch
> > below, we'll remove attributes and location list entries that fail to
> > resolve.
> > 
> > Will you please give it a try and let me know whether any other issues
> > remain?
> > 
> 
> > for  gcc/ChangeLog
> > from  Alexandre Oliva  <aoliva@redhat.com>
> > 
> > 	PR debug/41473
> > 	* dwarf2out.c (AT_loc_list_ptr): New.
> > 	(resolve_addr): Remove unresolved attributes and loc_list entries.
> > 
> > Index: gcc/dwarf2out.c
> > ===================================================================
> > --- gcc/dwarf2out.c.orig	2009-11-26 06:40:48.000000000 -0200
> > +++ gcc/dwarf2out.c	2009-11-26 07:04:41.000000000 -0200
> > @@ -7206,6 +7206,13 @@ AT_loc_list (dw_attr_ref a)
> >    return a->dw_attr_val.v.val_loc_list;
> >  }
> >  
> > +static inline dw_loc_list_ref *
> > +AT_loc_list_ptr (dw_attr_ref a)
> > +{
> > +  gcc_assert (a && AT_class (a) == dw_val_class_loc_list);
> > +  return &a->dw_attr_val.v.val_loc_list;
> > +}
> > +
> >  /* Add an address constant attribute value to a DIE.  */
> >  
> >  static inline void
> > @@ -20961,28 +20968,48 @@ resolve_addr (dw_die_ref die)
> >  {
> >    dw_die_ref c;
> >    dw_attr_ref a;
> > -  dw_loc_list_ref curr;
> > +  dw_loc_list_ref *curr;
> >    unsigned ix;
> >  
> >    for (ix = 0; VEC_iterate (dw_attr_node, die->die_attr, ix, a); ix++)
> >      switch (AT_class (a))
> >        {
> >        case dw_val_class_loc_list:
> > -	for (curr = AT_loc_list (a); curr != NULL; curr = curr->dw_loc_next)
> > -	  if (!resolve_addr_in_expr (curr->expr))
> > -	    curr->expr = NULL;
> > +	curr = AT_loc_list_ptr (a);
> > +	while (*curr)
> > +	  {
> > +	    if (!resolve_addr_in_expr ((*curr)->expr))
> > +	      {
> > +		dw_loc_list_ref next = (*curr)->dw_loc_next;
> > +		if (next && (*curr)->ll_symbol)
> > +		  {
> > +		    gcc_assert (!next->ll_symbol);
> > +		    next->ll_symbol = (*curr)->ll_symbol;
> > +		  }
> > +		*curr = next;
> > +	      }
> > +	    else
> > +	      curr = &(*curr)->dw_loc_next;
> > +	  }
> > +	if (!AT_loc_list (a))
> > +	  {
> > +	    remove_AT (die, a->dw_attr);
> > +	    ix--;
> > +	  }
> >  	break;
> >        case dw_val_class_loc:
> >  	if (!resolve_addr_in_expr (AT_loc (a)))
> > -	  a->dw_attr_val.v.val_loc = NULL;
> > +	  {
> > +	    remove_AT (die, a->dw_attr);
> > +	    ix--;
> > +	  }
> >  	break;
> >        case dw_val_class_addr:
> >  	if (a->dw_attr == DW_AT_const_value
> >  	    && resolve_one_addr (&a->dw_attr_val.v.val_addr, NULL))
> >  	  {
> > -	    a->dw_attr = DW_AT_location;
> > -	    a->dw_attr_val.val_class = dw_val_class_loc;
> > -	    a->dw_attr_val.v.val_loc = NULL;
> > +	    remove_AT (die, a->dw_attr);
> > +	    ix--;
> >  	  }
> >  	break;
> >        default:
> 
> > 
> > -- 
> > Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> > You must be the change you wish to see in the world. -- Gandhi
> > Be Free! -- http://FSFLA.org/   FSF Latin America board member
> > Free Software Evangelist      Red Hat Brazil Compiler Engineer

Re: [VTA, PR41473] drop NULL locations from lists

[Jason Merrill]

On 11/23/2009 09:39 PM, Alexandre Oliva wrote:
> 	PR debug/41473
> 	* dwarf2out.c (add_var_loc_to_decl): Don't drop initial empty
> 	locations.
> 	(new_loc_list): Drop gensym arg.  Move generation of ll_symbol...
> 	(gen_llsym): ... here.  New function.
> 	(add_loc_descr_to_loc_list): Removed.
> 	(loc_descriptor): Infer mode from CONST_DOUBLEs and CONST_VECTORs.
> 	(single_element_loc_list): Removed.
> 	(dw_loc_list): Don't create entries without a location.  Don't
> 	special-case the first node of the list, only single nodes.
> 	(single_element_loc_list_p): Simplify.
> 	(loc_list_from_tree): Don't use DECL_RTL if loc_list is nonempty.
> 	(add_location_or_const_value_attribute): Test var loc for NULL.
> 	(convert_cfa_to_fb_loc_list): Adjust calls to new new_loc_list,
> 	call gen_llsym if needed.

OK.

> 	PR debug/41473
> 	* dwarf2out.c (AT_loc_list_ptr): New.
> 	(resolve_addr): Remove unresolved attributes and loc_list entries.

OK.

Jason

Re: [VTA, PR41473] drop NULL locations from lists

[Jason Merrill]

On 11/23/2009 09:39 PM, Alexandre Oliva wrote:
> 	PR debug/41473
> 	* dwarf2out.c (add_var_loc_to_decl): Don't drop initial empty
> 	locations.
> 	(new_loc_list): Drop gensym arg.  Move generation of ll_symbol...
> 	(gen_llsym): ... here.  New function.
> 	(add_loc_descr_to_loc_list): Removed.
> 	(loc_descriptor): Infer mode from CONST_DOUBLEs and CONST_VECTORs.
> 	(single_element_loc_list): Removed.
> 	(dw_loc_list): Don't create entries without a location.  Don't
> 	special-case the first node of the list, only single nodes.
> 	(single_element_loc_list_p): Simplify.
> 	(loc_list_from_tree): Don't use DECL_RTL if loc_list is nonempty.
> 	(add_location_or_const_value_attribute): Test var loc for NULL.
> 	(convert_cfa_to_fb_loc_list): Adjust calls to new new_loc_list,
> 	call gen_llsym if needed.

OK.

> 	PR debug/41473
> 	* dwarf2out.c (AT_loc_list_ptr): New.
> 	(resolve_addr): Remove unresolved attributes and loc_list entries.

OK.

Jason