Bug List: (This bug is not in your last search results)   Show last search results      Search page      Enter new bug
Bug#: 37410
Product:  
Component:  
Status: RESOLVED
Resolution: FIXED
Assigned To: Dodji Seketeli <dodji@gcc.gnu.org>
Host:
Reported against  
Priority:  
Severity:  
Target Milestone:  
 
 
Target:
Reporter: Jan Kratochvil <jan.kratochvil@redhat.com>
Add CC:
CC:
Remove selected CCs
Build:
URL:
Summary:
Keywords:
Known to work:
Known to fail:

Attachment Description Type Created Size Actions
namespace-inherit.C Testcase. text/plain 2008-09-07 17:56 178 bytes Edit
PR37410-patch-0.txt first fix candidate patch 2008-09-23 13:04 3.40 KB Edit | Diff
PR37410-patch.txt Second version of the patch patch 2008-10-01 09:31 4.02 KB Edit | Diff
Create a New Attachment (proposed patch, testcase, etc.) View All

Bug 37410 depends on: Show dependency tree
Show dependency graph
Bug 37410 blocks:

Additional Comments:






View Bug Activity   |   Format For Printing   |   Clone This Bug


Description:   Last confirmed: Opened: 2008-09-07 17:55
DW_TAG_imported_module (c++ `using namespace') has validity only in the same
block in which they are stated.  It works right for the compiled code but it
gets defined for the whole function in DWARF.

DW_TAG_imported_module should be located in DW_TAG_lexical_block.  Without the
`int block_create;'  declaration the whole DW_TAG_lexical_block is missing
there so it should be created when needed.

The testcase produces:
1
2

Tests on Fedora:
compat-gcc-34-c++-3.4.6-9.x86_64 has no DW_TAG_imported_module there at all.
gcc-4.3.2-3.x86_64 and gcc-4.1.2-33.x86_64 produce the output below.

 <1><6f>: Abbrev Number: 5 (DW_TAG_subprogram)
    <70>   DW_AT_external    : 1        
    <71>   DW_AT_name        : (indirect string, offset: 0x7f): main    
    <75>   DW_AT_decl_file   : 1        
    <76>   DW_AT_decl_line   : 13       
    <77>   DW_AT_type        : <0x57>   
    <7b>   DW_AT_low_pc      : 0x4005ac 
    <83>   DW_AT_high_pc     : 0x4005fc 
    <8b>   DW_AT_frame_base  : 0x0      (location list)
    <8f>   DW_AT_sibling     : <0xf2>   
 <2><93>: Abbrev Number: 6 (DW_TAG_imported_module)
    <94>   DW_AT_decl_file   : 1        
    <95>   DW_AT_decl_line   : 19       
    <96>   DW_AT_import      : <0xf2>   [Abbrev Number: 11 (DW_TAG_namespace)]
 <2><9a>: Abbrev Number: 6 (DW_TAG_imported_module)
    <9b>   DW_AT_decl_file   : 1        
    <9c>   DW_AT_decl_line   : 27       
    <9d>   DW_AT_import      : <0x10e>  [Abbrev Number: 11 (DW_TAG_namespace)]
 <2><a1>: Abbrev Number: 7 (DW_TAG_variable)
    <a2>   DW_AT_name        : x        
    <a4>   DW_AT_decl_file   : 1        
    <a5>   DW_AT_decl_line   : 15       
    <a6>   DW_AT_type        : <0x57>   
    <aa>   DW_AT_location    : 2 byte block: 91 64      (DW_OP_fbreg: -28)
 <2><ad>: Abbrev Number: 8 (DW_TAG_lexical_block)
    <ae>   DW_AT_low_pc      : 0x4005b4 
    <b6>   DW_AT_high_pc     : 0x4005d2 
    <be>   DW_AT_sibling     : <0xd1>   
 <3><c2>: Abbrev Number: 9 (DW_TAG_variable)  
    <c3>   DW_AT_name        : (indirect string, offset: 0x32): block_create    
    <c7>   DW_AT_decl_file   : 1        
    <c8>   DW_AT_decl_line   : 18       
    <c9>   DW_AT_type        : <0x57>   
    <cd>   DW_AT_location    : 2 byte block: 91 68      (DW_OP_fbreg: -24)
 <2><d1>: Abbrev Number: 10 (DW_TAG_lexical_block)
    <d2>   DW_AT_low_pc      : 0x4005d2 
    <da>   DW_AT_high_pc     : 0x4005f0 
 <3><e2>: Abbrev Number: 9 (DW_TAG_variable)
    <e3>   DW_AT_name        : (indirect string, offset: 0x32): block_create    
    <e7>   DW_AT_decl_file   : 1        
    <e8>   DW_AT_decl_line   : 26       
    <e9>   DW_AT_type        : <0x57>   
    <ed>   DW_AT_location    : 2 byte block: 91 6c      (DW_OP_fbreg: -20)

------- Comment #1 From Jan Kratochvil 2008-09-07 17:56 -------
Created an attachment (id=16249) [edit]
Testcase.

------- Comment #2 From Dodji Seketeli 2008-09-23 13:04 -------
Created an attachment (id=16393) [edit]
first fix candidate

This patch seems to fix the problem for me and passes regtests on trunk.

------- Comment #3 From Dodji Seketeli 2008-09-23 13:08 -------
(From update of attachment 16393 [edit])
The idea of the patch is to add an IMPORTED_DECL node to the lexical BLOCK
enclosing the "using directive".This is done at gimplification time.

Then the patch modifies dwarf2out.c to recognize the new IMPORTED_DECL node and
generate the DW_TAG_imported_module dwarf tag accordingly.

------- Comment #4 From Jakub Jelinek 2008-09-23 18:48 -------
Just a few nits:
1) why do you need to call nreverse?  I don't think the order of vars in
BLOCK_VARS matters, and when you call nreverse you reorder them all but the one
you added anyway
2) have you checked that gimple_current_bind_expr + gimple_bind_block is
sufficient?  I believe during gimplification even artificial, block-less
GIMPLE_BINDs can be added.  So I think more robust would be to call
gimple_bind_expr_stack (), and walk the vector from the last GIMPLE_BIND in it
up to the first, stopping when you find a GIMPLE_BIND with non-NULL
gimple_bind_block.
3) I think usually gcc uses _1 suffixed helper functions instead of _real
4) I think defining IMPORTED_DECL_NS_NAME is overkill, just use DECL_NAME
5) there are a few minor formatting issues, e.g. missing space in *expr_p,0
   or
    IMPORTED_DECL_NS_DECL (using_directive) =
                             TREE_OPERAND (*expr_p, 0)
   (where
    IMPORTED_DECL_NS_DECL (using_directive)
      = TREE_OPERAND (*expr_p, 0);
   should be written).

------- Comment #5 From Dodji Seketeli 2008-10-01 09:31 -------
Created an attachment (id=16438) [edit]
Second version of the patch

@Jakub: Please find attached a second version of the patch that should address
your comments. Thanks.

------- Comment #6 From Jason Merrill 2008-10-01 18:01 -------
Subject: Re:  DW_TAG_imported_module is not in its DW_TAG_lexical_block

Please send patches to gcc-patches for review (and CC me) rather than 
attaching them to the PR.  (It would be nice if a bot would notice 
relevant subject lines on gcc-patches and add a link to the PR like we 
do for checkins).

I wouldn't limit the new IMPORTED_DECL to importing namespaces; as a 
backend tree code it should support importing other declarations as well 
even though C++ doesn't allow that usage.  That is, the comment in 
tree.def and the name of IMPORTED_DECL_NS_DECL should be more general, 
and the assert in dwarf2out that the imported decl is a namespace should 
either go away or get a comment that a language that wants to use it for 
other types of decl should make sure it works properly.

> +      /* Get the innermost inclosing GIMPLE_BIND that has a non NULL
> +         BLOCK, and append an IMPORTED_DECL not to its
> +	 BLOCK_VARS chained list.  */

"not"?

> --- a/gcc/cp/name-lookup.c
> +++ b/gcc/cp/name-lookup.c
> @@ -3493,11 +3493,6 @@ do_using_directive (tree name_space)
>        if (current_namespace != global_namespace)
>  	context = current_namespace;
>      }
> -
> -  /* Emit debugging info.  */
> -  if (!processing_template_decl)
> -    (*debug_hooks->imported_module_or_decl) (name_space, NULL_TREE,
> -					     context, false);
>  }

Seems like this will break debug info for namespace-scope using-directives.

Jason

------- Comment #7 From Dodji Seketeli 2008-10-02 12:21 -------
@jason: Okay, I have posted the patch that I hope should address your comments.
Sorry to not have posted the patch there earlier. I was not sure about the
approach and didn't want to add too much noise to gcc-patches with something I
was not sure about myself.

------- Comment #8 From Dodji Seketeli 2008-10-05 21:30 -------
Subject: Bug 37410

Author: dodji
Date: Sun Oct  5 21:29:32 2008
New Revision: 140895

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=140895
Log:
2008-09-30  Dodji Seketeli  <dodji@redhat.com>
gcc/ChangeLog:

        PR c++/37410
        * dwarf2out.c (dwarf2out_imported_module_or_decl): Split this
          function in two, making it call a new and reusable
          dwarf2out_imported_module_or_decl() that takes the containing
          BLOCK of the declaration in argument.
          (dwarf2out_imported_module_or_decl_real): New function.
          (decls_for_scope, gen_decl_die, dwarf2out_decl): Take
          IMPORTED_DECL in account.
        * tree.def: Added IMPORTED_DECL node type.
        * tree.h: Added accessors for IMPORTED_DECL nodes.
        * tree.c (init_ttree): Initialise IMPORTED_DECL node type.

gcc/cp/ChangeLog:

        PR c++/37410
        * cp-gimplify.c (cp_gimplify_expr): For each USING_STMT
          make sure an IMPORTED_DECL node is added to the BLOCK_VARS list
          of the innermost containing BLOCK.

gcc/testsuite/ChangeLog:

        PR c++/37410
        * g++.dg/debug/dwarf2/imported-module.C: New test.


Added:
    trunk/gcc/testsuite/g++.dg/debug/dwarf2/imported-module.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/cp-gimplify.c
    trunk/gcc/cp/name-lookup.c
    trunk/gcc/dwarf2out.c
    trunk/gcc/print-tree.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree.c
    trunk/gcc/tree.def
    trunk/gcc/tree.h

------- Comment #9 From Dodji Seketeli 2008-10-05 22:54 -------
Fixed in trunk

Bug List: (This bug is not in your last search results)   Show last search results      Search page      Enter new bug