On Sun, 26 Nov 2017, Jeff Law wrote:

> On 11/24/2017 02:53 PM, Jakub Jelinek wrote:
> > Hi!
> > 
> > On most targets, N_SLINE has addresses relative to the start of
> > the function, which means -gstabs{,+} is completely broken with
> > hot/cold partitioning (fails to assemble almost anything that
> > has both partitions).  This used to be bearable when it wasn't
> > the default, but now that we hot/cold partition by default it means
> > STABS is completely unusable.
> > 
> > Because STABS should die soon, I'm not trying to propose any extension,
> > just emit something that assemble and be tolerable for debugging purposes
> > (after all, debugging optimized code with stabs isn't really a good idea
> > because it doesn't handle block fragments anyway).
> > 
> > What the patch does is that it treats hot/cold partitioned functions
> > basically as two functions, say main and main.cold.1, in the hot partition
> > everything should be normal, except that lexical scopes that only appear in
> > cold partition are not emitted in the [lr]brac tree for the hot partition.
> > Then the cold partition is yet another N_FUN, set of N_SLINE relative to
> > the start of the cold partition, and finally another [lr]brac block tree.
> > This one doesn't include any lexical scopes that are solely in the hot
> > partition, but we can have scopes that are in both, those are duplicated,
> > sometimes with merged vars from multiple scopes.
> > 
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> > 
> > Could anybody from the debugger folks test it a little bit (say gdb
> > testsuite if it has any -O2 -gstabs/-O2 -gstabs+ tests)?
> > 
> > 2017-11-24  Jakub Jelinek  <ja...@redhat.com>
> > 
> >     PR debug/81307
> >     * dbxout.c (lastlineno): New variable.
> >     (dbx_debug_hooks): Use dbxout_switch_text_section as
> >     switch_text_section debug hook.
> >     (dbxout_function_end): Switch to current_function_section
> >     rather than function_section.  If crtl->has_bb_partition,
> >     output just one N_FUN, depending on in_cold_section_p.
> >     (dbxout_source_line): Remember last lineno in lastlineno.
> >     (dbxout_switch_text_section): New function.
> >     (dbxout_function_decl): Adjust dbxout_block caller.
> >     (dbx_block_with_cold_children): New function.
> >     (dbxout_block): Return true if any LBRAC/RBRAC have been
> >     emitted.  Use dbx_block_with_cold_children at depth == 0
> >     in second partition.  Add PARENT_BLOCKNUM argument, pass
> >     it optionally adjusted to children.  Output LBRAC/RBRAC
> >     around recursive call only if the block is in the current
> >     partition, if not and anything was output, emit empty
> >     range LBRAC/RBRAC.
> >     * final.c (final_scan_insn): Compute cold_function_name
> >     before calling switch_text_section debug hook.  Call
> >     that hook even if dwarf2out_do_frame if not emitting
> >     dwarf debug info.
> Alternately, just issue a warning that -gstabs isn't supported when
> hot/cold partitioning is enabled.  I'm just not sure it's worth the
> headache to bother making this even limp along.
> 
> No objection if you want to go ahead with your patch, you've already
> done the work, but fixing bugs in stabs support, ewwww.

Would have been such a nice excuse to drop STABS support ;)

Anyway, the patch looks sensible.  Eventually out documentation
should more prominently mention that STABS is on the way out and
using DWARF will result in a much better debugging experience.

Let's formally deprecate any non-DWARF debugging format support
in GCC 8 changes.html?

Thanks,
Richard.

Reply via email to