public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* Re: [RFA]dwarf reader:  Avoid complaint on const type
       [not found] <41597.7287375883$1274454923@news.gmane.org>
@ 2010-05-21 17:20 ` Tom Tromey
  2010-05-21 20:46   ` Pierre Muller
  0 siblings, 1 reply; 11+ messages in thread
From: Tom Tromey @ 2010-05-21 17:20 UTC (permalink / raw)
  To: Pierre Muller; +Cc: gdb-patches

>>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr> writes:

Pierre>   GDB CVS compiled for Linux arm gave several dwarf complaints 
Pierre> when debugging itself that I tried to fix.

Thanks.

Pierre>   Is this correct?

Yes.

Pierre> Maybe there are other position where DW_TAG_const_type
Pierre> and DW_TAG_volatile_type should be added...

Not that I know of.

Pierre> 2010-05-21  Pierre Muller  <muller@ics.u-strasbg.fr>
Pierre> 	* dwarf2read.c (process_die): Also allow DW_TAG_const_type
Pierre> 	and DW_TAG_volatile_type.
Pierre> 	(new_symbol): Likewise.

This is ok.  Thanks again.

Tom

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

* RE: [RFA]dwarf reader:  Avoid complaint on const type
  2010-05-21 17:20 ` [RFA]dwarf reader: Avoid complaint on const type Tom Tromey
@ 2010-05-21 20:46   ` Pierre Muller
  2010-06-16 16:08     ` Pedro Alves
  0 siblings, 1 reply; 11+ messages in thread
From: Pierre Muller @ 2010-05-21 20:46 UTC (permalink / raw)
  To: tromey; +Cc: gdb-patches

> Pierre>   GDB CVS compiled for Linux arm gave several dwarf complaints
> Pierre> when debugging itself that I tried to fix.
> 
> Thanks.
> 
> Pierre>   Is this correct?
> 
> Yes.
> 
> Pierre> Maybe there are other position where DW_TAG_const_type
> Pierre> and DW_TAG_volatile_type should be added...
> 
> Not that I know of.
> 
> Pierre> 2010-05-21  Pierre Muller  <muller@ics.u-strasbg.fr>
> Pierre> 	* dwarf2read.c (process_die): Also allow DW_TAG_const_type
> Pierre> 	and DW_TAG_volatile_type.
> Pierre> 	(new_symbol): Likewise.
> 
> This is ok.  Thanks again.

Thanks for the approval,
patch committed.

Pierre

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

* Re: [RFA]dwarf reader:  Avoid complaint on const type
  2010-05-21 20:46   ` Pierre Muller
@ 2010-06-16 16:08     ` Pedro Alves
  2010-06-20 22:39       ` Pierre Muller
       [not found]       ` <3752333521215815628@unknownmsgid>
  0 siblings, 2 replies; 11+ messages in thread
From: Pedro Alves @ 2010-06-16 16:08 UTC (permalink / raw)
  To: gdb-patches; +Cc: Pierre Muller, tromey

On Friday 21 May 2010 21:35:37, Pierre Muller wrote:

> > Pierre> 2010-05-21  Pierre Muller  <muller@ics.u-strasbg.fr>
> > Pierre> 	* dwarf2read.c (process_die): Also allow DW_TAG_const_type
> > Pierre> 	and DW_TAG_volatile_type.
> > Pierre> 	(new_symbol): Likewise.

This caused

 @@ -7385,16 +7385,16 @@ PASS: gdb.base/sigaltstack.exp: handle S
  PASS: gdb.base/sigaltstack.exp: handle SIGVTALRM print pass nostop
  PASS: gdb.base/sigaltstack.exp: handle SIGPROF print pass nostop
  PASS: gdb.base/sigaltstack.exp: break catcher if level == INNER
 -PASS: gdb.base/sigaltstack.exp: continue to catch
 +FAIL: gdb.base/sigaltstack.exp: continue to catch (the program exited)
  PASS: gdb.base/sigaltstack.exp: next
 -PASS: gdb.base/sigaltstack.exp: backtrace
 -PASS: gdb.base/sigaltstack.exp: finish from catch LEAF
 -PASS: gdb.base/sigaltstack.exp: finish to throw INNER
 -PASS: gdb.base/sigaltstack.exp: finish to catch INNER
 -PASS: gdb.base/sigaltstack.exp: finish from catch INNER
 -PASS: gdb.base/sigaltstack.exp: finish to OUTER
 -PASS: gdb.base/sigaltstack.exp: finish to catch MAIN
 -PASS: gdb.base/sigaltstack.exp: finish to MAIN
 +FAIL: gdb.base/sigaltstack.exp: backtrace (pattern 1)
 +FAIL: gdb.base/sigaltstack.exp: finish from catch LEAF (the program is no longer running)
 +FAIL: gdb.base/sigaltstack.exp: finish to throw INNER (the program is no longer running)
 +FAIL: gdb.base/sigaltstack.exp: finish to catch INNER (the program is no longer running)
 +FAIL: gdb.base/sigaltstack.exp: finish from catch INNER (the program is no longer running)
 +FAIL: gdb.base/sigaltstack.exp: finish to OUTER (the program is no longer running)
 +FAIL: gdb.base/sigaltstack.exp: finish to catch MAIN (the program is no longer running)
 +FAIL: gdb.base/sigaltstack.exp: finish to MAIN (the program is no longer running)

for me.

The problem is in the `level' variable in the test.


 (gdb) break catcher if level == INNER
 A syntax error in expression, near `== INNER'.
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 (gdb) PASS: gdb.base/sigaltstack.exp: break catcher if level == INNER
 continue
 Continuing.

 Program received signal SIGALRM, Alarm clock.

 Program received signal SIGVTALRM, Virtual timer expired.

 Program exited with code 03.
 (gdb) FAIL: gdb.base/sigaltstack.exp: continue to catch (the program exited)
 ...

Note that the variable has the same name as the enum:

 enum level { MAIN, OUTER, INNER, LEAF, NR_LEVELS };

 /* Levels completed flag.  */
 volatile enum level level = NR_LEVELS;


It boils down to this --- before the patch, running sigaltstack
under gdb (run to main):

 (gdb) p level
 $1 = NR_LEVELS
 (gdb) ptype level
 type = volatile enum level {MAIN, OUTER, INNER, LEAF, NR_LEVELS}

and after the patch:

 (gdb) p level
 Attempt to use a type name as an expression
 (gdb) ptype level
 type = volatile enum level {MAIN, OUTER, INNER, LEAF, NR_LEVELS}



Here's the relevant dwarf:

 <1><44d>: Abbrev Number: 20 (DW_TAG_enumeration_type)
    <44e>   DW_AT_name        : (indirect string, offset: 0xe3): level	
    <452>   DW_AT_byte_size   : 4	
    <453>   DW_AT_decl_file   : 1	
    <454>   DW_AT_decl_line   : 24	
    <455>   DW_AT_sibling     : <0x478>	
 <2><459>: Abbrev Number: 21 (DW_TAG_enumerator)
    <45a>   DW_AT_name        : (indirect string, offset: 0x2be): MAIN	
    <45e>   DW_AT_const_value : 0	
 <2><45f>: Abbrev Number: 21 (DW_TAG_enumerator)
    <460>   DW_AT_name        : (indirect string, offset: 0x281): OUTER	
    <464>   DW_AT_const_value : 1	
...
 <1><5b7>: Abbrev Number: 31 (DW_TAG_variable)
    <5b8>   DW_AT_name        : (indirect string, offset: 0xe3): level	
    <5bc>   DW_AT_decl_file   : 1	
    <5bd>   DW_AT_decl_line   : 27	
    <5be>   DW_AT_type        : <0x5cd>	
    <5c2>   DW_AT_external    : 1	
    <5c3>   DW_AT_location    : 9 byte block: 3 48 10 60 0 0 0 0 0 	(DW_OP_addr: 601048)
 <1><5cd>: Abbrev Number: 32 (DW_TAG_volatile_type)
    <5ce>   DW_AT_name        : (indirect string, offset: 0xe3): level	
    <5d2>   DW_AT_type        : <0x44d>	

Below's the patch again:  It's not clear to me why is it correct to
treat DW_TAG_const_type and DW_TAG_volatile_type qualifiers as
typedefs, though I haven't investigated this much yet.  FYI, or
in case it rings a bell...

> Index: src/gdb/dwarf2read.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/dwarf2read.c,v
> retrieving revision 1.386
> diff -u -p -r1.386 dwarf2read.c
> --- src/gdb/dwarf2read.c        17 May 2010 15:55:01 -0000      1.386
> +++ src/gdb/dwarf2read.c        21 May 2010 14:40:59 -0000
> @@ -3194,6 +3194,8 @@ process_die (struct die_info *die, struc
>      case DW_TAG_base_type:
>      case DW_TAG_subrange_type:
>      case DW_TAG_typedef:
> +    case DW_TAG_const_type:
> +    case DW_TAG_volatile_type:
>        /* Add a typedef symbol for the type definition, if it has a
>           DW_AT_name.  */
>        new_symbol (die, read_type_die (die, cu), cu);
> @@ -8742,6 +8744,8 @@ new_symbol (struct die_info *die, struct
>           break;
>         case DW_TAG_base_type:
>          case DW_TAG_subrange_type:
> +        case DW_TAG_const_type:
> +        case DW_TAG_volatile_type:
>           SYMBOL_CLASS (sym) = LOC_TYPEDEF;
>           SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
>           add_symbol_to_list (sym, cu->list_in_scope);

-- 
Pedro Alves

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

* RE: [RFA]dwarf reader:  Avoid complaint on const type
  2010-06-16 16:08     ` Pedro Alves
@ 2010-06-20 22:39       ` Pierre Muller
       [not found]       ` <3752333521215815628@unknownmsgid>
  1 sibling, 0 replies; 11+ messages in thread
From: Pierre Muller @ 2010-06-20 22:39 UTC (permalink / raw)
  To: 'Pedro Alves'; +Cc: gdb-patches

Hi Pedro,

  I checked this a little, but it is still unclear to me:
gcc-3 only emits a DW_AT_type attribute, no DW_AT_name,
so there is no problem in that case.
  On gcc16, I do see the failure that you see,
but I rechecked my tests when I submitted, and there 
is no sigaltstack failure.
  I think that wehen I submitted the patch, I was supposing that
if the DW_TAG_volatile_type would have a name attribute,
it would only be to give the name of the newly defined type,
i.e. for a typedef.
  Apparently, GCC changed a reports the old "unmodified" type name.

  The patch below should fix this.


Pierre Muller

PS: 
gcc does not emit typedefs for volatile types:


>>>>Start of test-vol.c<<<<<<
enum level { MAIN, OUTER, INNER, LEAF, NR_LEVELS };

/* Levels completed flag.  */
typedef volatile enum level level2;
volatile enum level level = NR_LEVELS;

typedef volatile int  volint;

volint vol;

level2 test = OUTER;

int local ()
{
  typedef volatile enum level level;
  level loc;
  loc = INNER;
  return 0;
}

int
main ()
{
  vol = 5;
  level = MAIN;
  test = LEAF;
  local ();
  return 0;
}
>>>>Start of test-vol.c<<<<<<
gcc -gdwarf2 test-vol.c
results in an  executable with no information
about volint type.
(stabs info gives correct information)...
I have to add here that I hate the C way of describing typedefs:
(gdb) inf type myint
type int;
This is almost completely useless, especially when you do a 
(gdb) inf type
myint does not appear anywhere :(
 

Back to the patch itself:

ChangeLog entry:

2010-06-21  Pierre Muller  <muller@ics.u-strasbg.fr>

	* dwarf2read.c (process_die): Do not call new_symbol
	for DW_TAG_volatile_type and DW_TAG_const_type.
	(new_symbol): Do not add the name of DW_TAG_volatile_type 
	and DW_TAG_const_type to the symbol list.



Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.400
diff -u -p -r1.400 dwarf2read.c
--- dwarf2read.c        17 Jun 2010 22:36:41 -0000      1.400
+++ dwarf2read.c        20 Jun 2010 21:57:23 -0000
@@ -3210,12 +3210,14 @@ process_die (struct die_info *die, struc
     case DW_TAG_base_type:
     case DW_TAG_subrange_type:
     case DW_TAG_typedef:
-    case DW_TAG_const_type:
-    case DW_TAG_volatile_type:
       /* Add a typedef symbol for the type definition, if it has a
          DW_AT_name.  */
       new_symbol (die, read_type_die (die, cu), cu);
       break;
+    case DW_TAG_const_type:
+    case DW_TAG_volatile_type:
+      read_type_die (die, cu);
+      break;
     case DW_TAG_common_block:
       read_common_block (die, cu);
       break;
@@ -8912,12 +8914,16 @@ new_symbol (struct die_info *die, struct
          break;
        case DW_TAG_base_type:
         case DW_TAG_subrange_type:
-        case DW_TAG_const_type:
-        case DW_TAG_volatile_type:
          SYMBOL_CLASS (sym) = LOC_TYPEDEF;
          SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
          add_symbol_to_list (sym, cu->list_in_scope);
          break;
+        case DW_TAG_const_type:
+        case DW_TAG_volatile_type:
+         /* The name of the type given in the dwarf name atribute is the
+            name of the `normal' type and not a new type name, so
+            do not register this as a new type name.  */
+         break;
        case DW_TAG_enumerator:
          attr = dwarf2_attr (die, DW_AT_const_value, cu);
          if (attr)

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

* Re: [RFA]dwarf reader: Avoid complaint on const type
       [not found]       ` <3752333521215815628@unknownmsgid>
@ 2010-06-28 20:27         ` Doug Evans
  2010-06-29 13:09           ` Pierre Muller
       [not found]           ` <29342.6726283089$1277816998@news.gmane.org>
  0 siblings, 2 replies; 11+ messages in thread
From: Doug Evans @ 2010-06-28 20:27 UTC (permalink / raw)
  To: Pierre Muller; +Cc: Pedro Alves, gdb-patches

On Sun, Jun 20, 2010 at 3:39 PM, Pierre Muller
<pierre.muller@ics-cnrs.unistra.fr> wrote:
> 2010-06-21  Pierre Muller  <muller@ics.u-strasbg.fr>
>
>        * dwarf2read.c (process_die): Do not call new_symbol
>        for DW_TAG_volatile_type and DW_TAG_const_type.
>        (new_symbol): Do not add the name of DW_TAG_volatile_type
>        and DW_TAG_const_type to the symbol list.
>
>
>
> Index: dwarf2read.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/dwarf2read.c,v
> retrieving revision 1.400
> diff -u -p -r1.400 dwarf2read.c
> --- dwarf2read.c        17 Jun 2010 22:36:41 -0000      1.400
> +++ dwarf2read.c        20 Jun 2010 21:57:23 -0000
> @@ -3210,12 +3210,14 @@ process_die (struct die_info *die, struc
>     case DW_TAG_base_type:
>     case DW_TAG_subrange_type:
>     case DW_TAG_typedef:
> -    case DW_TAG_const_type:
> -    case DW_TAG_volatile_type:
>       /* Add a typedef symbol for the type definition, if it has a
>          DW_AT_name.  */
>       new_symbol (die, read_type_die (die, cu), cu);
>       break;
> +    case DW_TAG_const_type:
> +    case DW_TAG_volatile_type:
> +      read_type_die (die, cu);
> +      break;
>     case DW_TAG_common_block:
>       read_common_block (die, cu);
>       break;
> @@ -8912,12 +8914,16 @@ new_symbol (struct die_info *die, struct
>          break;
>        case DW_TAG_base_type:
>         case DW_TAG_subrange_type:
> -        case DW_TAG_const_type:
> -        case DW_TAG_volatile_type:
>          SYMBOL_CLASS (sym) = LOC_TYPEDEF;
>          SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
>          add_symbol_to_list (sym, cu->list_in_scope);
>          break;
> +        case DW_TAG_const_type:
> +        case DW_TAG_volatile_type:
> +         /* The name of the type given in the dwarf name atribute is the
> +            name of the `normal' type and not a new type name, so
> +            do not register this as a new type name.  */
> +         break;
>        case DW_TAG_enumerator:
>          attr = dwarf2_attr (die, DW_AT_const_value, cu);
>          if (attr)
>
>

Hi.  I'd like to reach closure on this and get rid of this testsuite failure.

I *think* the patch to process_die is ok.

I'm not sure about the patch to new_symbol though.
AFAICT, new_symbol will never be called for
DW_TAG_volatile_type/const_type (*including* via bad debug info - IOW
the code itself would never call new_symbol for these tags).
*If* that is true, I wouldn't mind calling gdb_assert (0) for those
tags - make it clearer to the caller what the API is.
One may want to just leave it alone and let the default case flag a complaint.

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

* RE: [RFA]dwarf reader: Avoid complaint on const type
  2010-06-28 20:27         ` Doug Evans
@ 2010-06-29 13:09           ` Pierre Muller
  2010-07-01 17:09             ` Pedro Alves
       [not found]           ` <29342.6726283089$1277816998@news.gmane.org>
  1 sibling, 1 reply; 11+ messages in thread
From: Pierre Muller @ 2010-06-29 13:09 UTC (permalink / raw)
  To: 'Doug Evans'; +Cc: 'Pedro Alves', gdb-patches

 I agree with Doug's analysis that new_symbol
should never be called for those dwarf tags.
 The most logical seems to just revert the addition of 
of the two tags, which would lead to a complaint if
new_symbol would be called for those tags,
as it did before my original patch.
  As Doug explained, this should not happen anymore
as new_symbol is only called from process_die
and the first part of the patch removes this call for
DW_TAG_const_type and DW_TAG_volatile_type.

  Tested on x86_64 linux machine from GCC compiler farm,
nothing related to the patch seemed to appear.
(sigaltstack.exp showed failures but I don't think this is related).

Pierre
 
 
2010-06-29  Pierre Muller  <muller@ics.u-strasbg.fr>
	    Doug Evans  <dje@google.com>

	* dwarf2read.c (process_die): Only call read_type_die function for
	const and volatile modifiers.
	(new_symbol): Revert change adding a new type for DW_TAG_const_type
	or DW_TAG_volatile_type tag if it has a name attribute.

Index: src/gdb/dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.406
diff -u -p -r1.406 dwarf2read.c
--- src/gdb/dwarf2read.c	28 Jun 2010 22:03:31 -0000	1.406
+++ src/gdb/dwarf2read.c	29 Jun 2010 12:31:55 -0000
@@ -3220,12 +3220,15 @@ process_die (struct die_info *die, struc
     case DW_TAG_base_type:
     case DW_TAG_subrange_type:
     case DW_TAG_typedef:
-    case DW_TAG_const_type:
-    case DW_TAG_volatile_type:
       /* Add a typedef symbol for the type definition, if it has a
          DW_AT_name.  */
       new_symbol (die, read_type_die (die, cu), cu);
       break;
+      /* Type modifiers should be accepted without creating a new type
name.  */
+    case DW_TAG_const_type:
+    case DW_TAG_volatile_type:
+      read_type_die (die, cu);
+      break;
     case DW_TAG_common_block:
       read_common_block (die, cu);
       break;
@@ -8987,9 +8990,7 @@ new_symbol (struct die_info *die, struct
 	  add_symbol_to_list (sym, cu->list_in_scope);
 	  break;
 	case DW_TAG_base_type:
-        case DW_TAG_subrange_type:
-        case DW_TAG_const_type:
-        case DW_TAG_volatile_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);

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

* Re: [RFA]dwarf reader: Avoid complaint on const type
  2010-06-29 13:09           ` Pierre Muller
@ 2010-07-01 17:09             ` Pedro Alves
  2010-07-01 17:13               ` Joel Brobecker
  0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2010-07-01 17:09 UTC (permalink / raw)
  To: Pierre Muller; +Cc: 'Doug Evans', gdb-patches

I hope you're not waiting for me to review this.  (Honestly, in cases
like these, I prefer to simply revert the original broken patch, and
then calmly build a new correct patch from scratch.  It becomes
much easier to see and understand what was being addressed originaly
instead of having to look at the code in 3D.  I've noticed that
people here have revert fobia though ;-) ).

On Tuesday 29 June 2010 14:09:41, Pierre Muller wrote:
>  I agree with Doug's analysis that new_symbol
> should never be called for those dwarf tags.
>  The most logical seems to just revert the addition of 
> of the two tags, which would lead to a complaint if
> new_symbol would be called for those tags,
> as it did before my original patch.
>   As Doug explained, this should not happen anymore
> as new_symbol is only called from process_die
> and the first part of the patch removes this call for
> DW_TAG_const_type and DW_TAG_volatile_type.
> 
>   Tested on x86_64 linux machine from GCC compiler farm,
> nothing related to the patch seemed to appear.
> (sigaltstack.exp showed failures but I don't think this is related).
> 
> Pierre
>  
>  
> 2010-06-29  Pierre Muller  <muller@ics.u-strasbg.fr>
> 	    Doug Evans  <dje@google.com>
> 
> 	* dwarf2read.c (process_die): Only call read_type_die function for
> 	const and volatile modifiers.
> 	(new_symbol): Revert change adding a new type for DW_TAG_const_type
> 	or DW_TAG_volatile_type tag if it has a name attribute.
> 
> Index: src/gdb/dwarf2read.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/dwarf2read.c,v
> retrieving revision 1.406
> diff -u -p -r1.406 dwarf2read.c
> --- src/gdb/dwarf2read.c	28 Jun 2010 22:03:31 -0000	1.406
> +++ src/gdb/dwarf2read.c	29 Jun 2010 12:31:55 -0000
> @@ -3220,12 +3220,15 @@ process_die (struct die_info *die, struc
>      case DW_TAG_base_type:
>      case DW_TAG_subrange_type:
>      case DW_TAG_typedef:
> -    case DW_TAG_const_type:
> -    case DW_TAG_volatile_type:
>        /* Add a typedef symbol for the type definition, if it has a
>           DW_AT_name.  */
>        new_symbol (die, read_type_die (die, cu), cu);
>        break;
> +      /* Type modifiers should be accepted without creating a new type
> name.  */
> +    case DW_TAG_const_type:
> +    case DW_TAG_volatile_type:
> +      read_type_die (die, cu);
> +      break;
>      case DW_TAG_common_block:
>        read_common_block (die, cu);
>        break;
> @@ -8987,9 +8990,7 @@ new_symbol (struct die_info *die, struct
>  	  add_symbol_to_list (sym, cu->list_in_scope);
>  	  break;
>  	case DW_TAG_base_type:
> -        case DW_TAG_subrange_type:
> -        case DW_TAG_const_type:
> -        case DW_TAG_volatile_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);
> 
> 


-- 
Pedro Alves

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

* Re: [RFA]dwarf reader: Avoid complaint on const type
  2010-07-01 17:09             ` Pedro Alves
@ 2010-07-01 17:13               ` Joel Brobecker
  2010-07-21 17:16                 ` Pedro Alves
  0 siblings, 1 reply; 11+ messages in thread
From: Joel Brobecker @ 2010-07-01 17:13 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Pierre Muller, 'Doug Evans', gdb-patches

> I hope you're not waiting for me to review this.  (Honestly, in cases
> like these, I prefer to simply revert the original broken patch, and
> then calmly build a new correct patch from scratch.  It becomes
> much easier to see and understand what was being addressed originaly
> instead of having to look at the code in 3D.  I've noticed that
> people here have revert fobia though ;-) ).

Agreed on both counts - we should not be reluctant to revert.

-- 
Joel

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

* Re: [RFA]dwarf reader: Avoid complaint on const type
       [not found]           ` <29342.6726283089$1277816998@news.gmane.org>
@ 2010-07-02 21:47             ` Tom Tromey
  0 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2010-07-02 21:47 UTC (permalink / raw)
  To: Pierre Muller; +Cc: 'Doug Evans', 'Pedro Alves', gdb-patches

>>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr> writes:

Pierre> --- src/gdb/dwarf2read.c	28 Jun 2010 22:03:31 -0000	1.406
Pierre> +++ src/gdb/dwarf2read.c	29 Jun 2010 12:31:55 -0000
Pierre> @@ -3220,12 +3220,15 @@ process_die (struct die_info *die, struc
Pierre>      case DW_TAG_base_type:
Pierre>      case DW_TAG_subrange_type:
Pierre>      case DW_TAG_typedef:
Pierre> -    case DW_TAG_const_type:
Pierre> -    case DW_TAG_volatile_type:
Pierre>        /* Add a typedef symbol for the type definition, if it has a
Pierre>           DW_AT_name.  */
Pierre>        new_symbol (die, read_type_die (die, cu), cu);
Pierre>        break;
Pierre> +      /* Type modifiers should be accepted without creating a new type
Pierre> name.  */
Pierre> +    case DW_TAG_const_type:
Pierre> +    case DW_TAG_volatile_type:
Pierre> +      read_type_die (die, cu);
Pierre> +      break;

Why here and not a little earlier, with this case?

    /* These dies have a type, but processing them does not create
       a symbol or recurse to process the children.  Therefore we can
       read them on-demand through read_type_die.  */
    case DW_TAG_subroutine_type:
    case DW_TAG_set_type:
    case DW_TAG_array_type:
    case DW_TAG_pointer_type:
    case DW_TAG_ptr_to_member_type:
    case DW_TAG_reference_type:
    case DW_TAG_string_type:
      break;

Pierre>  	case DW_TAG_base_type:
Pierre> -        case DW_TAG_subrange_type:
Pierre> -        case DW_TAG_const_type:
Pierre> -        case DW_TAG_volatile_type:
Pierre> +	case DW_TAG_subrange_type:
Pierre>  	  SYMBOL_CLASS (sym) = LOC_TYPEDEF;
Pierre>  	  SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
Pierre>  	  add_symbol_to_list (sym, cu->list_in_scope);

Why remove DW_TAG_subrange_type here?

Tom

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

* Re: [RFA]dwarf reader: Avoid complaint on const type
  2010-07-01 17:13               ` Joel Brobecker
@ 2010-07-21 17:16                 ` Pedro Alves
  0 siblings, 0 replies; 11+ messages in thread
From: Pedro Alves @ 2010-07-21 17:16 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Pierre Muller, 'Doug Evans', gdb-patches

Since a month has passed and this hasn't been fully resolved
yet, I'm reverting the original patch, and adding a test
independent of sigaltstack.exp covering the problem.  I also
filed PR11827 for the problem, just so I could add a pointer in
the testcase.

I'll apply this to both the 7.2 branch and mainline in a bit.

-- 
Pedro Alves

gdb/
2010-07-21  Pedro Alves  <pedro@codesourcery.com>

	PR symtab/11827

	Revert:
	2010-05-21  Pierre Muller  <muller@ics.u-strasbg.fr>
	* dwarf2read.c (process_die): Also allow DW_TAG_const_type
	and DW_TAG_volatile_type.
	(new_symbol): Likewise.

gdb/testsuite/
2010-07-21  Pedro Alves  <pedro@codesourcery.com>

	PR symtab/11827

	* gdb.base/printcmds.c (enum some_volatile_enum): New enum.
	(some_volatile_enum): New variable.
	* gdb.base/printcmds.exp (test_print_enums): New.
	<top level>: Call it.

---
 gdb/dwarf2read.c                     |    4 ----
 gdb/testsuite/gdb.base/printcmds.c   |    6 ++++++
 gdb/testsuite/gdb.base/printcmds.exp |    6 ++++++
 3 files changed, 12 insertions(+), 4 deletions(-)

Index: src/gdb/testsuite/gdb.base/printcmds.c
===================================================================
--- src.orig/gdb/testsuite/gdb.base/printcmds.c	2010-07-21 18:03:30.000000000 +0100
+++ src/gdb/testsuite/gdb.base/printcmds.c	2010-07-21 18:05:08.000000000 +0100
@@ -90,6 +90,12 @@ struct some_arrays {
 
 struct some_arrays *parrays = &arrays;
 
+enum some_volatile_enum { enumvolval1, enumvolval2 };
+
+/* A volatile enum variable whose name is the same as the enumeration
+   name.  See PR11827.  */
+volatile enum some_volatile_enum some_volatile_enum = enumvolval1;
+
 /* -- */
 
 int main ()
Index: src/gdb/testsuite/gdb.base/printcmds.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/printcmds.exp	2010-07-21 18:03:30.000000000 +0100
+++ src/gdb/testsuite/gdb.base/printcmds.exp	2010-07-21 18:05:37.000000000 +0100
@@ -671,6 +671,11 @@ proc test_print_array_constants {} {
     gdb_test "print *&{4,5,6}\[1\]"	"Attempt to take address of value not located in memory."
 }
 
+proc test_print_enums {} {
+    # Regression test for PR11827.
+    gdb_test "print some_volatile_enum" "enumvolval1"
+}
+
 proc test_printf {} {
     gdb_test "printf \"x=%d,y=%d,z=%d\\n\", 5, 6, 7" "x=5,y=6,z=7"
     gdb_test "printf \"string=%.4sxx\\n\", teststring" "string=testxx"
@@ -800,6 +805,7 @@ if [set_lang_c] then {
 # We used to do the runto main here.
 	test_print_string_constants
 	test_print_array_constants
+	test_print_enums
 	test_printf
 	test_printf_with_dfp
     }
Index: src/gdb/dwarf2read.c
===================================================================
--- src.orig/gdb/dwarf2read.c	2010-07-21 18:06:55.000000000 +0100
+++ src/gdb/dwarf2read.c	2010-07-21 18:07:10.000000000 +0100
@@ -4192,8 +4192,6 @@ process_die (struct die_info *die, struc
     case DW_TAG_base_type:
     case DW_TAG_subrange_type:
     case DW_TAG_typedef:
-    case DW_TAG_const_type:
-    case DW_TAG_volatile_type:
       /* Add a typedef symbol for the type definition, if it has a
          DW_AT_name.  */
       new_symbol (die, read_type_die (die, cu), cu);
@@ -10001,8 +9999,6 @@ new_symbol (struct die_info *die, struct
 	  break;
 	case DW_TAG_base_type:
         case DW_TAG_subrange_type:
-        case DW_TAG_const_type:
-        case DW_TAG_volatile_type:
 	  SYMBOL_CLASS (sym) = LOC_TYPEDEF;
 	  SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
 	  add_symbol_to_list (sym, cu->list_in_scope);

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

* [RFA]dwarf reader:  Avoid complaint on const type
@ 2010-05-21 15:31 Pierre Muller
  0 siblings, 0 replies; 11+ messages in thread
From: Pierre Muller @ 2010-05-21 15:31 UTC (permalink / raw)
  To: gdb-patches

  GDB CVS compiled for Linux arm gave several dwarf complaints 
when debugging itself that I tried to fix.

  Here is a first fix:
It seem that some symbol directly come as
DW_TAG_const_type, I believe it was due to
typedef const struct reloc_howto_struct reloc_howto_type;
from bfd-in.h (or bfd-in2.h), but I did not recheck this
carefully.

  The patch below fixes that complaint.

  Tested on compiler farm machine gcc16, no regression found.

  Is this correct?
Maybe there are other position where DW_TAG_const_type
and DW_TAG_volatile_type should be added...

Pierre Muller
Pascal language support maintainer for GDB


2010-05-21  Pierre Muller  <muller@ics.u-strasbg.fr>

	* dwarf2read.c (process_die): Also allow DW_TAG_const_type
	and DW_TAG_volatile_type.
	(new_symbol): Likewise.

Index: src/gdb/dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.386
diff -u -p -r1.386 dwarf2read.c
--- src/gdb/dwarf2read.c	17 May 2010 15:55:01 -0000	1.386
+++ src/gdb/dwarf2read.c	21 May 2010 14:40:59 -0000
@@ -3194,6 +3194,8 @@ process_die (struct die_info *die, struc
     case DW_TAG_base_type:
     case DW_TAG_subrange_type:
     case DW_TAG_typedef:
+    case DW_TAG_const_type:
+    case DW_TAG_volatile_type:
       /* Add a typedef symbol for the type definition, if it has a
          DW_AT_name.  */
       new_symbol (die, read_type_die (die, cu), cu);
@@ -8742,6 +8744,8 @@ new_symbol (struct die_info *die, struct
 	  break;
 	case DW_TAG_base_type:
         case DW_TAG_subrange_type:
+        case DW_TAG_const_type:
+        case DW_TAG_volatile_type:
 	  SYMBOL_CLASS (sym) = LOC_TYPEDEF;
 	  SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
 	  add_symbol_to_list (sym, cu->list_in_scope);

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

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

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <41597.7287375883$1274454923@news.gmane.org>
2010-05-21 17:20 ` [RFA]dwarf reader: Avoid complaint on const type Tom Tromey
2010-05-21 20:46   ` Pierre Muller
2010-06-16 16:08     ` Pedro Alves
2010-06-20 22:39       ` Pierre Muller
     [not found]       ` <3752333521215815628@unknownmsgid>
2010-06-28 20:27         ` Doug Evans
2010-06-29 13:09           ` Pierre Muller
2010-07-01 17:09             ` Pedro Alves
2010-07-01 17:13               ` Joel Brobecker
2010-07-21 17:16                 ` Pedro Alves
     [not found]           ` <29342.6726283089$1277816998@news.gmane.org>
2010-07-02 21:47             ` Tom Tromey
2010-05-21 15:31 Pierre Muller

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