On Thu, Sep 27, 2018 at 3:28 AM, Richard Biener <rguent...@suse.de> wrote:
> On Wed, 26 Sep 2018, Jason Merrill wrote:
>
>> On Wed, Sep 26, 2018 at 9:35 AM, Richard Biener <rguent...@suse.de> wrote:
>> >
>> > We do not create a DW_AT_lexical_block for the outermost block in
>> > functions but we do for DW_AT_inlined_subroutines. That makes
>> > debuginfo look like if there were two of each local, the outer
>> > one (from the abstract instance) optimized out (visible via
>> > info locals in gdb).
>> >
>> > The following elides the outermost block also from inline instances.
>> > It's a bit tricky to reliably track that block given we remove unused
>> > blocks here and there. The trick is to have the block in the abstract
>> > instance _not_ point to itself (given we do not output it it isn't
>> > the abstract origin for itself).
>> >
>> > Bootstrapped on x86_64-unkown-linux-gnu, testing in progress.
>> >
>> > Again with some scan-assembler testcase, guality cannot do 'info locals'.
>> >
>> > OK?
>> >
>> > Thanks,
>> > Richard.
>> >
>> > 2018-09-26 Richard Biener <rguent...@suse.de>
>> >
>> > PR debug/87440
>> > * dwarf2out.c (set_block_origin_self): Do not mark outermost
>> > block as we do not output that.
>> > (gen_inlined_subroutine_die): Elide the originally outermost
>> > block, matching what we do for concrete instances.
>> > (decls_for_scope): Add parameter specifying whether to recurse
>> > to subblocks.
>> >
>> > * gcc.dg/debug/dwarf2/inline4.c: New testcase.
>> >
>> > Index: gcc/testsuite/gcc.dg/debug/dwarf2/inline4.c
>> > ===================================================================
>> > --- gcc/testsuite/gcc.dg/debug/dwarf2/inline4.c (nonexistent)
>> > +++ gcc/testsuite/gcc.dg/debug/dwarf2/inline4.c (working copy)
>> > @@ -0,0 +1,17 @@
>> > +/* Verify that the inline instance has no extra DW_TAG_lexical_block
>> > between
>> > + the DW_TAG_inlined_subroutine and the DW_TAG_variable for the local.
>> > */
>> > +/* { dg-options "-O -gdwarf -dA" } */
>> > +/* { dg-do compile } */
>> > +/* { dg-final { scan-assembler
>> > "DW_TAG_inlined_subroutine\[^\\(\]*\\(\[^\\)\]*\\)\[^\\(\]*\\(DIE
>> > \\(0x\[0-9a-f\]*\\) DW_TAG_formal_parameter\[^\\(\]*\\(DIE
>> > \\(0x\[0-9a-f\]*\\) DW_TAG_variable" } } */
>> > +/* { dg-final { scan-assembler-times "DW_TAG_inlined_subroutine" 2 } } */
>> > +
>> > +static int foo (int i)
>> > +{
>> > + volatile int j = i + 3;
>> > + return j - 2;
>> > +}
>> > +int main()
>> > +{
>> > + volatile int z = foo (-1);
>> > + return z;
>> > +}
>> > Index: gcc/dwarf2out.c
>> > ===================================================================
>> > --- gcc/dwarf2out.c (revision 264640)
>> > +++ gcc/dwarf2out.c (working copy)
>> > @@ -3867,7 +3867,7 @@ static void gen_subroutine_type_die (tre
>> > static void gen_typedef_die (tree, dw_die_ref);
>> > static void gen_type_die (tree, dw_die_ref);
>> > static void gen_block_die (tree, dw_die_ref);
>> > -static void decls_for_scope (tree, dw_die_ref);
>> > +static void decls_for_scope (tree, dw_die_ref, bool = true);
>> > static bool is_naming_typedef_decl (const_tree);
>> > static inline dw_die_ref get_context_die (tree);
>> > static void gen_namespace_die (tree, dw_die_ref);
>> > @@ -22389,7 +22389,13 @@ set_block_origin_self (tree stmt)
>> > {
>> > if (BLOCK_ABSTRACT_ORIGIN (stmt) == NULL_TREE)
>> > {
>> > - BLOCK_ABSTRACT_ORIGIN (stmt) = stmt;
>> > + /* We do not mark the outermost block as we are not outputting it.
>> > + This is then a reliable way of determining whether we should
>> > + do the same for an inline instance. */
>> > + if (TREE_CODE (BLOCK_SUPERCONTEXT (stmt)) != FUNCTION_DECL)
>> > + BLOCK_ABSTRACT_ORIGIN (stmt) = stmt;
>> > + else
>> > + gcc_assert (DECL_INITIAL (BLOCK_SUPERCONTEXT (stmt)) == stmt);
>> >
>> > {
>> > tree local_decl;
>> > @@ -24149,7 +24155,24 @@ gen_inlined_subroutine_die (tree stmt, d
>> > add_high_low_attributes (stmt, subr_die);
>> > add_call_src_coords_attributes (stmt, subr_die);
>> >
>> > - decls_for_scope (stmt, subr_die);
>> > + /* The inliner creates an extra BLOCK for the parameter setup,
>> > + we want to merge that with the actual outermost BLOCK of the
>> > + inlined function to avoid duplicate locals in consumers. Note
>> > + we specially mark that not as origin-self.
>> > + Do that by doing the recursion to subblocks on the single subblock
>> > + of STMT. */
>> > + bool unwrap_one = false;
>> > + if (BLOCK_SUBBLOCKS (stmt) && !BLOCK_CHAIN (BLOCK_SUBBLOCKS (stmt)))
>> > + {
>> > + tree origin = block_ultimate_origin (BLOCK_SUBBLOCKS (stmt));
>> > + if (origin
>> > + && TREE_CODE (origin) == BLOCK
>> > + && !BLOCK_ABSTRACT_ORIGIN (origin))
>>
>> Can we look at BLOCK_SUPERCONTEXT here rather than above?
>
> Ah, yes. It could be simply
>
> + && BLOCK_SUPERCONTEXT (origin) == decl)
>
> I guess. I also noticed that the very same bug was noticed in
> the fixed PP37801 in comment #4 by you but the fix that was installed
> didn't fix that part thus with the fix we now regress
> gcc.dg/debug/dwarf2/inline2.c which explicitely looks for the bogus
> DW_TAG_lexical_blocks. I have adjusted that testcase now.
>
> Re-bootstrap & regtest in progress on x86_64-unknown-linux-gnu, OK?
>
> Thanks,
> Richard.
>
> 2018-09-27 Richard Biener <rguent...@suse.de>
>
> PR debug/37801
> PR debug/87440
> * dwarf2out.c (set_block_origin_self): Do not mark outermost
> block as we do not output that.
> (gen_inlined_subroutine_die): Elide the originally outermost
> block, matching what we do for concrete instances.
> (decls_for_scope): Add parameter specifying whether to recurse
> to subblocks.
>
> * gcc.dg/debug/dwarf2/inline2.c: Adjust.
> * gcc.dg/debug/dwarf2/inline4.c: New testcase.
>
> Index: gcc/dwarf2out.c
> ===================================================================
> --- gcc/dwarf2out.c (revision 264662)
> +++ gcc/dwarf2out.c (working copy)
> @@ -3867,7 +3867,7 @@ static void gen_subroutine_type_die (tre
> static void gen_typedef_die (tree, dw_die_ref);
> static void gen_type_die (tree, dw_die_ref);
> static void gen_block_die (tree, dw_die_ref);
> -static void decls_for_scope (tree, dw_die_ref);
> +static void decls_for_scope (tree, dw_die_ref, bool = true);
> static bool is_naming_typedef_decl (const_tree);
> static inline dw_die_ref get_context_die (tree);
> static void gen_namespace_die (tree, dw_die_ref);
> @@ -24147,7 +24147,24 @@ gen_inlined_subroutine_die (tree stmt, d
> add_high_low_attributes (stmt, subr_die);
> add_call_src_coords_attributes (stmt, subr_die);
>
> - decls_for_scope (stmt, subr_die);
> + /* The inliner creates an extra BLOCK for the parameter setup,
> + we want to merge that with the actual outermost BLOCK of the
> + inlined function to avoid duplicate locals in consumers. Note
> + we specially mark that not as origin-self.
This sentence is no longer accurate; OK with it removed.
Jason
