Re: [PATCH] Fix profiledbootstrap ada checking failure (PR debug/79255)

[Eric Botcazou]

> eric@polaris:~/build/gcc/native/gcc> rm ada/sem_util.o
> eric@polaris:~/build/gcc/native/gcc> make ADAFLAGS="-gnatpgn"
> /home/eric/build/gcc/native/./prev-gcc/xgcc -
> B/home/eric/build/gcc/native/./prev-gcc/
> -B/home/eric/install/gcc/x86_64-suse- linux/bin/
> -B/home/eric/install/gcc/x86_64-suse-linux/bin/ -
> B/home/eric/install/gcc/x86_64-suse-linux/lib/ -isystem
> /home/eric/install/gcc/x86_64-suse-linux/include -isystem
> /home/eric/install/gcc/x86_64-suse-linux/sys-include-c -g -O2  -gnatpgn 
> - W -Wall -nostdinc -I- -I. -Iada/generated -Iada
> -I/home/eric/svn/gcc/gcc/ada - I/home/eric/svn/gcc/gcc/ada/gcc-interface
> /home/eric/svn/gcc/gcc/ada/sem_util.adb -o ada/sem_util.o
> 
> raised STORAGE_ERROR : stack overflow or erroneous memory access
> /home/eric/svn/gcc/gcc/ada/gcc-interface/Make-lang.in:119: recipe for target
> 'ada/sem_util.o' failed

We have parent & child functions that are (partially) inlined into each other:

   ---
   -- Set_Debug_Info_Needed --
   ---

   procedure Set_Debug_Info_Needed (T : Entity_Id) is

  procedure Set_Debug_Info_Needed_If_Not_Set (E : Entity_Id);
  pragma Inline (Set_Debug_Info_Needed_If_Not_Set);
  --  Used to set debug info in a related node if not set already

  --
  -- Set_Debug_Info_Needed_If_Not_Set --
  --

  procedure Set_Debug_Info_Needed_If_Not_Set (E : Entity_Id) is
  begin
 if Present (E) and then not Needs_Debug_Info (E) then
Set_Debug_Info_Needed (E);

--  For a private type, indicate that the full view also needs
--  debug information.

if Is_Type (E)
  and then Is_Private_Type (E)
  and then Present (Full_View (E))
then
   Set_Debug_Info_Needed (Full_View (E));
end if;
 end if;
  end Set_Debug_Info_Needed_If_Not_Set;

   --  Start of processing for Set_Debug_Info_Needed

so -fno-partial-inlining is a workaround.

-- 
Eric Botcazou

Re: [PATCH] Fix profiledbootstrap ada checking failure (PR debug/79255)

[Eric Botcazou]

> We have local modifications in the Ada front-end so I cannot reproduce it
> with the pristine tree either. :-(

I apparently screwed up yesterday, this can be reproduced as such in the 
directory of a boostrapped compiler:

eric@polaris:~/build/gcc/native/gcc> rm ada/sem_util.o 
eric@polaris:~/build/gcc/native/gcc> make ADAFLAGS="-gnatpgn"
/home/eric/build/gcc/native/./prev-gcc/xgcc -
B/home/eric/build/gcc/native/./prev-gcc/ -B/home/eric/install/gcc/x86_64-suse-
linux/bin/ -B/home/eric/install/gcc/x86_64-suse-linux/bin/ -
B/home/eric/install/gcc/x86_64-suse-linux/lib/ -isystem 
/home/eric/install/gcc/x86_64-suse-linux/include -isystem 
/home/eric/install/gcc/x86_64-suse-linux/sys-include-c -g -O2  -gnatpgn  -
W -Wall -nostdinc -I- -I. -Iada/generated -Iada -I/home/eric/svn/gcc/gcc/ada -
I/home/eric/svn/gcc/gcc/ada/gcc-interface 
/home/eric/svn/gcc/gcc/ada/sem_util.adb -o ada/sem_util.o

raised STORAGE_ERROR : stack overflow or erroneous memory access
/home/eric/svn/gcc/gcc/ada/gcc-interface/Make-lang.in:119: recipe for target 
'ada/sem_util.o' failed
make: *** [ada/sem_util.o] Error 1

(gdb) bt
#0  0x00a2d4f8 in ggc_internal_alloc(unsigned long, void (*)(void*), 
unsigned long, unsigned long) ()
#1  0x00c2e069 in ggc_internal_cleared_alloc(unsigned long, void (*)
(void*), unsigned long, unsigned long) ()
#2  0x00b23865 in one_reg_loc_descriptor(unsigned int, 
var_init_status)
()
#3  0x00b5ba98 in loc_descriptor(rtx_def*, machine_mode, 
var_init_status) ()
#4  0x00b5c0b2 in loc_descriptor(rtx_def*, machine_mode, 
var_init_status) ()
#5  0x00b5661a in dw_loc_list_1(tree_node*, rtx_def*, int, 
var_init_status) ()
#6  0x00b56e0d in loc_list_from_tree_1(tree_node*, int, 
loc_descr_context*) ()
#7  0x00b59aeb in loc_list_from_tree(tree_node*, int, 
loc_descr_context*) ()
#8  0x00b5e14f in add_location_or_const_value_attribute(die_struct*, 
tree_node*, bool) ()
#9  0x00b60435 in gen_formal_parameter_die(tree_node*, tree_node*, 
bool, die_struct*) ()
#10 0x00b464c3 in gen_decl_die(tree_node*, tree_node*, vlr_context*, 
die_struct*) ()
#11 0x00b6660d in process_scope_var(tree_node*, tree_node*, 
tree_node*, die_struct*) ()
#12 0x00b40e78 in decls_for_scope(tree_node*, die_struct*) ()
#13 0x00b409db in gen_block_die(tree_node*, die_struct*) ()
#14 0x00b40f0b in decls_for_scope(tree_node*, die_struct*) ()
#15 0x00b41b7f in gen_subprogram_die(tree_node*, die_struct*) ()
#16 0x00b45c5a in gen_decl_die(tree_node*, tree_node*, vlr_context*, 
die_struct*) ()
#17 0x00b6660d in process_scope_var(tree_node*, tree_node*, 
tree_node*, die_struct*) ()
#18 0x00b40fad in decls_for_scope(tree_node*, die_struct*) ()
#19 0x00b409db in gen_block_die(tree_node*, die_struct*) ()
---Type  to continue, or q  to quit---
#20 0x00b40f0b in decls_for_scope(tree_node*, die_struct*) ()
#21 0x00b409db in gen_block_die(tree_node*, die_struct*) ()
#22 0x00b40f0b in decls_for_scope(tree_node*, die_struct*) ()
#23 0x00b41b7f in gen_subprogram_die(tree_node*, die_struct*) ()
#24 0x00b45c5a in gen_decl_die(tree_node*, tree_node*, vlr_context*, 
die_struct*) ()
#25 0x00b6660d in process_scope_var(tree_node*, tree_node*, 
tree_node*, die_struct*) ()
#26 0x00b40fad in decls_for_scope(tree_node*, die_struct*) ()
#27 0x00b409db in gen_block_die(tree_node*, die_struct*) ()
#28 0x00b40f0b in decls_for_scope(tree_node*, die_struct*) ()
#29 0x00b409db in gen_block_die(tree_node*, die_struct*) ()
#30 0x00b40f0b in decls_for_scope(tree_node*, die_struct*) ()
#31 0x00b41b7f in gen_subprogram_die(tree_node*, die_struct*) ()
#32 0x00b45c5a in gen_decl_die(tree_node*, tree_node*, vlr_context*, 
die_struct*) ()
#33 0x00b6660d in process_scope_var(tree_node*, tree_node*, 
tree_node*, die_struct*) ()
#34 0x00b40fad in decls_for_scope(tree_node*, die_struct*) ()
#35 0x00b409db in gen_block_die(tree_node*, die_struct*) ()
#36 0x00b40f0b in decls_for_scope(tree_node*, die_struct*) ()
#37 0x00b409db in gen_block_die(tree_node*, die_struct*) ()
#38 0x00b40f0b in decls_for_scope(tree_node*, die_struct*) ()
#39 0x00b41b7f in gen_subprogram_die(tree_node*, die_struct*) ()
#40 0x00b45c5a in gen_decl_die(tree_node*, tree_node*, vlr_context*, 
die_struct*) ()
#41 0x00b6660d in process_scope_var(tree_node*, tree_node*, 
tree_node*, die_struct*) ()

with thousands of similar frames.

-- 
Eric Botcazou

Re: [PATCH] Fix profiledbootstrap ada checking failure (PR debug/79255)

[Eric Botcazou]

> The following C testcase shows how profiledbootstrap fails with checking
> compiler.  We have a (nested) FUNCTION_DECL inside of BLOCK_VARS of an
> inline function, when it gets inlined, it is moved into
> BLOCK_NONLOCALIZED_VARS.  And, decls_for_scope calls process_scope_var
> with NULL decl and non-NULL origin for all BLOCK_NONLOCALIZED_VARS.
> That is fine for variables, but for FUNCTION_DECLs it can actually
> try to dwarf2out_abstract_function that FUNCTION_DECL, which should be
> really done only when it is inlined (i.e. BLOCK_ABSTRACT_ORIGIN of
> some BLOCK).  The effect is that we actually add DW_AT_inline attribute
> to that DW_TAG_subroutine, and then later when processing it again
> we add DW_AT_low_pc etc. and ICE, because those attributes should not
> appear on DW_AT_inline functions.
> 
> Fixed by handling FUNCTION_DECLs always the same, whether in BLOCK_VARS
> or BLOCK_NONLOCALIZED_VARS.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2017-03-23  Jakub Jelinek  
> 
>   PR debug/79255
>   * dwarf2out.c (decls_for_scope): If BLOCK_NONLOCALIZED_VAR is
>   a FUNCTION_DECL, pass it as decl instead of origin to
>   process_scope_var.

Thanks for working on this, however the patch breaks bootstrap for us in stage 
#2 using a different set of bootstrap options (-gnatpgn -g).  There appears to 
be an endless recursion :

[...]

#27 0x00b7c929 in process_scope_var (stmt=0x742ebf60, 
decl=0x73c01700, origin=0x0, context_die=0x7fffe2ddd870)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:24833
#28 0x00b7ca6f in decls_for_scope (stmt=0x742ebf60, 
context_die=0x7fffe2ddd870)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:24867
#29 0x00b77e78 in gen_lexical_block_die (stmt=0x742ebf60, 
context_die=0x7fffe2ddd6e0)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:23207
#30 0x00b7c638 in gen_block_die (stmt=0x742ebf60, 
context_die=0x7fffe2ddd6e0)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:24795
#31 0x00b7cb07 in decls_for_scope (stmt=0x742ebde0, 
context_die=0x7fffe2ddd6e0)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:24881
#32 0x00b78017 in gen_inlined_subroutine_die (stmt=0x742ebde0, 
context_die=0x7fffe2ddd1e0)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:23245
#33 0x00b7c623 in gen_block_die (stmt=0x742ebde0, 
context_die=0x7fffe2ddd1e0)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:24792
#34 0x00b7cb07 in decls_for_scope (stmt=0x7449e000, 
---Type  to continue, or q  to quit---
context_die=0x7fffe2ddd1e0)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:24881
#35 0x00b7c64d in gen_block_die (stmt=0x7449e000, 
context_die=0x7fffe2ddd1e0)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:24798
#36 0x00b7cb07 in decls_for_scope (stmt=0x73f7bf00, 
context_die=0x7fffe2ddd1e0)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:24881
#37 0x00b77e78 in gen_lexical_block_die (stmt=0x73f7bf00, 
context_die=0x7fffe2ddaeb0)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:23207
#38 0x00b7c638 in gen_block_die (stmt=0x73f7bf00, 
context_die=0x7fffe2ddaeb0)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:24795
#39 0x00b7cb07 in decls_for_scope (stmt=0x73bf98a0, 
context_die=0x7fffe2ddaeb0)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:24881
#40 0x00b72766 in gen_subprogram_die (decl=0x73c01700, 
context_die=0x7fffe2ddae60)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:22441
#41 0x00b7e034 in gen_decl_die (decl=0x73c01700, origin=0x0, 
ctx=0x0, context_die=0x7fffe2ddae60)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:25289
#42 0x00b7c929 in process_scope_var (stmt=0x742ebf60, 
decl=0x73c01700, origin=0x0, context_die=0x7fffe2ddae60)
at /home/eric/gnat/gnat-head/src/gcc/dwarf2out.c:24833

[...]

We have local modifications in the Ada front-end so I cannot reproduce it with 
the pristine tree either. :-(  I'll try harder tomorrow.

-- 
Eric Botcazou

Re: [PATCH] Fix profiledbootstrap ada checking failure (PR debug/79255)

[Jakub Jelinek]

On Fri, Mar 24, 2017 at 09:07:54AM -0400, Jason Merrill wrote:
> >> And when it's cloned.
> >>
> >> But does it make sense for gen_decl_die to call
> >> dwarf2out_abstract_function when decl is null?  That seems wrong.
> >
> > Before r144529 we had just:
> >   if (DECL_ORIGIN (decl) != decl)
> > dwarf2out_abstract_function (DECL_ABSTRACT_ORIGIN (decl));
> > and that is indeed to handle clones etc.
> >
> > r144529 changed that to:
> >   if (origin || DECL_ORIGIN (decl) != decl)
> > dwarf2out_abstract_function (DECL_ABSTRACT_ORIGIN (decl));
> > All of the decl is NULL introduced in r144529 implies decl_or_origin
> > is abstract.
> >
> > Removing that origin || wouldn't really work, we'd have to rewrite most of
> > gen_decl_die FUNCTION_DECL handling to use decl_or_origin instead of
> > decl etc.
> 
> I was thinking to change it to
> 
>   if (decl && (origin || DECL_ORIGIN (decl) != decl))

But then you segfault immediately on the next:
  else if (cgraph_function_possibly_inlined_p (decl)
   && ! DECL_ABSTRACT_P (decl)
because decl is NULL.  So, it would be far easier to do:
  if (decl == NULL_TREE)
{
  decl = origin;
  origin = NULL_TREE;
}
before the
  if (origin || DECL_ORIGIN (decl) != decl)
with a comment, rather than to rewrite a couple of dozen decl_or_origin,
and change assumptions that non-NULL origin actually means anything for
FUNCTION_DECL.  That is IMO still uglier than the decls_for_scope change
though, but if you prefer that, I can test that.

Jakub

Re: [PATCH] Fix profiledbootstrap ada checking failure (PR debug/79255)

[Jason Merrill]

On Fri, Mar 24, 2017 at 3:46 AM, Jakub Jelinek  wrote:
> On Thu, Mar 23, 2017 at 05:24:31PM -0400, Jason Merrill wrote:
>> On Thu, Mar 23, 2017 at 4:44 PM, Jakub Jelinek  wrote:
>> > The following C testcase shows how profiledbootstrap fails with checking
>> > compiler.  We have a (nested) FUNCTION_DECL inside of BLOCK_VARS of an
>> > inline function, when it gets inlined, it is moved into
>> > BLOCK_NONLOCALIZED_VARS.  And, decls_for_scope calls process_scope_var
>> > with NULL decl and non-NULL origin for all BLOCK_NONLOCALIZED_VARS.
>> > That is fine for variables, but for FUNCTION_DECLs it can actually
>> > try to dwarf2out_abstract_function that FUNCTION_DECL, which should be
>> > really done only when it is inlined (i.e. BLOCK_ABSTRACT_ORIGIN of
>> > some BLOCK).
>>
>> And when it's cloned.
>>
>> But does it make sense for gen_decl_die to call
>> dwarf2out_abstract_function when decl is null?  That seems wrong.
>
> Before r144529 we had just:
>   if (DECL_ORIGIN (decl) != decl)
> dwarf2out_abstract_function (DECL_ABSTRACT_ORIGIN (decl));
> and that is indeed to handle clones etc.
>
> r144529 changed that to:
>   if (origin || DECL_ORIGIN (decl) != decl)
> dwarf2out_abstract_function (DECL_ABSTRACT_ORIGIN (decl));
> All of the decl is NULL introduced in r144529 implies decl_or_origin
> is abstract.
>
> Removing that origin || wouldn't really work, we'd have to rewrite most of
> gen_decl_die FUNCTION_DECL handling to use decl_or_origin instead of
> decl etc.

I was thinking to change it to

  if (decl && (origin || DECL_ORIGIN (decl) != decl))

Jason

Re: [PATCH] Fix profiledbootstrap ada checking failure (PR debug/79255)

[Jakub Jelinek]

On Fri, Mar 24, 2017 at 12:45:28PM +0100, Richard Biener wrote:
> On Fri, Mar 24, 2017 at 9:43 AM, Jakub Jelinek  wrote:
> > On Fri, Mar 24, 2017 at 09:29:00AM +0100, Richard Biener wrote:
> >> Yeah, the thing BLOCK_NONLOCALIZED_VARS wants to do is optimize generated
> >> dwarf by adding a DW_AT_abstract_origin (just to refer to the
> >> subprogram DIE) but
> >
> > Well, for FUNCTION_DECLs in BLOCK_VARS/BLOCK_NONLOCALIZED_VARS we actually 
> > don't
> > emit any further DIE and so there is no DW_AT_abstract_origin.
> > E.g. gen_subprogram_die has:
> >   /* Detect and ignore this case, where we are trying to output
> >  something we have already output.  */
> >   if (get_AT (old_die, DW_AT_low_pc)
> >   || get_AT (old_die, DW_AT_ranges))
> > return;
> 
> Hmm, but we do want to put the function in scope?  THus
> 
> void foo () {}
> void bar ()
> {
>   int foo;
>   {
>  void foo();
>  foo();
>   }
> }
> 
> should have a DIE for foo in bar (possibly refering to the concrete instance
> for optimization).

We actually do that.  If I change your testcase so that it actually triggers
the changed part of the code (so there is inlining etc.), so:
volatile int v;
__attribute__((noinline)) void foo () { v++; }
static inline void bar ()
{
  int foo;
  {
 void foo();
 foo();
  }
}
void
baz (void)
{
  bar ();
  bar ();
}
then at -gdwarf-3 -dA -O2 the difference between vanilla GCC and my patched GCC 
is
following (used -gdwarf-3 so that there are no DW_FORM_flag_present that
result in DIE offset changes):
.uleb128 0xd# (DIE (0x117) DW_TAG_subprogram)
.ascii "bar\0"  # DW_AT_name
.byte   0x1 # DW_AT_decl_file (prQQ.c)
.byte   0x3 # DW_AT_decl_line
.byte   0x3 # DW_AT_inline
.long   0x13c   # DW_AT_sibling
.uleb128 0xe# (DIE (0x123) DW_TAG_variable)
.ascii "foo\0"  # DW_AT_name
.byte   0x1 # DW_AT_decl_file (prQQ.c)
.byte   0x5 # DW_AT_decl_line
.long   0x41# DW_AT_type
.uleb128 0xf# (DIE (0x12e) DW_TAG_lexical_block)
.uleb128 0x10   # (DIE (0x12f) DW_TAG_subprogram)
.byte   0x1 # DW_AT_external
.ascii "foo\0"  # DW_AT_name
.byte   0x1 # DW_AT_decl_file (prQQ.c)
.byte   0x7 # DW_AT_decl_line
-   .byte   0   # DW_AT_inline
+   .byte   0x1 # DW_AT_declaration
.uleb128 0x11   # (DIE (0x138) DW_TAG_unspecified_parameters)
.byte   0   # end of children of DIE 0x12f
.byte   0   # end of children of DIE 0x12e
.byte   0   # end of children of DIE 0x117
.uleb128 0x12   # (DIE (0x13c) DW_TAG_subprogram)
.byte   0x1 # DW_AT_external
.ascii "foo\0"  # DW_AT_name
.byte   0x1 # DW_AT_decl_file (prQQ.c)
.byte   0x2 # DW_AT_decl_line
.quad   .LFB0   # DW_AT_low_pc
.quad   .LFE0   # DW_AT_high_pc
.byte   0x1 # DW_AT_frame_base
.byte   0x9c# DW_OP_call_frame_cfa
.byte   0x1 # DW_AT_GNU_all_call_sites
.byte   0   # end of children of DIE 0xb
and corresponding change in .debug_abbrev.
That seems to be the right change to me, inside of bar (the DW_AT_inline
one) we have a declaration of foo, and we properly use DW_AT_declaration for
it, then at the toplevel we actually have the full definition die for foo,
and then we have in two spots inside baz
.uleb128 0x6# (DIE (0x6d) DW_TAG_inlined_subroutine)
.long   0x117   # DW_AT_abstract_origin
without the need to duplicate the foo declaration in there,
that is inherited through DW_AT_abstract_origin.

Jakub

Re: [PATCH] Fix profiledbootstrap ada checking failure (PR debug/79255)

[Richard Biener]

On Fri, Mar 24, 2017 at 9:43 AM, Jakub Jelinek  wrote:
> On Fri, Mar 24, 2017 at 09:29:00AM +0100, Richard Biener wrote:
>> Yeah, the thing BLOCK_NONLOCALIZED_VARS wants to do is optimize generated
>> dwarf by adding a DW_AT_abstract_origin (just to refer to the
>> subprogram DIE) but
>
> Well, for FUNCTION_DECLs in BLOCK_VARS/BLOCK_NONLOCALIZED_VARS we actually 
> don't
> emit any further DIE and so there is no DW_AT_abstract_origin.
> E.g. gen_subprogram_die has:
>   /* Detect and ignore this case, where we are trying to output
>  something we have already output.  */
>   if (get_AT (old_die, DW_AT_low_pc)
>   || get_AT (old_die, DW_AT_ranges))
> return;

Hmm, but we do want to put the function in scope?  THus

void foo () {}
void bar ()
{
  int foo;
  {
 void foo();
 foo();
  }
}

should have a DIE for foo in bar (possibly refering to the concrete instance
for optimization).

Richard.

> That is why the posted testcase doesn't ICE without -fno-toplevel-reorder,
> normally the body is emitted earlier and so we don't do anything at all.
> Otherwise we just want to make sure we have a DIE and, if it is
> inline/clone, have also DW_AT_inline set, and if the DIE is without parent
> that we put it into proper place in the DIE tree.  And when we actually
> see the body of the function we fill locations and all other details.
>
> Jakub

Re: [PATCH] Fix profiledbootstrap ada checking failure (PR debug/79255)

[Jakub Jelinek]

On Fri, Mar 24, 2017 at 09:29:00AM +0100, Richard Biener wrote:
> Yeah, the thing BLOCK_NONLOCALIZED_VARS wants to do is optimize generated
> dwarf by adding a DW_AT_abstract_origin (just to refer to the
> subprogram DIE) but

Well, for FUNCTION_DECLs in BLOCK_VARS/BLOCK_NONLOCALIZED_VARS we actually don't
emit any further DIE and so there is no DW_AT_abstract_origin.
E.g. gen_subprogram_die has:
  /* Detect and ignore this case, where we are trying to output
 something we have already output.  */
  if (get_AT (old_die, DW_AT_low_pc)
  || get_AT (old_die, DW_AT_ranges))
return;
That is why the posted testcase doesn't ICE without -fno-toplevel-reorder,
normally the body is emitted earlier and so we don't do anything at all.
Otherwise we just want to make sure we have a DIE and, if it is
inline/clone, have also DW_AT_inline set, and if the DIE is without parent
that we put it into proper place in the DIE tree.  And when we actually
see the body of the function we fill locations and all other details.

Jakub

Re: [PATCH] Fix profiledbootstrap ada checking failure (PR debug/79255)

[Richard Biener]

On Fri, Mar 24, 2017 at 8:46 AM, Jakub Jelinek  wrote:
> On Thu, Mar 23, 2017 at 05:24:31PM -0400, Jason Merrill wrote:
>> On Thu, Mar 23, 2017 at 4:44 PM, Jakub Jelinek  wrote:
>> > The following C testcase shows how profiledbootstrap fails with checking
>> > compiler.  We have a (nested) FUNCTION_DECL inside of BLOCK_VARS of an
>> > inline function, when it gets inlined, it is moved into
>> > BLOCK_NONLOCALIZED_VARS.  And, decls_for_scope calls process_scope_var
>> > with NULL decl and non-NULL origin for all BLOCK_NONLOCALIZED_VARS.
>> > That is fine for variables, but for FUNCTION_DECLs it can actually
>> > try to dwarf2out_abstract_function that FUNCTION_DECL, which should be
>> > really done only when it is inlined (i.e. BLOCK_ABSTRACT_ORIGIN of
>> > some BLOCK).
>>
>> And when it's cloned.
>>
>> But does it make sense for gen_decl_die to call
>> dwarf2out_abstract_function when decl is null?  That seems wrong.
>
> Before r144529 we had just:
>   if (DECL_ORIGIN (decl) != decl)
> dwarf2out_abstract_function (DECL_ABSTRACT_ORIGIN (decl));
> and that is indeed to handle clones etc.
>
> r144529 changed that to:
>   if (origin || DECL_ORIGIN (decl) != decl)
> dwarf2out_abstract_function (DECL_ABSTRACT_ORIGIN (decl));
> All of the decl is NULL introduced in r144529 implies decl_or_origin
> is abstract.
>
> Removing that origin || wouldn't really work, we'd have to rewrite most of
> gen_decl_die FUNCTION_DECL handling to use decl_or_origin instead of
> decl etc.
>
> But it doesn't look right to me, conceptually a FUNCTION_DECL in
> BLOCK_NONLOCALIZED_VARS is nothing that represents a clone or something
> abstract in itself.  We can't keep it in BLOCK_VARS just because those
> are chained through DECL_CHAIN and a single FUNCTION_DECL can't be
> put into multiple BLOCK_VARS chains.  It is still the same FUNCTION_DECL,
> not a sign that we want to have in each inline function a separate
> function declaration with abstract origin of the original FUNCTION_DECL.

Yeah, the thing BLOCK_NONLOCALIZED_VARS wants to do is optimize generated
dwarf by adding a DW_AT_abstract_origin (just to refer to the
subprogram DIE) but
it doesn't actually care about refering to an abstract instance (a
concrete one would
work just fine).

So I think in the context of scope vars calling
dwarf2out_abstract_function is _always_
wrong.  But it would require quite some refactoring to do this in a clean way.

Richard.

> Jakub

Re: [PATCH] Fix profiledbootstrap ada checking failure (PR debug/79255)

[Jakub Jelinek]

On Thu, Mar 23, 2017 at 05:24:31PM -0400, Jason Merrill wrote:
> On Thu, Mar 23, 2017 at 4:44 PM, Jakub Jelinek  wrote:
> > The following C testcase shows how profiledbootstrap fails with checking
> > compiler.  We have a (nested) FUNCTION_DECL inside of BLOCK_VARS of an
> > inline function, when it gets inlined, it is moved into
> > BLOCK_NONLOCALIZED_VARS.  And, decls_for_scope calls process_scope_var
> > with NULL decl and non-NULL origin for all BLOCK_NONLOCALIZED_VARS.
> > That is fine for variables, but for FUNCTION_DECLs it can actually
> > try to dwarf2out_abstract_function that FUNCTION_DECL, which should be
> > really done only when it is inlined (i.e. BLOCK_ABSTRACT_ORIGIN of
> > some BLOCK).
> 
> And when it's cloned.
> 
> But does it make sense for gen_decl_die to call
> dwarf2out_abstract_function when decl is null?  That seems wrong.

Before r144529 we had just:
  if (DECL_ORIGIN (decl) != decl)
dwarf2out_abstract_function (DECL_ABSTRACT_ORIGIN (decl));
and that is indeed to handle clones etc.

r144529 changed that to:
  if (origin || DECL_ORIGIN (decl) != decl)
dwarf2out_abstract_function (DECL_ABSTRACT_ORIGIN (decl));
All of the decl is NULL introduced in r144529 implies decl_or_origin
is abstract.

Removing that origin || wouldn't really work, we'd have to rewrite most of
gen_decl_die FUNCTION_DECL handling to use decl_or_origin instead of
decl etc.

But it doesn't look right to me, conceptually a FUNCTION_DECL in
BLOCK_NONLOCALIZED_VARS is nothing that represents a clone or something
abstract in itself.  We can't keep it in BLOCK_VARS just because those
are chained through DECL_CHAIN and a single FUNCTION_DECL can't be
put into multiple BLOCK_VARS chains.  It is still the same FUNCTION_DECL,
not a sign that we want to have in each inline function a separate
function declaration with abstract origin of the original FUNCTION_DECL.

Jakub

Re: [PATCH] Fix profiledbootstrap ada checking failure (PR debug/79255)

[Jason Merrill]

On Thu, Mar 23, 2017 at 4:44 PM, Jakub Jelinek  wrote:
> The following C testcase shows how profiledbootstrap fails with checking
> compiler.  We have a (nested) FUNCTION_DECL inside of BLOCK_VARS of an
> inline function, when it gets inlined, it is moved into
> BLOCK_NONLOCALIZED_VARS.  And, decls_for_scope calls process_scope_var
> with NULL decl and non-NULL origin for all BLOCK_NONLOCALIZED_VARS.
> That is fine for variables, but for FUNCTION_DECLs it can actually
> try to dwarf2out_abstract_function that FUNCTION_DECL, which should be
> really done only when it is inlined (i.e. BLOCK_ABSTRACT_ORIGIN of
> some BLOCK).

And when it's cloned.

But does it make sense for gen_decl_die to call
dwarf2out_abstract_function when decl is null?  That seems wrong.

Jason

[PATCH] Fix profiledbootstrap ada checking failure (PR debug/79255)

[Jakub Jelinek]

Hi!

The following C testcase shows how profiledbootstrap fails with checking
compiler.  We have a (nested) FUNCTION_DECL inside of BLOCK_VARS of an
inline function, when it gets inlined, it is moved into
BLOCK_NONLOCALIZED_VARS.  And, decls_for_scope calls process_scope_var
with NULL decl and non-NULL origin for all BLOCK_NONLOCALIZED_VARS.
That is fine for variables, but for FUNCTION_DECLs it can actually
try to dwarf2out_abstract_function that FUNCTION_DECL, which should be
really done only when it is inlined (i.e. BLOCK_ABSTRACT_ORIGIN of
some BLOCK).  The effect is that we actually add DW_AT_inline attribute
to that DW_TAG_subroutine, and then later when processing it again
we add DW_AT_low_pc etc. and ICE, because those attributes should not
appear on DW_AT_inline functions.

Fixed by handling FUNCTION_DECLs always the same, whether in BLOCK_VARS
or BLOCK_NONLOCALIZED_VARS.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-03-23  Jakub Jelinek  

PR debug/79255
* dwarf2out.c (decls_for_scope): If BLOCK_NONLOCALIZED_VAR is
a FUNCTION_DECL, pass it as decl instead of origin to
process_scope_var.

* gcc.dg/pr79255.c: New test.

--- gcc/dwarf2out.c.jj  2017-03-22 19:31:41.525055795 +0100
+++ gcc/dwarf2out.c 2017-03-23 17:57:09.419362739 +0100
@@ -24861,8 +24861,13 @@ decls_for_scope (tree stmt, dw_die_ref c
 if we've done it once already.  */
   if (! early_dwarf)
for (i = 0; i < BLOCK_NUM_NONLOCALIZED_VARS (stmt); i++)
- process_scope_var (stmt, NULL, BLOCK_NONLOCALIZED_VAR (stmt, i),
-context_die);
+ {
+   decl = BLOCK_NONLOCALIZED_VAR (stmt, i);
+   if (TREE_CODE (decl) == FUNCTION_DECL)
+ process_scope_var (stmt, decl, NULL_TREE, context_die);
+   else
+ process_scope_var (stmt, NULL_TREE, decl, context_die);
+ }
 }
 
   /* Even if we're at -g1, we need to process the subblocks in order to get
--- gcc/testsuite/gcc.dg/pr79255.c.jj   2017-03-23 17:57:44.711911298 +0100
+++ gcc/testsuite/gcc.dg/pr79255.c  2017-03-23 17:56:24.0 +0100
@@ -0,0 +1,21 @@
+/* PR bootstrap/79255 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -g -fno-toplevel-reorder -Wno-attributes" } */
+
+static inline __attribute__((always_inline)) int foo (int x);
+
+int
+baz (void)
+{
+  return foo (3) + foo (6) + foo (9);
+}
+
+static inline __attribute__((always_inline)) int
+foo (int x)
+{
+  auto inline int __attribute__((noinline)) bar (int x)
+  {
+return x + 3;
+  }
+  return bar (x) + bar (x + 2);
+}

Jakub