public inbox for archer@sourceware.org
 help / color / mirror / Atom feed
* using directive patch
@ 2008-09-03 18:07 Sami Wagiaalla
  2008-09-05 19:15 ` Tom Tromey
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Sami Wagiaalla @ 2008-09-03 18:07 UTC (permalink / raw)
  To: Project Archer

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

the attach patch solves the following problem

namespace A{
   int x;
}

int main{
   using namespace A;
   x;
}

With the patch x is visible when stopped in main, and no where else.

The above is the only variation I test. There is more to come, but I 
wanted to get some feed back early on.



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

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b00b374..e981dba 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,22 @@
+2008-09-03  Sami Wagiaalla  <swagiaal@redhat.com>
+
+	* dwarf2read.c (process_die): Added call to 
+	read_import_statement to the appropriate dies.
+	(read_import_statement): New function.
+	(read_func_scope): Resets the using directives from saved
+	context.
+	* cp-namespace.c: Changed cp_adding_using to extern.
+	Moved cp_adding_using prototype from here...
+	* cp-support.h: ...to here.
+	* buildsym.h: Add global variable using_directives.
+	Added using_directives variable to context struct.
+	Added extern prototype for 
+	* buildsym.c (finish_block): Set using directives for
+	the block, and reset the global variable.
+	(push_context): Save and reset using_directives.
+	* block.c (block_using): Return using directives for
+	the given block instead of its owning static block.
+
 2008-07-29  Tom Tromey  <tromey@redhat.com>
 
 	* cli/cli-decode.c (lookup_cmd_1): Use memcpy.
diff --git a/gdb/block.c b/gdb/block.c
index fd96a52..d48f546 100644
--- a/gdb/block.c
+++ b/gdb/block.c
@@ -218,13 +218,12 @@ block_set_scope (struct block *block, const char *scope,
 struct using_direct *
 block_using (const struct block *block)
 {
-  const struct block *static_block = block_static_block (block);
 
-  if (static_block == NULL
-      || BLOCK_NAMESPACE (static_block) == NULL)
+  if (block == NULL
+      || BLOCK_NAMESPACE (block) == NULL)
     return NULL;
   else
-    return BLOCK_NAMESPACE (static_block)->using;
+    return BLOCK_NAMESPACE (block)->using;
 }
 
 /* Set BLOCK's using member to USING; if needed, allocate memory via
diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index f3850a2..d80120c 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -396,6 +396,12 @@ finish_block (struct symbol *symbol, struct pending **listhead,
       opblock = pblock;
     }
 
+  block_set_using (block,
+		   using_directives,
+		   &objfile->objfile_obstack);
+  
+  using_directives = NULL;
+
   record_pending_block (objfile, block, opblock);
 
   return block;
@@ -1214,10 +1220,12 @@ push_context (int desc, CORE_ADDR valu)
   new->params = param_symbols;
   new->old_blocks = pending_blocks;
   new->start_addr = valu;
+  new->using_directives = using_directives;
   new->name = NULL;
 
   local_symbols = NULL;
   param_symbols = NULL;
+  using_directives = NULL;
 
   return new;
 }
diff --git a/gdb/buildsym.h b/gdb/buildsym.h
index 294bc41..0c4c851 100644
--- a/gdb/buildsym.h
+++ b/gdb/buildsym.h
@@ -125,6 +125,10 @@ EXTERN struct pending *local_symbols;
 
 EXTERN struct pending *param_symbols;
 
+/* using directives local to lexical context */
+
+EXTERN struct  using_direct *using_directives;
+
 /* Stack representing unclosed lexical contexts (that will become
    blocks, eventually).  */
 
@@ -138,6 +142,10 @@ struct context_stack
 
     struct pending *params;
 
+    /* Pending using directives at the time we entered */
+
+    struct  using_direct *using_directives;
+    
     /* Pointer into blocklist as of entry */
 
     struct pending_block *old_blocks;
diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 9c7ea59..9c629b8 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -61,11 +61,6 @@ const char *processing_current_prefix;
 
 static struct using_direct *using_list;
 
-static struct using_direct *cp_add_using (const char *name,
-					  unsigned int inner_len,
-					  unsigned int outer_len,
-					  struct using_direct *next);
-
 static struct using_direct *cp_copy_usings (struct using_direct *using,
 					    struct obstack *obstack);
 
@@ -269,7 +264,7 @@ cp_is_anonymous (const char *namespace)
    using xmalloc.  It copies the strings, so NAME can be a temporary
    string.  */
 
-static struct using_direct *
+extern struct using_direct *
 cp_add_using (const char *name,
 	      unsigned int inner_len,
 	      unsigned int outer_len,
diff --git a/gdb/cp-support.h b/gdb/cp-support.h
index 8c0119e..6f4b8c0 100644
--- a/gdb/cp-support.h
+++ b/gdb/cp-support.h
@@ -82,6 +82,11 @@ extern void cp_add_using_directive (const char *name,
 				    unsigned int outer_length,
 				    unsigned int inner_length);
 
+extern struct using_direct *cp_add_using (const char *name,
+					  unsigned int inner_len,
+					  unsigned int outer_len,
+					  struct using_direct *next);
+
 extern void cp_initialize_namespace (void);
 
 extern void cp_finalize_namespace (struct block *static_block,
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 8f1062d..99d89b9 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -932,6 +932,8 @@ static void read_common_block (struct die_info *, struct dwarf2_cu *);
 
 static void read_namespace (struct die_info *die, struct dwarf2_cu *);
 
+static void read_import_statement (struct die_info *die, struct dwarf2_cu *);
+
 static const char *namespace_name (struct die_info *die,
 				   int *is_anonymous, struct dwarf2_cu *);
 
@@ -2772,6 +2774,7 @@ process_die (struct die_info *die, struct dwarf2_cu *cu)
 	 Fortran compilers start generating that info.  */
       processing_has_namespace_info = 1;
       gdb_assert (die->child == NULL);
+      read_import_statement(die, cu);
       break;
     default:
       new_symbol (die, NULL, cu);
@@ -2779,6 +2782,26 @@ process_die (struct die_info *die, struct dwarf2_cu *cu)
     }
 }
 
+
+/*
+ * Read the import statement specified by the given die and record it.
+ */ 
+static void read_import_statement(struct die_info *die, struct dwarf2_cu *cu){
+  static struct die_info* imported_die;
+  struct attribute *import_attr = dwarf2_attr (die, DW_AT_import, cu);
+  char* imported_name;
+  
+  imported_die = follow_die_ref (die, import_attr, cu);
+  imported_name = dwarf2_name (imported_die, cu);
+  
+  using_directives = cp_add_using (imported_name,
+		  strlen(imported_name),
+		  0,
+  	    using_directives);
+  
+}
+
+
 static void
 initialize_cu_func_list (struct dwarf2_cu *cu)
 {
@@ -3042,7 +3065,7 @@ read_func_scope (struct die_info *die, struct dwarf2_cu *cu)
      back to building a containing block's symbol lists.  */
   local_symbols = new->locals;
   param_symbols = new->params;
-
+  using_directives = new->using_directives;
   /* If we've finished processing a top-level function, subsequent
      symbols go in the file symbol list.  */
   if (outermost_context_p ())

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

* Re: using directive patch
  2008-09-03 18:07 using directive patch Sami Wagiaalla
@ 2008-09-05 19:15 ` Tom Tromey
  2008-09-08 15:18   ` Sami Wagiaalla
  2008-09-07 18:37 ` Jan Kratochvil
  2008-09-10 19:03 ` using directive patch Sami Wagiaalla
  2 siblings, 1 reply; 15+ messages in thread
From: Tom Tromey @ 2008-09-05 19:15 UTC (permalink / raw)
  To: Sami Wagiaalla; +Cc: Project Archer

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

Sami> The above is the only variation I test. There is more to come, but I
Sami> wanted to get some feed back early on.

I don't understand it all, but it looks reasonable to me.

There are a number of formatting nits, but that is nothing much to
worry about.  The GNU Coding Standards cover how this code ought to
look.

It would be nice if this could be done without a new global variable,
but I understand that you're copying existing practice.

One important thing for merging (to archer trunk) is test cases.
Ideally the tests would specify expected behavior, both ordinary and
error cases.

Tom

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

* Re: using directive patch
  2008-09-03 18:07 using directive patch Sami Wagiaalla
  2008-09-05 19:15 ` Tom Tromey
@ 2008-09-07 18:37 ` Jan Kratochvil
  2008-09-08 15:16   ` Sami Wagiaalla
  2008-09-18 21:14   ` DW_TAG_imported_declaration DW_TAG_base_type w/o DW_AT_name [Re: using directive patch] Jan Kratochvil
  2008-09-10 19:03 ` using directive patch Sami Wagiaalla
  2 siblings, 2 replies; 15+ messages in thread
From: Jan Kratochvil @ 2008-09-07 18:37 UTC (permalink / raw)
  To: Sami Wagiaalla; +Cc: Project Archer

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

Hi,

the patch did not apply for HEAD - the current GIT snapshot should be updated.
Fixed a real crash on missing imported `DW_AT_name', fixed a possible crash on
missing `DW_AT_import'.

Unfortunately it cannot work right as there is a GCC Bug I submitted now for
`using namespace' inside blocks:
	http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37410
I expect that after fixing this GCC Bug the testcase included below (*1) will
fail due to the line `using_directives = NULL;' in finish_block() - IMO it
should restore (not reset) the original using-directives context.  Due to the
GCC Bug the GDB Bug is not reproducible.

There is also still missing the renaming support.


Thanks,
Jan


(*1):
At `break2' try to print `aaa'.
------------------------------------------------------------------------------
#include <stdio.h>

namespace A1
  {
    int aaa = 1;
  };
namespace A2
  {
    int bbb = 2;
  };

int
main (void)
{
  int x;
  using namespace A1;
  x = 0;

  {
    using namespace A2;
    int block_create;

    block_create = bbb; /* break1 */
    printf ("%d\n", bbb);
  }

  printf ("%d\n", aaa); /* break2 */

  return 0;
}
------------------------------------------------------------------------------

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

--- ./gdb/block.c	2008-09-07 10:12:55.000000000 +0200
+++ ./gdb/block.c	2008-09-07 20:22:44.000000000 +0200
@@ -207,24 +207,16 @@ block_set_scope (struct block *block, co
 }
 
 /* This returns the first using directives associated to BLOCK, if
-   any.  */
-
-/* FIXME: carlton/2003-04-23: This uses the fact that we currently
-   only have using directives in static blocks, because we only
-   generate using directives from anonymous namespaces.  Eventually,
-   when we support using directives everywhere, we'll want to replace
-   this by some iterator functions.  */
+   any.  Each BLOCK_NAMESPACE()->USING already contains all the namespaces
+   imported at that code point - even those from its parent blocks.  */
 
 struct using_direct *
 block_using (const struct block *block)
 {
-  const struct block *static_block = block_static_block (block);
-
-  if (static_block == NULL
-      || BLOCK_NAMESPACE (static_block) == NULL)
+  if (block == NULL || BLOCK_NAMESPACE (block) == NULL)
     return NULL;
   else
-    return BLOCK_NAMESPACE (static_block)->using;
+    return BLOCK_NAMESPACE (block)->using;
 }
 
 /* Set BLOCK's using member to USING; if needed, allocate memory via
--- ./gdb/buildsym.c	2008-08-21 20:40:34.000000000 +0200
+++ ./gdb/buildsym.c	2008-09-07 20:10:29.000000000 +0200
@@ -384,6 +384,10 @@ finish_block (struct symbol *symbol, str
       opblock = pblock;
     }
 
+  block_set_using (block, using_directives, &objfile->objfile_obstack);
+  
+  using_directives = NULL;
+
   record_pending_block (objfile, block, opblock);
 
   return block;
@@ -1202,10 +1206,12 @@ push_context (int desc, CORE_ADDR valu)
   new->params = param_symbols;
   new->old_blocks = pending_blocks;
   new->start_addr = valu;
+  new->using_directives = using_directives;
   new->name = NULL;
 
   local_symbols = NULL;
   param_symbols = NULL;
+  using_directives = NULL;
 
   return new;
 }
--- ./gdb/buildsym.h	2008-04-07 21:29:55.000000000 +0200
+++ ./gdb/buildsym.h	2008-09-07 17:25:56.000000000 +0200
@@ -125,6 +125,10 @@ EXTERN struct pending *local_symbols;
 
 EXTERN struct pending *param_symbols;
 
+/* using directives local to lexical context */
+
+EXTERN struct using_direct *using_directives;
+
 /* Stack representing unclosed lexical contexts (that will become
    blocks, eventually).  */
 
@@ -138,6 +142,10 @@ struct context_stack
 
     struct pending *params;
 
+    /* Pending using directives at the time we entered */
+
+    struct using_direct *using_directives;
+
     /* Pointer into blocklist as of entry */
 
     struct pending_block *old_blocks;
--- ./gdb/cp-namespace.c	2008-08-21 20:40:34.000000000 +0200
+++ ./gdb/cp-namespace.c	2008-09-07 17:26:15.000000000 +0200
@@ -35,11 +35,6 @@
 
 static struct using_direct *using_list;
 
-static struct using_direct *cp_add_using (const char *name,
-					  unsigned int inner_len,
-					  unsigned int outer_len,
-					  struct using_direct *next);
-
 static struct using_direct *cp_copy_usings (struct using_direct *using,
 					    struct obstack *obstack);
 
@@ -237,7 +232,7 @@ cp_is_anonymous (const char *namespace)
    using xmalloc.  It copies the strings, so NAME can be a temporary
    string.  */
 
-static struct using_direct *
+struct using_direct *
 cp_add_using (const char *name,
 	      unsigned int inner_len,
 	      unsigned int outer_len,
--- ./gdb/cp-support.h	2008-08-21 20:40:34.000000000 +0200
+++ ./gdb/cp-support.h	2008-09-07 17:12:50.000000000 +0200
@@ -80,6 +80,11 @@ extern void cp_add_using_directive (cons
 				    unsigned int outer_length,
 				    unsigned int inner_length);
 
+extern struct using_direct *cp_add_using (const char *name,
+					  unsigned int inner_len,
+					  unsigned int outer_len,
+					  struct using_direct *next);
+
 extern void cp_initialize_namespace (void);
 
 extern void cp_finalize_namespace (struct block *static_block,
--- ./gdb/dwarf2read.c	2008-09-02 00:21:45.000000000 +0200
+++ ./gdb/dwarf2read.c	2008-09-07 17:51:01.000000000 +0200
@@ -937,6 +937,8 @@ static void read_common_block (struct di
 
 static void read_namespace (struct die_info *die, struct dwarf2_cu *);
 
+static void read_import_statement (struct die_info *die, struct dwarf2_cu *);
+
 static const char *namespace_name (struct die_info *die,
 				   int *is_anonymous, struct dwarf2_cu *);
 
@@ -2756,14 +2758,12 @@ process_die (struct die_info *die, struc
       break;
     case DW_TAG_imported_declaration:
     case DW_TAG_imported_module:
-      /* FIXME: carlton/2002-10-16: Eventually, we should use the
-	 information contained in these.  DW_TAG_imported_declaration
-	 dies shouldn't have children; DW_TAG_imported_module dies
-	 shouldn't in the C++ case, but conceivably could in the
-	 Fortran case.  */
       processing_has_namespace_info = 1;
-      complaint (&symfile_complaints, _("unsupported tag: '%s'"),
-		 dwarf_tag_name (die->tag));
+      if (die->child != NULL && (die->tag == DW_TAG_imported_declaration
+				 || cu->language != language_fortran))
+	complaint (&symfile_complaints, _("Tag '%s' has unexpected children"),
+		   dwarf_tag_name (die->tag));
+      read_import_statement (die, cu);
       break;
     default:
       new_symbol (die, NULL, cu);
@@ -2809,6 +2809,37 @@ dwarf2_full_name (struct die_info *die, 
   return name;
 }
 
+/* Read the import statement specified by the given die and record it.  */ 
+
+static void
+read_import_statement (struct die_info *die, struct dwarf2_cu *cu)
+{
+  struct attribute *import_attr;
+  struct die_info *imported_die;
+  char *imported_name;
+  
+  import_attr = dwarf2_attr (die, DW_AT_import, cu);
+  if (import_attr == NULL)
+    {
+      complaint (&symfile_complaints, _("Tag '%s' has no DW_AT_import"),
+		 dwarf_tag_name (die->tag));
+      return;
+    }
+
+  imported_die = follow_die_ref (die, import_attr, &cu);
+  imported_name = dwarf2_name (imported_die, cu);
+  if (imported_name == NULL)
+    {
+      /* C++ imports from std:: DW_TAG_base_type with no DW_AT_name - why?  */
+      return;
+    }
+
+  /* FIXME: dwarf2_name (die); for the local name after import.  */
+  
+  using_directives = cp_add_using (imported_name, strlen (imported_name), 0,
+                                   using_directives);
+}
+
 static void
 initialize_cu_func_list (struct dwarf2_cu *cu)
 {
@@ -3043,6 +3074,7 @@ read_func_scope (struct die_info *die, s
      back to building a containing block's symbol lists.  */
   local_symbols = new->locals;
   param_symbols = new->params;
+  using_directives = new->using_directives;
 
   /* If we've finished processing a top-level function, subsequent
      symbols go in the file symbol list.  */

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

* Re: using directive patch
  2008-09-07 18:37 ` Jan Kratochvil
@ 2008-09-08 15:16   ` Sami Wagiaalla
  2008-09-08 15:31     ` Jan Kratochvil
  2008-09-18 21:14   ` DW_TAG_imported_declaration DW_TAG_base_type w/o DW_AT_name [Re: using directive patch] Jan Kratochvil
  1 sibling, 1 reply; 15+ messages in thread
From: Sami Wagiaalla @ 2008-09-08 15:16 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Project Archer

Thanks a lot for the comments

Jan Kratochvil wrote:
> Hi,
> 
> the patch did not apply for HEAD - the current GIT snapshot should be updated.
> Fixed a real crash on missing imported `DW_AT_name', fixed a possible crash on
> missing `DW_AT_import'.

Yeah I noticed that after I mailed the patch. Locally I used 
namespace_name()

> 
> Unfortunately it cannot work right as there is a GCC Bug I submitted now for
> `using namespace' inside blocks:
> 	http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37410

Good catch. I never noticed that before.

> I expect that after fixing this GCC Bug the testcase included below (*1) will
> fail due to the line `using_directives = NULL;' in finish_block() - IMO it
> should restore (not reset) the original using-directives context.  Due to the
> GCC Bug the GDB Bug is not reproducible.

Come to think of it I dont think that the line `using_directives = 
NULL;' is needed in finish_block(), but but I thought that the context 
is restored by the caller in this case, as the caller is the one who 
'pops' the context (like the line 'using_directives = 
new->using_directives;' in read_func_scope). Same should be done for 
read_lexical_block_scope. Is this correct ?

> 
> There is also still missing the renaming support.
> 
> 
> Thanks,
> Jan
> 
> 
> (*1):
> At `break2' try to print `aaa'.
> ------------------------------------------------------------------------------
> #include <stdio.h>
> 
> namespace A1
>   {
>     int aaa = 1;
>   };
> namespace A2
>   {
>     int bbb = 2;
>   };
> 
> int
> main (void)
> {
>   int x;
>   using namespace A1;
>   x = 0;
> 
>   {
>     using namespace A2;
>     int block_create;
> 
>     block_create = bbb; /* break1 */
>     printf ("%d\n", bbb);
>   }
> 
>   printf ("%d\n", aaa); /* break2 */
> 
>   return 0;
> }
> ------------------------------------------------------------------------------
> 

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

* Re: using directive patch
  2008-09-05 19:15 ` Tom Tromey
@ 2008-09-08 15:18   ` Sami Wagiaalla
  0 siblings, 0 replies; 15+ messages in thread
From: Sami Wagiaalla @ 2008-09-08 15:18 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Project Archer

Tom Tromey wrote:
>>>>>> "Sami" == Sami Wagiaalla <swagiaal@redhat.com> writes:
> 
> Sami> The above is the only variation I test. There is more to come, but I
> Sami> wanted to get some feed back early on.
> 
> I don't understand it all, but it looks reasonable to me.
> 
> There are a number of formatting nits, but that is nothing much to
> worry about.  The GNU Coding Standards cover how this code ought to
> look.
> 
> It would be nice if this could be done without a new global variable,
> but I understand that you're copying existing practice.
> 

Yes exactly I was copying parameters and local variables.

> One important thing for merging (to archer trunk) is test cases.
> Ideally the tests would specify expected behavior, both ordinary and
> error cases.
> 

Coming right up.

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

* Re: using directive patch
  2008-09-08 15:16   ` Sami Wagiaalla
@ 2008-09-08 15:31     ` Jan Kratochvil
  0 siblings, 0 replies; 15+ messages in thread
From: Jan Kratochvil @ 2008-09-08 15:31 UTC (permalink / raw)
  To: Sami Wagiaalla; +Cc: Project Archer

On Mon, 08 Sep 2008 17:15:24 +0200, Sami Wagiaalla wrote:
>> I expect that after fixing this GCC Bug the testcase included below (*1) will
>> fail due to the line `using_directives = NULL;' in finish_block() - IMO it
>> should restore (not reset) the original using-directives context.  Due to the
>> GCC Bug the GDB Bug is not reproducible.
>
> Come to think of it I dont think that the line `using_directives =  
> NULL;' is needed in finish_block(), but but I thought that the context  
> is restored by the caller in this case, as the caller is the one who  
> 'pops' the context (like the line 'using_directives =  
> new->using_directives;' in read_func_scope). Same should be done for  
> read_lexical_block_scope. Is this correct ?

I do not want to comment more without having a right DWARF at hand (either by
fixed GCC PR 37410 or fixed up for one testcase by hand).

Roughly I think using_directives should be rather updatedalready in
pop_context().  Moreover the variables local_symbols, param_symbols and
using_directives should be replaced by direct references to current
context_stack[context_stack_depth] (as suggested by Tom T.).  But such cleanup
I find as an unrelated patch (which this namespaces fix may make sense to have
dependent on).


Regards,
Jan

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

* Re: using directive patch
  2008-09-03 18:07 using directive patch Sami Wagiaalla
  2008-09-05 19:15 ` Tom Tromey
  2008-09-07 18:37 ` Jan Kratochvil
@ 2008-09-10 19:03 ` Sami Wagiaalla
  2008-09-16 16:54   ` Tom Tromey
  2 siblings, 1 reply; 15+ messages in thread
From: Sami Wagiaalla @ 2008-09-10 19:03 UTC (permalink / raw)
  To: Project Archer

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

Here is a second version of the patch after incorporating the 
suggestions and adding a test case.

I should also explain what the patch is doing:

Traditionally gdb only supported using directives only for anonymous 
name spaces. It did that by adding to the static block (block 
representing compilation unit scope) a list of using directives which 
state that namespace x is visible in scope y.

This patch make it so that every block not just the static block can own 
a list of using directives. That list states that namespace x is visible 
in 'this' block. The patch also initializes this list during the 
exploration of dies.

During the exploration of the die tree, when a new scope is started the 
context corresponding to the parent still unfinished scope is pushed 
into a context stack (i.e. local variables and parameters for the outer 
scope are saved aside until the exploration of the current one is done) 
once the current scope is finished the information is saved in a block 
object and the parent context is restored (poped). This patch add 
initialization of using directives parallel to the pattern described above.

this patch can be seen on the branch origin/archer-swagiaal-using-directive

git branch archer-swagiaal-using-directive 
origin/archer-swagiaal-using-directive

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

diff --git a/ChangeLog b/ChangeLog
index 5114518..3258557 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,22 @@
+2008-09-09  Sami Wagiaalla  <swagiaal@redhat.com>
+
+	* dwarf2read.c (process_die): Added call to
+	read_import_statement to the appropriate dies.
+	(read_import_statement): New function.
+	(read_func_scope): Resets the using directives from saved
+	context.	
+	* cp-namespace.c: Changed cp_adding_using to extern.
+	Moved cp_adding_using prototype from here...
+	* cp-support.h: ...to here.
+	* buildsym.h: Add global variable using_directives.
+	Added using_directives variable to context struct.
+	Added extern prototype for
+	* buildsym.c (finish_block): Set using directives for
+	the block, and reset the global variable.
+	(push_context): Save and reset using_directives.
+	* block.c (block_using): Return using directives for
+	the given block instead of its owning static block.
+
 2008-08-31  Aaron W. LaFramboise  <aaronavay62@aaronwl.com>
 
 	* configure.ac (RPATH_ENVVAR): Use PATH on Windows.
diff --git a/gdb/block.c b/gdb/block.c
index 58dcf72..45c10bf 100644
--- a/gdb/block.c
+++ b/gdb/block.c
@@ -207,24 +207,16 @@ block_set_scope (struct block *block, const char *scope,
 }
 
 /* This returns the first using directives associated to BLOCK, if
-   any.  */
-
-/* FIXME: carlton/2003-04-23: This uses the fact that we currently
-   only have using directives in static blocks, because we only
-   generate using directives from anonymous namespaces.  Eventually,
-   when we support using directives everywhere, we'll want to replace
-   this by some iterator functions.  */
+   any.  Each BLOCK_NAMESPACE()->USING already contains all the namespaces
+   imported at that code point - even those from its parent blocks.  */
 
 struct using_direct *
 block_using (const struct block *block)
 {
-  const struct block *static_block = block_static_block (block);
-
-  if (static_block == NULL
-      || BLOCK_NAMESPACE (static_block) == NULL)
+  if (block == NULL || BLOCK_NAMESPACE (block) == NULL)
     return NULL;
   else
-    return BLOCK_NAMESPACE (static_block)->using;
+    return BLOCK_NAMESPACE (block)->using;
 }
 
 /* Set BLOCK's using member to USING; if needed, allocate memory via
diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index 1945c22..171d5d0 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -384,6 +384,8 @@ finish_block (struct symbol *symbol, struct pending **listhead,
       opblock = pblock;
     }
 
+  block_set_using (block, using_directives, &objfile->objfile_obstack);
+
   record_pending_block (objfile, block, opblock);
 
   return block;
@@ -1202,10 +1204,12 @@ push_context (int desc, CORE_ADDR valu)
   new->params = param_symbols;
   new->old_blocks = pending_blocks;
   new->start_addr = valu;
+  new->using_directives = using_directives;
   new->name = NULL;
 
   local_symbols = NULL;
   param_symbols = NULL;
+  using_directives = NULL;
 
   return new;
 }
diff --git a/gdb/buildsym.h b/gdb/buildsym.h
index 294bc41..189fcec 100644
--- a/gdb/buildsym.h
+++ b/gdb/buildsym.h
@@ -125,6 +125,10 @@ EXTERN struct pending *local_symbols;
 
 EXTERN struct pending *param_symbols;
 
+/* using directives local to lexical context */
+
+EXTERN struct using_direct *using_directives;
+
 /* Stack representing unclosed lexical contexts (that will become
    blocks, eventually).  */
 
@@ -138,6 +142,10 @@ struct context_stack
 
     struct pending *params;
 
+    /* Pending using directives at the time we entered */
+
+    struct using_direct *using_directives;
+
     /* Pointer into blocklist as of entry */
 
     struct pending_block *old_blocks;
diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 4b45f6c..5830af6 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -35,11 +35,6 @@
 
 static struct using_direct *using_list;
 
-static struct using_direct *cp_add_using (const char *name,
-					  unsigned int inner_len,
-					  unsigned int outer_len,
-					  struct using_direct *next);
-
 static struct using_direct *cp_copy_usings (struct using_direct *using,
 					    struct obstack *obstack);
 
@@ -237,7 +232,7 @@ cp_is_anonymous (const char *namespace)
    using xmalloc.  It copies the strings, so NAME can be a temporary
    string.  */
 
-static struct using_direct *
+struct using_direct *
 cp_add_using (const char *name,
 	      unsigned int inner_len,
 	      unsigned int outer_len,
diff --git a/gdb/cp-support.h b/gdb/cp-support.h
index a004783..ce34022 100644
--- a/gdb/cp-support.h
+++ b/gdb/cp-support.h
@@ -80,6 +80,11 @@ extern void cp_add_using_directive (const char *name,
 				    unsigned int outer_length,
 				    unsigned int inner_length);
 
+extern struct using_direct *cp_add_using (const char *name,
+					  unsigned int inner_len,
+					  unsigned int outer_len,
+					  struct using_direct *next);
+
 extern void cp_initialize_namespace (void);
 
 extern void cp_finalize_namespace (struct block *static_block,
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 1b68e2a..1d82944 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -937,6 +937,8 @@ static void read_common_block (struct die_info *, struct dwarf2_cu *);
 
 static void read_namespace (struct die_info *die, struct dwarf2_cu *);
 
+static void read_import_statement (struct die_info *die, struct dwarf2_cu *);
+
 static const char *namespace_name (struct die_info *die,
 				   int *is_anonymous, struct dwarf2_cu *);
 
@@ -2756,14 +2758,12 @@ process_die (struct die_info *die, struct dwarf2_cu *cu)
       break;
     case DW_TAG_imported_declaration:
     case DW_TAG_imported_module:
-      /* FIXME: carlton/2002-10-16: Eventually, we should use the
-	 information contained in these.  DW_TAG_imported_declaration
-	 dies shouldn't have children; DW_TAG_imported_module dies
-	 shouldn't in the C++ case, but conceivably could in the
-	 Fortran case.  */
       processing_has_namespace_info = 1;
-      complaint (&symfile_complaints, _("unsupported tag: '%s'"),
-		 dwarf_tag_name (die->tag));
+      if (die->child != NULL && (die->tag == DW_TAG_imported_declaration
+				 || cu->language != language_fortran))
+	complaint (&symfile_complaints, _("Tag '%s' has unexpected children"),
+		   dwarf_tag_name (die->tag));
+      read_import_statement (die, cu);
       break;
     default:
       new_symbol (die, NULL, cu);
@@ -2809,6 +2809,38 @@ dwarf2_full_name (struct die_info *die, struct dwarf2_cu *cu)
   return name;
 }
 
+/* Read the import statement specified by the given die and record it.  */ 
+
+static void
+read_import_statement (struct die_info *die, struct dwarf2_cu *cu)
+{
+  struct attribute *import_attr;
+  struct die_info *imported_die;
+  const char *imported_name;
+  int is_anonymous = 0;
+  
+  import_attr = dwarf2_attr (die, DW_AT_import, cu);
+  if (import_attr == NULL)
+    {
+      complaint (&symfile_complaints, _("Tag '%s' has no DW_AT_import"),
+		 dwarf_tag_name (die->tag));
+      return;
+    }
+
+  imported_die = follow_die_ref (die, import_attr, &cu);
+  imported_name = namespace_name (imported_die, &is_anonymous, cu);
+  if (imported_name == NULL)
+    {
+      /* C++ imports from std:: DW_TAG_base_type with no DW_AT_name - why?  */
+      return;
+    }
+
+  /* FIXME: dwarf2_name (die); for the local name after import.  */
+  
+  using_directives = cp_add_using (imported_name, strlen (imported_name), 0,
+                                   using_directives);
+}
+
 static void
 initialize_cu_func_list (struct dwarf2_cu *cu)
 {
@@ -3043,6 +3075,7 @@ read_func_scope (struct die_info *die, struct dwarf2_cu *cu)
      back to building a containing block's symbol lists.  */
   local_symbols = new->locals;
   param_symbols = new->params;
+  using_directives = new->using_directives;
 
   /* If we've finished processing a top-level function, subsequent
      symbols go in the file symbol list.  */
@@ -3105,6 +3138,7 @@ read_lexical_block_scope (struct die_info *die, struct dwarf2_cu *cu)
       dwarf2_record_block_ranges (die, block, baseaddr, cu);
     }
   local_symbols = new->locals;
+  using_directives = new->using_directives;
 }
 
 /* Get low and high pc attributes from DW_AT_ranges attribute value OFFSET.
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index f078b70..88d233a 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2008-09-09  Sami Wagiaalla  <swagiaal@redhat.com>
+
+	* gdb.cp/namespace-using.cc: New test.
+	* gdb.cp/namespace-using.exp: New test.
+
 2008-09-09  Pedro Alves  <pedro@codesourcery.com>
 
 	* gdb.base/hook-stop-continue.c: New.
diff --git a/gdb/testsuite/gdb.cp/namespace-using.cc b/gdb/testsuite/gdb.cp/namespace-using.cc
new file mode 100644
index 0000000..562e50d
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/namespace-using.cc
@@ -0,0 +1,45 @@
+namespace A
+{
+  int a = 1;
+  int x = 2;
+}
+
+int marker4(){
+	using A::x;
+	return 0;
+}
+
+int marker3(){
+	return marker4();
+}
+
+int marker2()
+{
+  namespace B = A;
+  B::a;
+  return marker3();
+}
+
+int marker1()
+{
+  int total = 0;
+  {
+    int b = 1;
+    {
+      using namespace A;
+      int c = 2;
+      {
+        int d = 3;
+        total = a + b + c + d + marker2(); // marker1 stop
+      }
+    }
+  }
+  return total;
+}
+
+int main()
+{
+  using namespace A;
+  a;
+  return marker1();
+}
diff --git a/gdb/testsuite/gdb.cp/namespace-using.exp b/gdb/testsuite/gdb.cp/namespace-using.exp
new file mode 100644
index 0000000..9e161ba
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/namespace-using.exp
@@ -0,0 +1,135 @@
+# Copyright 2008 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+set prms_id 0
+set bug_id 0
+
+set testfile namespace-using
+set srcfile ${testfile}.cc
+set binfile ${objdir}/${subdir}/${testfile}
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug c++}] != "" } {
+    untested "Couldn't compile test program"
+    return -1
+}
+
+# Get things started.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+############################################
+# test printing of namespace imported within
+# the function.
+
+if ![runto_main] then {
+    perror "couldn't run to breakpoint main"
+    continue
+}
+
+send_gdb "print a\n"
+gdb_expect {
+   -re "\\$\[0-9\]* = 1" { pass "print a test1" }
+   -re ".*$gdb_prompt $" { fail "print a test1" }
+   timeout { fail "(timeout) print a" }
+}
+
+# Test that names are not printed when they 
+# are not imported
+
+send_gdb "break marker3\n"
+gdb_continue_to_breakpoint "marker3"
+
+send_gdb "print a\n"
+gdb_expect {
+   -re "\\$\[0-9\]* = 1" { fail "print a" }
+   -re ".*$gdb_prompt $" { pass "print a" }
+   timeout { fail "(timeout) print a" }
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+
+############################################
+# test printing of namespace imported into 
+# a scope containing the pc.
+
+if ![runto_main] then {
+    perror "couldn't run to breakpoint main"
+    continue
+}
+
+gdb_breakpoint [gdb_get_line_number "marker1 stop"]
+gdb_continue_to_breakpoint "marker1 stop"
+
+send_gdb "print a\n"
+gdb_expect {
+   -re "\\$\[0-9\]* = 1" { pass "print a" }
+   -re ".*$gdb_prompt $" { fail "print a" }
+   timeout { fail "(timeout) print a" }
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+
+############################################
+# Test printing of namespace aliases
+
+setup_kfail "gdb/380" "*-*-*"
+if ![runto marker2] then {
+    perror "couldn't run to breakpoint marker2"
+    continue
+}
+
+send_gdb "print B::a\n"
+gdb_expect {
+   -re "\\$\[0-9\]* = 1" { pass "print B::a" }
+   -re ".*$gdb_prompt $" { fail "print B::a" }
+   timeout { fail "(timeout) print B::a" }
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+
+############################################
+# Test printing of namespace aliases
+
+setup_kfail "gdb/381" "*-*-*"
+if ![runto marker4] then {
+    perror "couldn't run to breakpoint marker2"
+    continue
+}
+
+send_gdb "print x\n"
+gdb_expect {
+   -re "\\$\[0-9\]* = 2" { pass "print x" }
+   -re ".*$gdb_prompt $" { fail "print x" }
+   timeout { fail "(timeout) print x" }
+}
+

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

* Re: using directive patch
  2008-09-10 19:03 ` using directive patch Sami Wagiaalla
@ 2008-09-16 16:54   ` Tom Tromey
  0 siblings, 0 replies; 15+ messages in thread
From: Tom Tromey @ 2008-09-16 16:54 UTC (permalink / raw)
  To: Sami Wagiaalla; +Cc: Project Archer

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

Sami> Here is a second version of the patch after incorporating the
Sami> suggestions and adding a test case.

Looking good.

Sami> +  if (imported_name == NULL)
Sami> +    {
Sami> +      /* C++ imports from std:: DW_TAG_base_type with no DW_AT_name - why?  */
Sami> +      return;

I'm curious about this comment -- could you say more?
Is this a g++ bug?

Sami> +send_gdb "print a\n"
Sami> +gdb_expect {
Sami> +   -re "\\$\[0-9\]* = 1" { pass "print a" }
Sami> +   -re ".*$gdb_prompt $" { fail "print a" }
Sami> +   timeout { fail "(timeout) print a" }
Sami> +}

You might like gdb_test, which makes it somewhat simpler to write a
send-and-response sequence like this.

Tom

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

* DW_TAG_imported_declaration DW_TAG_base_type w/o DW_AT_name  [Re: using directive patch]
  2008-09-07 18:37 ` Jan Kratochvil
  2008-09-08 15:16   ` Sami Wagiaalla
@ 2008-09-18 21:14   ` Jan Kratochvil
  2009-06-16 15:14     ` Sami Wagiaalla
  1 sibling, 1 reply; 15+ messages in thread
From: Jan Kratochvil @ 2008-09-18 21:14 UTC (permalink / raw)
  To: Sami Wagiaalla; +Cc: Project Archer, Dodji Seketeli

On Sun, 07 Sep 2008 20:36:12 +0200, Jan Kratochvil wrote:
> +/* Read the import statement specified by the given die and record it.  */ 
> +
> +static void
> +read_import_statement (struct die_info *die, struct dwarf2_cu *cu)
> +{
...
> +  import_attr = dwarf2_attr (die, DW_AT_import, cu);
...
> +  imported_die = follow_die_ref (die, import_attr, &cu);
> +  imported_name = dwarf2_name (imported_die, cu);
> +  if (imported_name == NULL)
> +    {
> +      /* C++ imports from std:: DW_TAG_base_type with no DW_AT_name - why?  */
> +      return;
> +    }

As discussed on the meeting call the reasons for this exception:

#include <iostream>
using namespace std;
int main () { return 0; }

g++ -g -o std std.C 
gcc-4.3.2-3.x86_64

I get 37KB of ELF with:

(1) Many never-used DW_TAG_imported_declaration DIEs - IMO GCC PR debug/14168.

    But they make some sense as one should be able to access std:: items at
    least from the debugger.
    
    They need to be defined in DWARF - we cannot import whole "std::" by
    a single DIE as the debugger does not parse the header files.  Still such
    imports should be shared across Compilation Units (by a tool doing
    -feliminate-dwarf2-dups right).

(2) DW_TAG_imported_declaration -> DW_TAG_base_type with no DW_AT_name:

 <1><46>: Abbrev Number: 4 (DW_TAG_namespace)
    <47>   DW_AT_name        : std      
 <2><51>: Abbrev Number: 5 (DW_TAG_imported_declaration)
    <54>   DW_AT_import      : <0x50e>  [Abbrev Number: 24 (DW_TAG_base_type)]
 <1><50e>: Abbrev Number: 24 (DW_TAG_base_type)
    <50f>   DW_AT_byte_size   : 8       
    <510>   DW_AT_encoding    : 5       (signed)

    I do not see a meaning of such import - if it has no name.  But this bug
    may be already covered by PR debug/14168 or possibly by PR debug/14169.


Regards,
Jan

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

* Re: DW_TAG_imported_declaration DW_TAG_base_type w/o DW_AT_name  [Re: using directive patch]
  2008-09-18 21:14   ` DW_TAG_imported_declaration DW_TAG_base_type w/o DW_AT_name [Re: using directive patch] Jan Kratochvil
@ 2009-06-16 15:14     ` Sami Wagiaalla
  2009-06-16 18:16       ` Tom Tromey
  0 siblings, 1 reply; 15+ messages in thread
From: Sami Wagiaalla @ 2009-06-16 15:14 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Project Archer, Dodji Seketeli

Jan Kratochvil wrote:
> On Sun, 07 Sep 2008 20:36:12 +0200, Jan Kratochvil wrote:
>> +/* Read the import statement specified by the given die and record it.  */ 
>> +
>> +static void
>> +read_import_statement (struct die_info *die, struct dwarf2_cu *cu)
>> +{
> ...
>> +  import_attr = dwarf2_attr (die, DW_AT_import, cu);
> ...
>> +  imported_die = follow_die_ref (die, import_attr, &cu);
>> +  imported_name = dwarf2_name (imported_die, cu);
>> +  if (imported_name == NULL)
>> +    {
>> +      /* C++ imports from std:: DW_TAG_base_type with no DW_AT_name - why?  */
>> +      return;
>> +    }

> (2) DW_TAG_imported_declaration -> DW_TAG_base_type with no DW_AT_name:
> 
>  <1><46>: Abbrev Number: 4 (DW_TAG_namespace)
>     <47>   DW_AT_name        : std      
>  <2><51>: Abbrev Number: 5 (DW_TAG_imported_declaration)
>     <54>   DW_AT_import      : <0x50e>  [Abbrev Number: 24 (DW_TAG_base_type)]
>  <1><50e>: Abbrev Number: 24 (DW_TAG_base_type)
>     <50f>   DW_AT_byte_size   : 8       
>     <510>   DW_AT_encoding    : 5       (signed)
> 
>     I do not see a meaning of such import - if it has no name.  But this bug
>     may be already covered by PR debug/14168 or possibly by PR debug/14169.
> 

Is this a bug though. This imports the type specified by 0x50e keeping its
name in the current context.

like here:

namespace A{
   class B{
   public:
     int	x;
   };
}

int main(){
   using A::B;
   B b;
   return 0;
}

...
  <2><7b>: Abbrev Number: 8 (DW_TAG_imported_declaration)
     <7c>   DW_AT_decl_file   : 1
     <7d>   DW_AT_decl_line   : 9
     <7e>   DW_AT_import      : <0x3b>   [Abbrev Number: 4 (DW_TAG_class_type)]
...


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

* Re: DW_TAG_imported_declaration DW_TAG_base_type w/o DW_AT_name  [Re: using directive patch]
  2009-06-16 15:14     ` Sami Wagiaalla
@ 2009-06-16 18:16       ` Tom Tromey
  2009-06-16 20:38         ` Jan Kratochvil
  0 siblings, 1 reply; 15+ messages in thread
From: Tom Tromey @ 2009-06-16 18:16 UTC (permalink / raw)
  To: Sami Wagiaalla; +Cc: Jan Kratochvil, Project Archer, Dodji Seketeli

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

Jan> I do not see a meaning of such import - if it has no name.  But this bug
Jan> may be already covered by PR debug/14168 or possibly by PR debug/14169.

Sami> Is this a bug though. This imports the type specified by 0x50e keeping its
Sami> name in the current context.

Yeah, see Dwarf 3 section 3.2.3.  It is clear about the meaning of a
missing DW_AT_name in this situation.

Tom

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

* Re: DW_TAG_imported_declaration DW_TAG_base_type w/o DW_AT_name [Re: using directive patch]
  2009-06-16 18:16       ` Tom Tromey
@ 2009-06-16 20:38         ` Jan Kratochvil
  2009-06-17 15:02           ` Sami Wagiaalla
  2009-06-17 16:42           ` Tom Tromey
  0 siblings, 2 replies; 15+ messages in thread
From: Jan Kratochvil @ 2009-06-16 20:38 UTC (permalink / raw)
  To: Sami Wagiaalla, Tom Tromey; +Cc: Project Archer, Dodji Seketeli

On Tue, 16 Jun 2009 17:12:39 +0200, Sami Wagiaalla wrote:
> Jan Kratochvil wrote:
>> On Sun, 07 Sep 2008 20:36:12 +0200, Jan Kratochvil wrote:
>>> +/* Read the import statement specified by the given die and record 
>>> it.  */ +
>>> +static void
>>> +read_import_statement (struct die_info *die, struct dwarf2_cu *cu)
>>> +{
>> ...
>>> +  import_attr = dwarf2_attr (die, DW_AT_import, cu);
>> ...
>>> +  imported_die = follow_die_ref (die, import_attr, &cu);
>>> +  imported_name = dwarf2_name (imported_die, cu);

The important difference here is "imported_die".  It is not "die" here.

>>> +  if (imported_name == NULL)
>>> +    {
>>> +      /* C++ imports from std:: DW_TAG_base_type with no DW_AT_name - why?  */
>>> +      return;
>>> +    }
>
>> (2) DW_TAG_imported_declaration -> DW_TAG_base_type with no DW_AT_name:
>>
>>  <1><46>: Abbrev Number: 4 (DW_TAG_namespace)
>>     <47>   DW_AT_name        : std
>>  <2><51>: Abbrev Number: 5 (DW_TAG_imported_declaration)
>>     <54>   DW_AT_import      : <0x50e>  [Abbrev Number: 24 (DW_TAG_base_type)]
>>  <1><50e>: Abbrev Number: 24 (DW_TAG_base_type)
>>     <50f>   DW_AT_byte_size   : 8
>>     <510>   DW_AT_encoding    : 5       (signed)
>>
>>     I do not see a meaning of such import - if it has no name.  But this bug
>>     may be already covered by PR debug/14168 or possibly by PR debug/14169.
>>
>
> Is this a bug though. This imports the type specified by 0x50e keeping its
> name in the current context.
>
> like here:
>
> namespace A{
>   class B{
>   public:
>     int	x;
>   };
> }
>
> int main(){
>   using A::B;
>   B b;
>   return 0;
> }
>
> ...
>  <2><7b>: Abbrev Number: 8 (DW_TAG_imported_declaration)
>     <7c>   DW_AT_decl_file   : 1
>     <7d>   DW_AT_decl_line   : 9
>     <7e>   DW_AT_import      : <0x3b>   [Abbrev Number: 4 (DW_TAG_class_type)]
> ...

This case is different.  Full relevant DWARF part dump of it:
    < c>   DW_AT_producer    : (indirect string, offset: 0x0): GNU C++ 4.4.0 20090506 (Red Hat 4.4.0-4)
 <3><8c>: Abbrev Number: 9 (DW_TAG_imported_declaration)
    <8f>   DW_AT_import      : <0x3b>   [Abbrev Number: 4 (DW_TAG_class_type)]
 <1><3b>: Abbrev Number: 4 (DW_TAG_class_type)
    <3c>   DW_AT_specification: <0x36>
    <40>   DW_AT_byte_size   : 4
 <2><36>: Abbrev Number: 3 (DW_TAG_class_type)
    <37>   DW_AT_name        : B
    <39>   DW_AT_declaration : 1
(removed DW_AT_decl_file, DW_AT_decl_line, DW_AT_sibling in this mail)

Results in:
	die = <8c>
	imported_die = <3b>
	imported_name = "B"
so the import gets processed, doesn't it?  Do you have a reproducer of
incorrect GDB behavior?


That code:
+  if (imported_name == NULL)
+    {
+      /* C++ imports from std:: DW_TAG_base_type with no DW_AT_name - why?  */
+      return;
+    }

was there to handle the case I can find in (in fact arbitrary C++ code):
firefox-debuginfo-3.5-0.20.beta4.fc11.x86_64
/usr/lib/debug/usr/lib64/firefox-3.5b4/components/libbrowsercomps.so.debug
    < c>   DW_AT_producer    : (indirect string, offset: 0x121d): GNU C++ 4.4.0 20090427 (Red Hat 4.4.0-3)
 <1><e6d>: Abbrev Number: 48 (DW_TAG_namespace)
    <e6e>   DW_AT_name        : std
    <e74>   DW_AT_sibling     : <0xe87>
 <2><e78>: Abbrev Number: 49 (DW_TAG_imported_declaration)
    <e7b>   DW_AT_import      : <0xe87> [Abbrev Number: 6 (DW_TAG_base_type)]
 <1><e87>: Abbrev Number: 6 (DW_TAG_base_type)
    <e88>   DW_AT_byte_size   : 8
    <e89>   DW_AT_encoding    : 5       (signed)

In this case here is DW_AT_name in neither <e78> nor <e87>.  I still do not
understand meaning of the DIE <e78>.

Sure DIE <e87> is there to be referenced by other DIEs by its offset.  Just in
this file neither <e78> nor <e87> is referenced so they are IMO
excessive/unused there.  IIRC there is a GCC Bug for it but cannot find it.



On Tue, 16 Jun 2009 20:16:16 +0200, Tom Tromey wrote:
> >>>>> "Sami" == Sami Wagiaalla <swagiaal@redhat.com> writes:
> 
> Jan> I do not see a meaning of such import - if it has no name.  But this bug
> Jan> may be already covered by PR debug/14168 or possibly by PR debug/14169.
> 
> Sami> Is this a bug though. This imports the type specified by 0x50e keeping its
> Sami> name in the current context.
> 
> Yeah, see Dwarf 3 section 3.2.3.  It is clear about the meaning of a
> missing DW_AT_name in this situation.

DWARF3:
# An imported declaration may also have a DW_AT_name attribute whose value is
# a null-terminated string containing the name, as it appears in the source
# program, by which the imported entity is to be known in the context of the
# imported declaration entry (which may be different than the name of the
# entity being imported). If no name is present, then the name by which the
# entity is to be known is the same as the name of the entity being imported.  

DW_AT_name present at this DIE?
                     DW_TAG_imported_declaration  DW_TAG_base_type
A (invalid)          anonymous                    anonymous
B (standard import)  anonymous                    present
C (unhandled now)    present                      anonymous
D (renamed import)   present                      present

if (imported_name == NULL) catches A and C.  A should be caught as invalid.
It is true C should be parsed correctly but fortunately I do not know about
any existing DWARF files containing the C case (there may be some)

Sami's "namespace A" example above is the case B.

My case from firefox-debuginfo above is the case A.



Thanks,
Jan

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

* Re: DW_TAG_imported_declaration DW_TAG_base_type w/o DW_AT_name [Re: using directive patch]
  2009-06-16 20:38         ` Jan Kratochvil
@ 2009-06-17 15:02           ` Sami Wagiaalla
  2009-06-17 16:42           ` Tom Tromey
  1 sibling, 0 replies; 15+ messages in thread
From: Sami Wagiaalla @ 2009-06-17 15:02 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Tom Tromey, Project Archer, Dodji Seketeli

> Results in:
> 	die = <8c>
> 	imported_die = <3b>
> 	imported_name = "B"
> so the import gets processed, doesn't it?

Yes agreed... bad code example.

> Do you have a reproducer of incorrect GDB behavior?

Yes. Upon further digging I found the gcc bug.
Here is a much better example:

namespace A{
   typedef int B;
}

int main(){
   using A::B;
   B b;
   return b;
}

...
  <2><51>: Abbrev Number: 3 (DW_TAG_imported_declaration)
     <52>   DW_AT_decl_file   : 1
     <53>   DW_AT_decl_line   : 6
     <54>   DW_AT_import      : <0x75>
  <2><58>: Abbrev Number: 4 (DW_TAG_typedef)
     <59>   DW_AT_name        : B
     <5b>   DW_AT_decl_file   : 1
     <5c>   DW_AT_decl_line   : 2
     <5d>   DW_AT_type        : <0x6e>
...
  <1><75>: Abbrev Number: 7 (DW_TAG_base_type)
     <76>   DW_AT_byte_size   : 4
     <77>   DW_AT_encoding    : 5        (signed)

Import from line 6 is not importing 0x75 as gcc reports. It is importing 
0x58. In which case the imported_name would be B.

Will open a gcc bug :)

Sami

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

* Re: DW_TAG_imported_declaration DW_TAG_base_type w/o DW_AT_name [Re: using directive patch]
  2009-06-16 20:38         ` Jan Kratochvil
  2009-06-17 15:02           ` Sami Wagiaalla
@ 2009-06-17 16:42           ` Tom Tromey
  2009-06-19 16:46             ` Jan Kratochvil
  1 sibling, 1 reply; 15+ messages in thread
From: Tom Tromey @ 2009-06-17 16:42 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Sami Wagiaalla, Project Archer, Dodji Seketeli

Jan> DW_AT_name present at this DIE?
Jan>                      DW_TAG_imported_declaration  DW_TAG_base_type
Jan> A (invalid)          anonymous                    anonymous
Jan> B (standard import)  anonymous                    present
Jan> C (unhandled now)    present                      anonymous
Jan> D (renamed import)   present                      present

Thanks for this.  I see I was not fully understanding the code.
That comment should explain the failing case more clearly, like this.

Tom

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

* Re: DW_TAG_imported_declaration DW_TAG_base_type w/o DW_AT_name [Re: using directive patch]
  2009-06-17 16:42           ` Tom Tromey
@ 2009-06-19 16:46             ` Jan Kratochvil
  0 siblings, 0 replies; 15+ messages in thread
From: Jan Kratochvil @ 2009-06-19 16:46 UTC (permalink / raw)
  To: Sami Wagiaalla; +Cc: Project Archer, Dodji Seketeli, Tom Tromey

Hi Sami,

On Wed, 17 Jun 2009 18:42:24 +0200, Tom Tromey wrote:
> Thanks for this.  I see I was not fully understanding the code.
> That comment should explain the failing case more clearly, like this.

please check it into the appropriate branches
(archer-swagiaal-using-directive, archer-keiths-expr-cumulative, assuming the
FSF GDB post will be updated anyway).


Thanks,
Jan


gdb/
2009-06-19  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* dwarf2read.c (read_import_statement <imported_name == NULL>): Extend
	the comment.

--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -2966,7 +2966,18 @@ read_import_statement (struct die_info *die, struct dwarf2_cu *cu)
   imported_name = namespace_name (imported_die, &is_anonymous, cu);
   if (imported_name == NULL)
     {
-      /* C++ imports from std:: DW_TAG_base_type with no DW_AT_name - why?  */
+      /*                      DW_TAG_imported_declaration  DW_TAG_base_type
+			      variable DIE                 variable IMPORTED_DIE
+	 A (invalid)          anonymous                    anonymous
+	 B (standard import)  anonymous                    DW_AT_name present
+	 C (unhandled now)    DW_AT_name present           anonymous
+	 D (renamed import)   DW_AT_name present           DW_AT_name present
+
+	 The NULL condition for this block of code catches cases A and C.
+
+	 FIXME: Case C should be fixed below to be parsed correctly.  There are
+	 AFAIK no existing DWARF files containing such C case.  */
+
       return;
     }
 

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

end of thread, other threads:[~2009-06-19 16:46 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-09-03 18:07 using directive patch Sami Wagiaalla
2008-09-05 19:15 ` Tom Tromey
2008-09-08 15:18   ` Sami Wagiaalla
2008-09-07 18:37 ` Jan Kratochvil
2008-09-08 15:16   ` Sami Wagiaalla
2008-09-08 15:31     ` Jan Kratochvil
2008-09-18 21:14   ` DW_TAG_imported_declaration DW_TAG_base_type w/o DW_AT_name [Re: using directive patch] Jan Kratochvil
2009-06-16 15:14     ` Sami Wagiaalla
2009-06-16 18:16       ` Tom Tromey
2009-06-16 20:38         ` Jan Kratochvil
2009-06-17 15:02           ` Sami Wagiaalla
2009-06-17 16:42           ` Tom Tromey
2009-06-19 16:46             ` Jan Kratochvil
2008-09-10 19:03 ` using directive patch Sami Wagiaalla
2008-09-16 16:54   ` 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).