public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [patch] Defer symbol addition until construction is complete
@ 2010-07-19 15:28 sami wagiaalla
  2010-07-29 20:58 ` Tom Tromey
  0 siblings, 1 reply; 10+ messages in thread
From: sami wagiaalla @ 2010-07-19 15:28 UTC (permalink / raw)
  To: gdb-patches

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

In new_symbol adding the symbol to the pending symbol list is performed 
at various times and it is added to various lists making uniform 
treatment of of symbols at addition point difficult. This patch unifies 
the addition point and defers it to the end of construction.

Tested by running the testsuit on Fedora 13 with gcc 4.4.4 on x8664.




[-- Attachment #2: defer_symbol_addtion.patch --]
[-- Type: text/plain, Size: 6102 bytes --]

Complete symbol construction before adding to pending list.

2010-07-13  Sami Wagiaalla  <swagiaal@redhat.com>

	* dwarf2read.c (new_symbol): Add symbol to variable list at end of
	fucntion after symbol construction is complete.

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 622331e..a8692ea 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -8651,6 +8651,8 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
   struct attribute *attr = NULL;
   struct attribute *attr2 = NULL;
   CORE_ADDR baseaddr;
+  struct pending **list_to_add = NULL;
+
   int inlined_func = (die->tag == DW_TAG_inlined_subroutine);
 
   baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
@@ -8738,11 +8740,11 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
                  access them globally.  For instance, we want to be able
                  to break on a nested subprogram without having to
                  specify the context.  */
-	      add_symbol_to_list (sym, &global_symbols);
+	      list_to_add = &global_symbols;
 	    }
 	  else
 	    {
-	      add_symbol_to_list (sym, cu->list_in_scope);
+	      list_to_add = cu->list_in_scope;
 	    }
 	  break;
 	case DW_TAG_inlined_subroutine:
@@ -8767,9 +8769,9 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
 	      dwarf2_const_value (attr, sym, cu);
 	      attr2 = dwarf2_attr (die, DW_AT_external, cu);
 	      if (attr2 && (DW_UNSND (attr2) != 0))
-		add_symbol_to_list (sym, &global_symbols);
+		list_to_add = &global_symbols;
 	      else
-		add_symbol_to_list (sym, cu->list_in_scope);
+		list_to_add = cu->list_in_scope;
 	      break;
 	    }
 	  attr = dwarf2_attr (die, DW_AT_location, cu);
@@ -8779,7 +8781,6 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
 	      attr2 = dwarf2_attr (die, DW_AT_external, cu);
 	      if (attr2 && (DW_UNSND (attr2) != 0))
 		{
-		  struct pending **list_to_add;
 
 		  /* Workaround gfortran PR debug/40040 - it uses
 		     DW_AT_location for variables in -fPIC libraries which may
@@ -8799,10 +8800,9 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
 		     but it may be block-scoped.  */
 		  list_to_add = (cu->list_in_scope == &file_symbols
 				 ? &global_symbols : cu->list_in_scope);
-		  add_symbol_to_list (sym, list_to_add);
 		}
 	      else
-		add_symbol_to_list (sym, cu->list_in_scope);
+		list_to_add = cu->list_in_scope;
 	    }
 	  else
 	    {
@@ -8816,21 +8816,18 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
 	      if (attr2 && (DW_UNSND (attr2) != 0)
 		  && dwarf2_attr (die, DW_AT_type, cu) != NULL)
 		{
-		  struct pending **list_to_add;
-
 		  /* A variable with DW_AT_external is never static, but it
 		     may be block-scoped.  */
 		  list_to_add = (cu->list_in_scope == &file_symbols
 				 ? &global_symbols : cu->list_in_scope);
 
 		  SYMBOL_CLASS (sym) = LOC_UNRESOLVED;
-		  add_symbol_to_list (sym, list_to_add);
 		}
 	      else if (!die_is_declaration (die, cu))
 		{
 		  /* Use the default LOC_OPTIMIZED_OUT class.  */
 		  gdb_assert (SYMBOL_CLASS (sym) == LOC_OPTIMIZED_OUT);
-		  add_symbol_to_list (sym, cu->list_in_scope);
+		  list_to_add = cu->list_in_scope;
 		}
 	    }
 	  break;
@@ -8862,7 +8859,7 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
 	      SYMBOL_TYPE (sym) = ref_type;
 	    }
 
-	  add_symbol_to_list (sym, cu->list_in_scope);
+	  list_to_add = cu->list_in_scope;
 	  break;
 	case DW_TAG_unspecified_parameters:
 	  /* From varargs functions; gdb doesn't seem to have any
@@ -8887,14 +8884,10 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
 	       saves you.  See the OtherFileClass tests in
 	       gdb.c++/namespace.exp.  */
 
-	    struct pending **list_to_add;
-
 	    list_to_add = (cu->list_in_scope == &file_symbols
 			   && (cu->language == language_cplus
 			       || cu->language == language_java)
 			   ? &global_symbols : cu->list_in_scope);
-	  
-	    add_symbol_to_list (sym, list_to_add);
 
 	    /* The semantics of C++ state that "struct foo { ... }" also
 	       defines a typedef for "foo".  A Java class declaration also
@@ -8914,7 +8907,7 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
 	case DW_TAG_typedef:
 	  SYMBOL_CLASS (sym) = LOC_TYPEDEF;
 	  SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
-	  add_symbol_to_list (sym, cu->list_in_scope);
+	  list_to_add = cu->list_in_scope;
 	  break;
 	case DW_TAG_base_type:
         case DW_TAG_subrange_type:
@@ -8922,7 +8915,7 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
         case DW_TAG_volatile_type:
 	  SYMBOL_CLASS (sym) = LOC_TYPEDEF;
 	  SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
-	  add_symbol_to_list (sym, cu->list_in_scope);
+	  list_to_add = cu->list_in_scope;
 	  break;
 	case DW_TAG_enumerator:
 	  attr = dwarf2_attr (die, DW_AT_const_value, cu);
@@ -8934,19 +8927,16 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
 	    /* NOTE: carlton/2003-11-10: See comment above in the
 	       DW_TAG_class_type, etc. block.  */
 
-	    struct pending **list_to_add;
-
 	    list_to_add = (cu->list_in_scope == &file_symbols
 			   && (cu->language == language_cplus
 			       || cu->language == language_java)
 			   ? &global_symbols : cu->list_in_scope);
 	  
-	    add_symbol_to_list (sym, list_to_add);
 	  }
 	  break;
 	case DW_TAG_namespace:
 	  SYMBOL_CLASS (sym) = LOC_TYPEDEF;
-	  add_symbol_to_list (sym, &global_symbols);
+	  list_to_add = &global_symbols;
 	  break;
 	default:
 	  /* Not a tag we recognize.  Hopefully we aren't processing
@@ -8958,6 +8948,9 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
 	  break;
 	}
 
+      if (list_to_add != NULL)
+	add_symbol_to_list (sym, list_to_add);
+
       /* For the benefit of old versions of GCC, check for anonymous
 	 namespaces based on the demangled name.  */
       if (!processing_has_namespace_info


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

* Re: [patch] Defer symbol addition until construction is complete
  2010-07-19 15:28 [patch] Defer symbol addition until construction is complete sami wagiaalla
@ 2010-07-29 20:58 ` Tom Tromey
  2010-08-09 18:38   ` sami wagiaalla
  0 siblings, 1 reply; 10+ messages in thread
From: Tom Tromey @ 2010-07-29 20:58 UTC (permalink / raw)
  To: sami wagiaalla; +Cc: gdb-patches

>>>>> "Sami" == sami wagiaalla <swagiaal@redhat.com> writes:

Sami> In new_symbol adding the symbol to the pending symbol list is
Sami> performed at various times and it is added to various lists making
Sami> uniform treatment of of symbols at addition point difficult. This
Sami> patch unifies the addition point and defers it to the end of
Sami> construction.

Looks quite reasonable.

Sami> 	* dwarf2read.c (new_symbol): Add symbol to variable list at end of
Sami> 	fucntion after symbol construction is complete.

Typo: "function".

Also you will need to rebase a little and s/new_symbol/new_symbol_full/
here.

Sami>  	      if (attr2 && (DW_UNSND (attr2) != 0))
Sami>  		{
Sami> -		  struct pending **list_to_add;
Sami> 
Sami>  		  /* Workaround gfortran PR debug/40040 - it uses

Remove the blank line as well.

Sami>  	    list_to_add = (cu->list_in_scope == &file_symbols
Sami>  			   && (cu->language == language_cplus
Sami>  			       || cu->language == language_java)
Sami>  			   ? &global_symbols : cu->list_in_scope);
Sami> 	  
Sami> -	    add_symbol_to_list (sym, list_to_add);

Here too.

Ok with those changes.

Tom

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

* Re: [patch] Defer symbol addition until construction is complete
  2010-07-29 20:58 ` Tom Tromey
@ 2010-08-09 18:38   ` sami wagiaalla
  2010-08-09 19:55     ` Tom Tromey
  2010-08-10  7:44     ` Regression gdb.cp/temargs.exp: test value of P in inner_m [Re: [patch] Defer symbol addition until construction is complete] Jan Kratochvil
  0 siblings, 2 replies; 10+ messages in thread
From: sami wagiaalla @ 2010-08-09 18:38 UTC (permalink / raw)
  To: gdb-patches

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


> Ok with those changes.
>

I am reposting since the rebase want trivial; I change the template 
symbol code to take advantage of this clean up.

Is this cool ?

Sami

[-- Attachment #2: defer_symbol_addtion.patch --]
[-- Type: text/plain, Size: 6842 bytes --]

Complete symbol construction before adding to pending list.

2010-08-09  Sami Wagiaalla  <swagiaal@redhat.com>

	* dwarf2read.c (new_symbol): Add symbol to variable list at end of
	function after symbol construction is complete.
	Do the same for template symbol addition to template_symbols list.

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 5275c58..e7f7d19 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -9996,6 +9996,8 @@ new_symbol_full (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
   struct attribute *attr = NULL;
   struct attribute *attr2 = NULL;
   CORE_ADDR baseaddr;
+  struct pending **list_to_add = NULL;
+
   int inlined_func = (die->tag == DW_TAG_inlined_subroutine);
 
   baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
@@ -10088,11 +10090,11 @@ new_symbol_full (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
                  access them globally.  For instance, we want to be able
                  to break on a nested subprogram without having to
                  specify the context.  */
-	      add_symbol_to_list (sym, &global_symbols);
+	      list_to_add = &global_symbols;
 	    }
 	  else
 	    {
-	      add_symbol_to_list (sym, cu->list_in_scope);
+	      list_to_add = cu->list_in_scope;
 	    }
 	  break;
 	case DW_TAG_inlined_subroutine:
@@ -10129,17 +10131,12 @@ new_symbol_full (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	    {
 	      dwarf2_const_value (attr, sym, cu);
 	      attr2 = dwarf2_attr (die, DW_AT_external, cu);
-	      if (suppress_add)
-		{
-		  sym->hash_next = objfile->template_symbols;
-		  objfile->template_symbols = sym;
-		}
-	      else
+	      if (!suppress_add)
 		{
 		  if (attr2 && (DW_UNSND (attr2) != 0))
-		    add_symbol_to_list (sym, &global_symbols);
+		    list_to_add = &global_symbols;
 		  else
-		    add_symbol_to_list (sym, cu->list_in_scope);
+		    list_to_add = cu->list_in_scope;
 		}
 	      break;
 	    }
@@ -10159,8 +10156,6 @@ new_symbol_full (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 		}
 	      else if (attr2 && (DW_UNSND (attr2) != 0))
 		{
-		  struct pending **list_to_add;
-
 		  /* Workaround gfortran PR debug/40040 - it uses
 		     DW_AT_location for variables in -fPIC libraries which may
 		     get overriden by other libraries/executable and get
@@ -10179,10 +10174,9 @@ new_symbol_full (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 		     but it may be block-scoped.  */
 		  list_to_add = (cu->list_in_scope == &file_symbols
 				 ? &global_symbols : cu->list_in_scope);
-		  add_symbol_to_list (sym, list_to_add);
 		}
 	      else
-		add_symbol_to_list (sym, cu->list_in_scope);
+		list_to_add = cu->list_in_scope;
 	    }
 	  else
 	    {
@@ -10196,33 +10190,19 @@ new_symbol_full (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	      if (attr2 && (DW_UNSND (attr2) != 0)
 		  && dwarf2_attr (die, DW_AT_type, cu) != NULL)
 		{
-		  struct pending **list_to_add;
-
 		  /* A variable with DW_AT_external is never static, but it
 		     may be block-scoped.  */
 		  list_to_add = (cu->list_in_scope == &file_symbols
 				 ? &global_symbols : cu->list_in_scope);
 
 		  SYMBOL_CLASS (sym) = LOC_UNRESOLVED;
-		  if (suppress_add)
-		    {
-		      sym->hash_next = objfile->template_symbols;
-		      objfile->template_symbols = sym;
-		    }
-		  else
-		    add_symbol_to_list (sym, list_to_add);
 		}
 	      else if (!die_is_declaration (die, cu))
 		{
 		  /* Use the default LOC_OPTIMIZED_OUT class.  */
 		  gdb_assert (SYMBOL_CLASS (sym) == LOC_OPTIMIZED_OUT);
-		  if (suppress_add)
-		    {
-		      sym->hash_next = objfile->template_symbols;
-		      objfile->template_symbols = sym;
-		    }
-		  else
-		    add_symbol_to_list (sym, cu->list_in_scope);
+		  if (!suppress_add)
+		    list_to_add = cu->list_in_scope;
 		}
 	    }
 	  break;
@@ -10254,7 +10234,7 @@ new_symbol_full (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	      SYMBOL_TYPE (sym) = ref_type;
 	    }
 
-	  add_symbol_to_list (sym, cu->list_in_scope);
+	  list_to_add = cu->list_in_scope;
 	  break;
 	case DW_TAG_unspecified_parameters:
 	  /* From varargs functions; gdb doesn't seem to have any
@@ -10282,21 +10262,12 @@ new_symbol_full (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	       saves you.  See the OtherFileClass tests in
 	       gdb.c++/namespace.exp.  */
 
-	    if (suppress_add)
+	    if (!suppress_add)
 	      {
-		sym->hash_next = objfile->template_symbols;
-		objfile->template_symbols = sym;
-	      }
-	    else
-	      {
-		struct pending **list_to_add;
-
 		list_to_add = (cu->list_in_scope == &file_symbols
 			       && (cu->language == language_cplus
 				   || cu->language == language_java)
 			       ? &global_symbols : cu->list_in_scope);
-
-		add_symbol_to_list (sym, list_to_add);
 	      }
 
 	    /* The semantics of C++ state that "struct foo { ... }" also
@@ -10317,13 +10288,13 @@ new_symbol_full (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	case DW_TAG_typedef:
 	  SYMBOL_CLASS (sym) = LOC_TYPEDEF;
 	  SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
-	  add_symbol_to_list (sym, cu->list_in_scope);
+	  list_to_add = cu->list_in_scope;
 	  break;
 	case DW_TAG_base_type:
         case DW_TAG_subrange_type:
 	  SYMBOL_CLASS (sym) = LOC_TYPEDEF;
 	  SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
-	  add_symbol_to_list (sym, cu->list_in_scope);
+	  list_to_add = cu->list_in_scope;
 	  break;
 	case DW_TAG_enumerator:
 	  attr = dwarf2_attr (die, DW_AT_const_value, cu);
@@ -10335,19 +10306,15 @@ new_symbol_full (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	    /* NOTE: carlton/2003-11-10: See comment above in the
 	       DW_TAG_class_type, etc. block.  */
 
-	    struct pending **list_to_add;
-
 	    list_to_add = (cu->list_in_scope == &file_symbols
 			   && (cu->language == language_cplus
 			       || cu->language == language_java)
 			   ? &global_symbols : cu->list_in_scope);
-
-	    add_symbol_to_list (sym, list_to_add);
 	  }
 	  break;
 	case DW_TAG_namespace:
 	  SYMBOL_CLASS (sym) = LOC_TYPEDEF;
-	  add_symbol_to_list (sym, &global_symbols);
+	  list_to_add = &global_symbols;
 	  break;
 	default:
 	  /* Not a tag we recognize.  Hopefully we aren't processing
@@ -10359,6 +10326,16 @@ new_symbol_full (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	  break;
 	}
 
+      if (suppress_add)
+	{
+	  sym->hash_next = objfile->template_symbols;
+	  objfile->template_symbols = sym;
+	  list_to_add = NULL;
+	}
+
+      if (list_to_add != NULL)
+	add_symbol_to_list (sym, list_to_add);
+
       /* For the benefit of old versions of GCC, check for anonymous
 	 namespaces based on the demangled name.  */
       if (!processing_has_namespace_info

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

* Re: [patch] Defer symbol addition until construction is complete
  2010-08-09 18:38   ` sami wagiaalla
@ 2010-08-09 19:55     ` Tom Tromey
  2010-08-10  7:44     ` Regression gdb.cp/temargs.exp: test value of P in inner_m [Re: [patch] Defer symbol addition until construction is complete] Jan Kratochvil
  1 sibling, 0 replies; 10+ messages in thread
From: Tom Tromey @ 2010-08-09 19:55 UTC (permalink / raw)
  To: sami wagiaalla; +Cc: gdb-patches

>>>>> "Sami" == sami wagiaalla <swagiaal@redhat.com> writes:

Sami> I am reposting since the rebase want trivial; I change the template
Sami> symbol code to take advantage of this clean up.

Sami> Is this cool ?

Yes, thanks.

Tom

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

* Regression gdb.cp/temargs.exp: test value of P in inner_m  [Re: [patch] Defer symbol addition until construction is complete]
  2010-08-09 18:38   ` sami wagiaalla
  2010-08-09 19:55     ` Tom Tromey
@ 2010-08-10  7:44     ` Jan Kratochvil
  2010-08-10 14:34       ` sami wagiaalla
  2010-08-31 18:37       ` Tom Tromey
  1 sibling, 2 replies; 10+ messages in thread
From: Jan Kratochvil @ 2010-08-10  7:44 UTC (permalink / raw)
  To: sami wagiaalla; +Cc: gdb-patches, Tom Tromey

On Mon, 09 Aug 2010 20:38:26 +0200, sami wagiaalla wrote:
> Complete symbol construction before adding to pending list.
> 
> 2010-08-09  Sami Wagiaalla  <swagiaal@redhat.com>
> 
> 	* dwarf2read.c (new_symbol): Add symbol to variable list at end of
> 	function after symbol construction is complete.
> 	Do the same for template symbol addition to template_symbols list.

checked-in as:
http://sourceware.org/ml/gdb-cvs/2010-08/msg00040.html
3b26cb4b133031280fa022e2a06e58f333ed5e8d

> Is this cool ?

This patch has a cold regression. :-)

-PASS: gdb.cp/temargs.exp: test value of P in inner_m
+FAIL: gdb.cp/temargs.exp: test value of P in inner_m
:
print P == &a_global
$4 = true
(gdb) PASS: gdb.cp/temargs.exp: test value of P in inner_m
->
print P == &a_global
No symbol "P" in current context.
(gdb) FAIL: gdb.cp/temargs.exp: test value of P in inner_m

It affects {x86_64,x86_64-m32,i686}-fedora14snapshot-linux-gnu.
It does not affect {x86_64,x86_64-m32,i686}-fedora13-linux-gnu.

PASS->FAIL gcc-4.5.0-3.fc14.{x86_64,i686}
XFAIL      gcc-4.4.4-10.fc13.{x86_64,i686}
PASS->FAIL GNU C++ 4.6.0 20100810 (experimental)
PASS->FAIL GNU C++ 4.5.2 20100810 (prerelease)
XFAIL      GNU C++ 4.4.5 20100810 (prerelease)

XFAIL is:
print P == &a_global
No symbol "P" in current context.
(gdb) XFAIL: gdb.cp/temargs.exp: test value of P in inner_m


Thanks,
Jan

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

* Re: Regression gdb.cp/temargs.exp: test value of P in inner_m  [Re: [patch] Defer symbol addition until construction is complete]
  2010-08-10  7:44     ` Regression gdb.cp/temargs.exp: test value of P in inner_m [Re: [patch] Defer symbol addition until construction is complete] Jan Kratochvil
@ 2010-08-10 14:34       ` sami wagiaalla
  2010-08-20 20:21         ` sami wagiaalla
  2010-08-31 18:37       ` Tom Tromey
  1 sibling, 1 reply; 10+ messages in thread
From: sami wagiaalla @ 2010-08-10 14:34 UTC (permalink / raw)
  To: gdb-patches

On 08/10/2010 03:44 AM, Jan Kratochvil wrote:
> On Mon, 09 Aug 2010 20:38:26 +0200, sami wagiaalla wrote:
>> Complete symbol construction before adding to pending list.
>>
>> 2010-08-09  Sami Wagiaalla<swagiaal@redhat.com>
>>
>> 	* dwarf2read.c (new_symbol): Add symbol to variable list at end of
>> 	function after symbol construction is complete.
>> 	Do the same for template symbol addition to template_symbols list.
>
> checked-in as:
> http://sourceware.org/ml/gdb-cvs/2010-08/msg00040.html
> 3b26cb4b133031280fa022e2a06e58f333ed5e8d
>
>> Is this cool ?
>
> This patch has a cold regression. :-)
>
> -PASS: gdb.cp/temargs.exp: test value of P in inner_m
> +FAIL: gdb.cp/temargs.exp: test value of P in inner_m
> :

Okay let me take a look. Thanks for letting me know.

Sami

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

* Re: Regression gdb.cp/temargs.exp: test value of P in inner_m  [Re: [patch] Defer symbol addition until construction is complete]
  2010-08-10 14:34       ` sami wagiaalla
@ 2010-08-20 20:21         ` sami wagiaalla
  2010-08-20 21:54           ` Tom Tromey
  0 siblings, 1 reply; 10+ messages in thread
From: sami wagiaalla @ 2010-08-20 20:21 UTC (permalink / raw)
  To: gdb-patches

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


>> -PASS: gdb.cp/temargs.exp: test value of P in inner_m
>> +FAIL: gdb.cp/temargs.exp: test value of P in inner_m
>> :
>
> Okay let me take a look. Thanks for letting me know.
>

Apologies it took so long to get back to this I had some difficulty 
setting up the environment.

The original patch:

http://sourceware.org/ml/gdb-cvs/2010-08/msg00040.html
3b26cb4b133031280fa022e2a06e58f333ed5e8d

suppressed one too many regressions. Specifically (if you look at 
3b26cb4b133^) the regression on line 10185 when the tag is 
DW_TAG_template_value_param and DW_AT_location != NULL.

Patch attached.

[-- Attachment #2: regression.patch --]
[-- Type: text/x-patch, Size: 548 bytes --]

Fix defer add regression.

2010-08-20  Sami Wagiaalla  <swagiaal@redhat.com>

	* dwarf2read.c (new_symbol_full): Do not set list_to_add to NULL
	if suppress_add == 1.

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 78491c8..cd1c084 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -10330,7 +10330,6 @@ new_symbol_full (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	{
 	  sym->hash_next = objfile->template_symbols;
 	  objfile->template_symbols = sym;
-	  list_to_add = NULL;
 	}
 
       if (list_to_add != NULL)

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

* Re: Regression gdb.cp/temargs.exp: test value of P in inner_m  [Re: [patch] Defer symbol addition until construction is complete]
  2010-08-20 20:21         ` sami wagiaalla
@ 2010-08-20 21:54           ` Tom Tromey
  2010-08-31 15:18             ` sami wagiaalla
  0 siblings, 1 reply; 10+ messages in thread
From: Tom Tromey @ 2010-08-20 21:54 UTC (permalink / raw)
  To: sami wagiaalla; +Cc: gdb-patches

>>>>> "Sami" == sami wagiaalla <swagiaal@redhat.com> writes:

Sami> suppressed one too many regressions. Specifically (if you look at
Sami> 3b26cb4b133^) the regression on line 10185 when the tag is
Sami> DW_TAG_template_value_param and DW_AT_location != NULL.

I don't think this patch is correct.
IIUC it will go ahead and add the template parameters to whatever scope
is being read.  That isn't right, and in fact that is what suppress_add
was intended to prevent.

I see that I missed some case in the suppress_add patch.
But that is an oversight -- I think the code in dwarf2read.c as it is
now looks ok.

Tom

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

* Re: Regression gdb.cp/temargs.exp: test value of P in inner_m  [Re: [patch] Defer symbol addition until construction is complete]
  2010-08-20 21:54           ` Tom Tromey
@ 2010-08-31 15:18             ` sami wagiaalla
  0 siblings, 0 replies; 10+ messages in thread
From: sami wagiaalla @ 2010-08-31 15:18 UTC (permalink / raw)
  To: gdb-patches

On 08/20/2010 05:54 PM, Tom Tromey wrote:
>>>>>> "Sami" == sami wagiaalla<swagiaal@redhat.com>  writes:
>
> Sami>  suppressed one too many regressions. Specifically (if you look at
> Sami>  3b26cb4b133^) the regression on line 10185 when the tag is
> Sami>  DW_TAG_template_value_param and DW_AT_location != NULL.
>
> I don't think this patch is correct.
> IIUC it will go ahead and add the template parameters to whatever scope
> is being read.  That isn't right, and in fact that is what suppress_add
> was intended to prevent.

I looked into this some more. Here is the problem IIUC:

The template parameter P in this case is a member of the structure not 
the function, but, from what I can tell, the template param patch 
(240d54e9) supports storing and searching of template parameters only if 
they are members of a function. If that is correct then this patch just 
exposed that problem as opposed to case the regression.

Sami

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

* Re: Regression gdb.cp/temargs.exp: test value of P in inner_m  [Re: [patch] Defer symbol addition until construction is complete]
  2010-08-10  7:44     ` Regression gdb.cp/temargs.exp: test value of P in inner_m [Re: [patch] Defer symbol addition until construction is complete] Jan Kratochvil
  2010-08-10 14:34       ` sami wagiaalla
@ 2010-08-31 18:37       ` Tom Tromey
  1 sibling, 0 replies; 10+ messages in thread
From: Tom Tromey @ 2010-08-31 18:37 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: sami wagiaalla, gdb-patches

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> This patch has a cold regression. :-)
Jan> -PASS: gdb.cp/temargs.exp: test value of P in inner_m
Jan> +FAIL: gdb.cp/temargs.exp: test value of P in inner_m

I debugged this today.  I believe the problem is a g++ regression -- it
is not reporting all the template parameters.

I updated the relevant bug with my findings:

    http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41736

Sami> I looked into this some more. Here is the problem IIUC:

Sami> The template parameter P in this case is a member of the structure not
Sami> the function, but, from what I can tell, the template param patch
Sami> (240d54e9) supports storing and searching of template parameters only
Sami> if they are members of a function. If that is correct then this patch
Sami> just exposed that problem as opposed to case the regression.

Template parameters for types are put into the type's cplus_specific
structure.  See dwarf2read.c:read_structure_type.

Tom

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

end of thread, other threads:[~2010-08-31 18:37 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-19 15:28 [patch] Defer symbol addition until construction is complete sami wagiaalla
2010-07-29 20:58 ` Tom Tromey
2010-08-09 18:38   ` sami wagiaalla
2010-08-09 19:55     ` Tom Tromey
2010-08-10  7:44     ` Regression gdb.cp/temargs.exp: test value of P in inner_m [Re: [patch] Defer symbol addition until construction is complete] Jan Kratochvil
2010-08-10 14:34       ` sami wagiaalla
2010-08-20 20:21         ` sami wagiaalla
2010-08-20 21:54           ` Tom Tromey
2010-08-31 15:18             ` sami wagiaalla
2010-08-31 18:37       ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).