Bug 37410 - DW_TAG_imported_module is not in its DW_TAG_lexical_block
Summary: DW_TAG_imported_module is not in its DW_TAG_lexical_block
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: debug (show other bugs)
Version: 4.3.2
: P3 minor
Target Milestone: ---
Assignee: Dodji Seketeli
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-09-07 17:55 UTC by Jan Kratochvil
Modified: 2008-10-05 22:54 UTC (History)
5 users (show)

See Also:
Host: x86_64-fedora-linux-gnu
Target: x86_64-fedora-linux-gnu
Build: x86_64-fedora-linux-gnu
Known to work:
Known to fail:
Last reconfirmed:


Attachments
Testcase. (178 bytes, text/plain)
2008-09-07 17:56 UTC, Jan Kratochvil
Details
first fix candidate (3.40 KB, patch)
2008-09-23 13:04 UTC, Dodji Seketeli
Details | Diff
Second version of the patch (4.02 KB, patch)
2008-10-01 09:31 UTC, Dodji Seketeli
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Kratochvil 2008-09-07 17:55:27 UTC
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 Jan Kratochvil 2008-09-07 17:56:13 UTC
Created attachment 16249 [details]
Testcase.
Comment 2 Dodji Seketeli 2008-09-23 13:04:45 UTC
Created attachment 16393 [details]
first fix candidate

This patch seems to fix the problem for me and passes regtests on trunk.
Comment 3 Dodji Seketeli 2008-09-23 13:08:31 UTC
Comment on attachment 16393 [details]
first fix candidate

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 Jakub Jelinek 2008-09-23 18:48:06 UTC
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 Dodji Seketeli 2008-10-01 09:31:26 UTC
Created attachment 16438 [details]
Second version of the patch

@Jakub: Please find attached a second version of the patch that should address your comments. Thanks.
Comment 6 Jason Merrill 2008-10-01 18:01:53 UTC
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 Dodji Seketeli 2008-10-02 12:21:16 UTC
@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 Dodji Seketeli 2008-10-05 21:30:54 UTC
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 Dodji Seketeli 2008-10-05 22:54:28 UTC
Fixed in trunk