[1/4] RFC: skip DIEs which only declare an enum

[1/4] RFC: skip DIEs which only declare an enum

[Tom Tromey]

With -gdwarf-4:

-PASS: gdb.cp/classes.exp: print ('ClassWithEnum::PrivEnum') 42
-PASS: gdb.cp/classes.exp: print (ClassWithEnum::PrivEnum) 42
+FAIL: gdb.cp/classes.exp: print ('ClassWithEnum::PrivEnum') 42
+FAIL: gdb.cp/classes.exp: print (ClassWithEnum::PrivEnum) 42

What happens here is that gcc emits multiple .debug_type CUs.  While
reading one such CU, we see an empty declaration for PrivEnum.  This
gets turned into a symbol, and later we end up with an incomplete type.

This patch fixes the problem by simply skipping declaration-only enums.
This makes it so only the full definition is put into the symbol table,
letting check_typedef work.

This is one of the patches I am less sure about.  Is it really safe?  It
also may change the error a user sees in some obscure case -- but to my
mind there is not much practical difference between "type not found" and
"type found but useless".

Built and regtested by the buildbot.

Tom

b/gdb/ChangeLog:
2011-07-15  Tom Tromey  <tromey@redhat.com>

	* dwarf2read.c (process_enumeration_scope): Do nothing if DIE is a
	declaration.

From 70edf0b3d0c20619964be5692f4782efec033c94 Mon Sep 17 00:00:00 2001
From: Tom Tromey <tromey@redhat.com>
Date: Thu, 14 Jul 2011 14:48:47 -0600
Subject: [PATCH 1/4] do not process an enum which is just a declaration

---
 gdb/ChangeLog    |    5 +++++
 gdb/dwarf2read.c |    3 +++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index fde5b6a..0fd3845 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -7612,6 +7612,9 @@ process_enumeration_scope (struct die_info *die, struct dwarf2_cu *cu)
 {
   struct type *this_type;
 
+  if (die_is_declaration (die, cu))
+    return;
+
   this_type = get_die_type (die, cu);
   if (this_type == NULL)
     this_type = read_enumeration_type (die, cu);
-- 
1.7.6

Re: [1/4] RFC: skip DIEs which only declare an enum

[Daniel Jacobowitz]

On Fri, Jul 15, 2011 at 2:06 PM, Tom Tromey <tromey@redhat.com> wrote:
> With -gdwarf-4:
>
> -PASS: gdb.cp/classes.exp: print ('ClassWithEnum::PrivEnum') 42
> -PASS: gdb.cp/classes.exp: print (ClassWithEnum::PrivEnum) 42
> +FAIL: gdb.cp/classes.exp: print ('ClassWithEnum::PrivEnum') 42
> +FAIL: gdb.cp/classes.exp: print (ClassWithEnum::PrivEnum) 42
>
> What happens here is that gcc emits multiple .debug_type CUs.  While
> reading one such CU, we see an empty declaration for PrivEnum.  This
> gets turned into a symbol, and later we end up with an incomplete type.
>
> This patch fixes the problem by simply skipping declaration-only enums.
> This makes it so only the full definition is put into the symbol table,
> letting check_typedef work.
>
> This is one of the patches I am less sure about.  Is it really safe?  It
> also may change the error a user sees in some obscure case -- but to my
> mind there is not much practical difference between "type not found" and
> "type found but useless".

Generally speaking, I agree with your sentiment.  Can't you
forward-declare enums in the latest C++ draft, though?  If the
declaration is all we have, something like (enum X) 42 is more useful
than <unknown type> (both examples made up; I don't know what we
print.

We ought to be able to resolve the incomplete type if we have both.
But I know that doesn't always work right.

-- 
Thanks,
Daniel

Re: [1/4] RFC: skip DIEs which only declare an enum

[Tom Tromey]

>>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:

Daniel> Generally speaking, I agree with your sentiment.  Can't you
Daniel> forward-declare enums in the latest C++ draft, though?

Yes, thanks for pointing that out.  The F15 g++ accepts this translation
unit with -std=c++0x:

    enum x : int;
    enum x a;

This generates:

 <1><1d>: Abbrev Number: 2 (DW_TAG_enumeration_type)
    <1e>   DW_AT_name        : x        
    <20>   DW_AT_declaration : 1        
    <21>   DW_AT_byte_size   : 4        
    <22>   DW_AT_decl_file   : 1        
    <23>   DW_AT_decl_line   : 1        


I am going withdraw this patch and look at other ways to fix this
problem.

Tom

Re: [1/4] RFC: skip DIEs which only declare an enum

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:

Tom> I am going withdraw this patch and look at other ways to fix this
Tom> problem.

Here is what I came up with.

It seems to me that the bad case is exactly when we are reading a
.debug_types CU and the enum type we are reading is not the signatured
type.  In this case, the enum type is needed for other types in the CU,
but doesn't need a symbol, as presumably there is some other
.debug_types CU that has the true definition.

So, this patch implements just this exception.  I rebased on it on patch
#2, so the series ordering is different now.

Built and regtested by the buildbot.

In the absence of comments I plan to commit this in a couple of days.

Tom


2011-07-18  Tom Tromey  <tromey@redhat.com>

	* dwarf2read.c (process_enumeration_scope): Do not call new_symbol
	in some declaration-only cases.

From 79fc44c91390d14bcf0f98f7e19d9b8888952595 Mon Sep 17 00:00:00 2001
From: Tom Tromey <tromey@redhat.com>
Date: Mon, 18 Jul 2011 11:54:45 -0600
Subject: [PATCH 2/2] fix enum declaration regression

---
 gdb/ChangeLog    |    5 +++++
 gdb/dwarf2read.c |   26 ++++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 4c8a732..e6cc94a 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1204,6 +1204,11 @@ static struct die_info *follow_die_sig (struct die_info *,
 					struct attribute *,
 					struct dwarf2_cu **);
 
+static struct signatured_type *lookup_signatured_type_at_offset
+    (struct objfile *objfile,
+     struct dwarf2_section_info *section,
+     unsigned int offset);
+
 static void read_signatured_type_at_offset (struct objfile *objfile,
 					    struct dwarf2_section_info *sect,
 					    unsigned int offset);
@@ -7714,6 +7719,27 @@ process_enumeration_scope (struct die_info *die, struct dwarf2_cu *cu)
 	TYPE_UNSIGNED (this_type) = 1;
     }
 
+  /* If we are reading an enum from a .debug_types unit, and the enum
+     is a declaration, and the enum is not the signatured type in the
+     unit, then we do not want to add a symbol for it.  Adding a
+     symbol would in some cases obscure the true definition of the
+     enum, giving users an incomplete type when the definition is
+     actually available.  Note that we do not want to do this for all
+     enums which are just declarations, because C++0x allows forward
+     enum declarations.  */
+  if (cu->per_cu->debug_type_section
+      && die_is_declaration (die, cu))
+    {
+      struct signatured_type *type_sig;
+
+      type_sig
+	= lookup_signatured_type_at_offset (dwarf2_per_objfile->objfile,
+					    cu->per_cu->debug_type_section,
+					    cu->per_cu->offset);
+      if (type_sig->type_offset != die->offset)
+	return;
+    }
+
   new_symbol (die, this_type, cu);
 }
 
-- 
1.7.6

Re: [1/4] RFC: skip DIEs which only declare an enum

[Tom Tromey]

Tom> In the absence of comments I plan to commit this in a couple of days.

I'm checking it in now.

Tom