[PATCH] Fix hot/cold partitioning with -gstabs{,+} (PR debug/81307)

[PATCH] Fix hot/cold partitioning with -gstabs{,+} (PR debug/81307)

[Jakub Jelinek]

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  <jakub@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.

--- gcc/dbxout.c.jj	2017-11-01 22:49:18.000000000 +0100
+++ gcc/dbxout.c	2017-11-24 18:04:24.053081853 +0100
@@ -244,6 +244,10 @@ static GTY(()) int source_label_number =
 
 static GTY(()) const char *lastfile;
 
+/* Last line number mentioned in a NOTE insn.  */
+
+static GTY(()) unsigned int lastlineno;
+
 /* Used by PCH machinery to detect if 'lastfile' should be reset to
    base_input_file.  */
 static GTY(()) int lastfile_is_base;
@@ -334,6 +338,7 @@ static void debug_free_queue (void);
 
 static void dbxout_source_line (unsigned int, unsigned int, const char *,
 				int, bool);
+static void dbxout_switch_text_section (void);
 static void dbxout_begin_prologue (unsigned int, unsigned int, const char *);
 static void dbxout_source_file (const char *);
 static void dbxout_function_end (tree);
@@ -380,7 +385,7 @@ const struct gcc_debug_hooks dbx_debug_h
   dbxout_handle_pch,		         /* handle_pch */
   debug_nothing_rtx_insn,	         /* var_location */
   debug_nothing_tree,			 /* size_function */
-  debug_nothing_void,                    /* switch_text_section */
+  dbxout_switch_text_section,            /* switch_text_section */
   debug_nothing_tree_tree,		 /* set_name */
   0,                                     /* start_end_main_source_file */
   TYPE_SYMTAB_IS_ADDRESS                 /* tree_type_symtab_field */
@@ -902,7 +907,7 @@ dbxout_function_end (tree decl ATTRIBUTE
 
   /* The Lscope label must be emitted even if we aren't doing anything
      else; dbxout_block needs it.  */
-  switch_to_section (function_section (current_function_decl));
+  switch_to_section (current_function_section ());
 
   /* Convert Lscope into the appropriate format for local labels in case
      the system doesn't insert underscores in front of user generated
@@ -923,11 +928,12 @@ dbxout_function_end (tree decl ATTRIBUTE
   if (crtl->has_bb_partition)
     {
       dbxout_begin_empty_stabs (N_FUN);
-      dbxout_stab_value_label_diff (crtl->subsections.hot_section_end_label,
-				    crtl->subsections.hot_section_label);
-      dbxout_begin_empty_stabs (N_FUN);
-      dbxout_stab_value_label_diff (crtl->subsections.cold_section_end_label,
-				    crtl->subsections.cold_section_label);
+      if (in_cold_section_p)
+	dbxout_stab_value_label_diff (crtl->subsections.cold_section_end_label,
+				      crtl->subsections.cold_section_label);
+      else
+	dbxout_stab_value_label_diff (crtl->subsections.hot_section_end_label,
+				      crtl->subsections.hot_section_label);
     }
   else
     {
@@ -1215,7 +1221,7 @@ dbxout_handle_pch (unsigned at_end)
 
 #if defined (DBX_DEBUGGING_INFO)
 
-static void dbxout_block (tree, int, tree);
+static bool dbxout_block (tree, int, tree, int);
 
 /* Output debugging info to FILE to switch to sourcefile FILENAME.  */
 
@@ -1289,6 +1295,60 @@ dbxout_source_line (unsigned int lineno,
   else
     dbxout_stabd (N_SLINE, lineno);
 #endif
+  lastlineno = lineno;
+}
+
+/* Unfortunately, at least when emitting relative addresses, STABS
+   has no way to express multiple partitions.  Represent a function
+   as two functions in this case.  */
+
+static void
+dbxout_switch_text_section (void)
+{
+  /* The N_FUN tag at the end of the function is a GNU extension,
+     which may be undesirable, and is unnecessary if we do not have
+     named sections.  */
+  in_cold_section_p = !in_cold_section_p;
+  switch_to_section (current_function_section ());
+  dbxout_block (DECL_INITIAL (current_function_decl), 0,
+		DECL_ARGUMENTS (current_function_decl), -1);
+  dbxout_function_end (current_function_decl);
+  in_cold_section_p = !in_cold_section_p;
+
+  switch_to_section (current_function_section ());
+
+  tree context = decl_function_context (current_function_decl);
+  extern tree cold_function_name;
+
+  dbxout_begin_complex_stabs ();
+  stabstr_I (cold_function_name);
+  stabstr_S (":f");
+
+  tree type = TREE_TYPE (current_function_decl);
+  if (TREE_TYPE (type))
+    dbxout_type (TREE_TYPE (type), 0);
+  else
+    dbxout_type (void_type_node, 0);
+
+  if (context != 0)
+    {
+      stabstr_C (',');
+      stabstr_I (cold_function_name);
+      stabstr_C (',');
+      stabstr_I (DECL_NAME (context));
+    }
+
+  dbxout_finish_complex_stabs (current_function_decl, N_FUN, 0,
+			       crtl->subsections.cold_section_label, 0);
+
+  /* pre-increment the scope counter */
+  scope_labelno++;
+
+  dbxout_source_line (lastlineno, 0, lastfile, 0, true);
+  /* Output function begin block at function scope, referenced
+     by dbxout_block, dbxout_source_line and dbxout_function_end.  */
+  emit_pending_bincls_if_required ();
+  targetm.asm_out.internal_label (asm_out_file, "LFBB", scope_labelno);
 }
 
 /* Describe the beginning of an internal block within a function.  */
@@ -1322,7 +1382,7 @@ dbxout_function_decl (tree decl)
 #ifndef DBX_FUNCTION_FIRST
   dbxout_begin_function (decl);
 #endif
-  dbxout_block (DECL_INITIAL (decl), 0, DECL_ARGUMENTS (decl));
+  dbxout_block (DECL_INITIAL (decl), 0, DECL_ARGUMENTS (decl), -1);
   dbxout_function_end (decl);
 }
 
@@ -3664,6 +3724,26 @@ dbx_output_rbrac (const char *label,
     dbxout_stab_value_label (label);
 }
 
+/* Return true is at least one block among BLOCK, its children or siblings
+   which has TREE_USED, TREE_ASM_WRITTEN and BLOCK_IN_COLD_SECTION_P
+   set.  If there is none, clear TREE_USED bit on such blocks.  */
+
+static bool
+dbx_block_with_cold_children (tree block)
+{
+  bool ret = false;
+  for (; block; block = BLOCK_CHAIN (block))
+    if (TREE_USED (block) && TREE_ASM_WRITTEN (block))
+      {
+	bool children = dbx_block_with_cold_children (BLOCK_SUBBLOCKS (block));
+	if (BLOCK_IN_COLD_SECTION_P (block) || children)
+	  ret = true;
+	else
+	  TREE_USED (block) = false;
+      }
+  return ret;
+}
+
 /* Output everything about a symbol block (a BLOCK node
    that represents a scope level),
    including recursive output of contained blocks.
@@ -3679,22 +3759,31 @@ dbx_output_rbrac (const char *label,
    except for the outermost block.
 
    Actually, BLOCK may be several blocks chained together.
-   We handle them all in sequence.  */
+   We handle them all in sequence.
 
-static void
-dbxout_block (tree block, int depth, tree args)
+   Return true if we emitted any LBRAC/RBRAC.  */
+
+static bool
+dbxout_block (tree block, int depth, tree args, int parent_blocknum)
 {
+  bool ret = false;
   char begin_label[20];
   /* Reference current function start using LFBB.  */
   ASM_GENERATE_INTERNAL_LABEL (begin_label, "LFBB", scope_labelno);
 
-  while (block)
+  /* If called for the second partition, ignore blocks that don't have
+     any children in the second partition.  */
+  if (crtl->has_bb_partition && in_cold_section_p && depth == 0)
+    dbx_block_with_cold_children (block);
+
+  for (; block; block = BLOCK_CHAIN (block))
     {
       /* Ignore blocks never expanded or otherwise marked as real.  */
       if (TREE_USED (block) && TREE_ASM_WRITTEN (block))
 	{
 	  int did_output;
 	  int blocknum = BLOCK_NUMBER (block);
+	  int this_parent = parent_blocknum;
 
 	  /* In dbx format, the syms of a block come before the N_LBRAC.
 	     If nothing is output, we don't need the N_LBRAC, either.  */
@@ -3708,11 +3797,13 @@ dbxout_block (tree block, int depth, tre
 	     the block.  Use the block's tree-walk order to generate
 	     the assembler symbols LBBn and LBEn
 	     that final will define around the code in this block.  */
-	  if (did_output)
+	  if (did_output
+	      && BLOCK_IN_COLD_SECTION_P (block) == in_cold_section_p)
 	    {
 	      char buf[20];
 	      const char *scope_start;
 
+	      ret = true;
 	      if (depth == 0)
 		/* The outermost block doesn't get LBB labels; use
 		   the LFBB local symbol emitted by dbxout_begin_prologue.  */
@@ -3721,16 +3812,21 @@ dbxout_block (tree block, int depth, tre
 		{
 		  ASM_GENERATE_INTERNAL_LABEL (buf, "LBB", blocknum);
 		  scope_start = buf;
+		  this_parent = blocknum;
 		}
 
 	      dbx_output_lbrac (scope_start, begin_label);
 	    }
 
 	  /* Output the subblocks.  */
-	  dbxout_block (BLOCK_SUBBLOCKS (block), depth + 1, NULL_TREE);
+	  bool children
+	    = dbxout_block (BLOCK_SUBBLOCKS (block), depth + 1, NULL_TREE,
+			    this_parent);
+	  ret |= children;
 
 	  /* Refer to the marker for the end of the block.  */
-	  if (did_output)
+	  if (did_output
+	      && BLOCK_IN_COLD_SECTION_P (block) == in_cold_section_p)
 	    {
 	      char buf[100];
 	      if (depth == 0)
@@ -3743,9 +3839,29 @@ dbxout_block (tree block, int depth, tre
 
 	      dbx_output_rbrac (buf, begin_label);
 	    }
+	  else if (did_output && !children)
+	    {
+	      /* If we emitted any vars and didn't output any LBRAC/RBRAC,
+		 either at this level or any lower level, we need to emit
+		 an empty LBRAC/RBRAC pair now.  */
+	      char buf[20];
+	      const char *scope_start;
+
+	      ret = true;
+	      if (parent_blocknum == -1)
+		scope_start = begin_label;
+	      else
+		{
+		  ASM_GENERATE_INTERNAL_LABEL (buf, "LBB", parent_blocknum);
+		  scope_start = buf;
+		}
+
+	      dbx_output_lbrac (scope_start, begin_label);
+	      dbx_output_rbrac (scope_start, begin_label);
+	    }
 	}
-      block = BLOCK_CHAIN (block);
     }
+  return ret;
 }
 
 /* Output the information about a function and its arguments and result.
--- gcc/final.c.jj	2017-11-17 08:40:30.000000000 +0100
+++ gcc/final.c	2017-11-24 16:45:56.051424037 +0100
@@ -2210,8 +2210,17 @@ final_scan_insn (rtx_insn *insn, FILE *f
 	case NOTE_INSN_SWITCH_TEXT_SECTIONS:
 	  in_cold_section_p = !in_cold_section_p;
 
+	  if (in_cold_section_p)
+	    cold_function_name
+	      = clone_function_name (current_function_decl, "cold");
+
 	  if (dwarf2out_do_frame ())
-	    dwarf2out_switch_text_section ();
+	    {
+	      dwarf2out_switch_text_section ();
+	      if (!dwarf2_debug_info_emitted_p (current_function_decl)
+		  && !DECL_IGNORED_P (current_function_decl))
+		debug_hooks->switch_text_section ();
+	    }
 	  else if (!DECL_IGNORED_P (current_function_decl))
 	    debug_hooks->switch_text_section ();
 
@@ -2223,8 +2232,6 @@ final_scan_insn (rtx_insn *insn, FILE *f
 	     suffixing "cold" to the original function's name.  */
 	  if (in_cold_section_p)
 	    {
-	      cold_function_name
-		= clone_function_name (current_function_decl, "cold");
 #ifdef ASM_DECLARE_COLD_FUNCTION_NAME
 	      ASM_DECLARE_COLD_FUNCTION_NAME (asm_out_file,
 					      IDENTIFIER_POINTER

	Jakub

Re: [PATCH] Fix hot/cold partitioning with -gstabs{,+} (PR debug/81307)

[Jeff Law]

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  <jakub@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.

Jeff

Re: [PATCH] Fix hot/cold partitioning with -gstabs{,+} (PR debug/81307)

[Richard Biener]

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  <jakub@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.

Re: [PATCH] Fix hot/cold partitioning with -gstabs{,+} (PR debug/81307)

[Jakub Jelinek]

On Mon, Nov 27, 2017 at 09:21:36AM +0100, Richard Biener wrote:
> > 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?

I'm not against deprecating them.

	Jakub

Re: [PATCH] Fix hot/cold partitioning with -gstabs{,+} (PR debug/81307)

[Jim Wilson]

On 11/24/2017 01:53 PM, Jakub Jelinek wrote:
> Could anybody from the debugger folks test it a little bit (say gdb
> testsuite if it has any -O2 -gstabs/-O2 -gstabs+ tests)?

The gdb testsuite just uses "-g" by default.  It doesn't have any tests 
with optimization by default, though this can be done.  I tried it.  I 
modified an x86_64-linux toolchain to emit stabs by default, and used it 
on the gdb testsuite.  I get about 40k passes, the same number with or 
without your patch.  When I try adding -O2, I get about 28k passes, and 
there are about 26 more with your patch than without.  I do see a few 
assembler errors without the patch, and none with.

> @@ -3664,6 +3724,26 @@ dbx_output_rbrac (const char *label,
>       dbxout_stab_value_label (label);
>   }
>   
> +/* Return true is at least one block among BLOCK, its children or siblings

is -> if

Otherwise this looks OK to me, though I think you went to more trouble 
than necessary.  I can think of 3 simpler ways to fix this.
1) In opts.c, check for DBX_DEBUG and -freorder-blocks-and-partition and 
disable the optimization.
2) In the x86 port, #undef DBX_DEBUGGING_INFO so you can't generate it 
anymore and hence won't test it anymore.
3) In the testsuite, drop stabs from the debug torture list.  And maybe 
add dwarf-5 at the same time, as we really should be testing that.

Jim

Re: [PATCH] Fix hot/cold partitioning with -gstabs{,+} (PR debug/81307)

[Jakub Jelinek]

On Mon, Nov 27, 2017 at 01:36:12PM -0800, Jim Wilson wrote:
> On 11/24/2017 01:53 PM, Jakub Jelinek wrote:
> > Could anybody from the debugger folks test it a little bit (say gdb
> > testsuite if it has any -O2 -gstabs/-O2 -gstabs+ tests)?
> 
> The gdb testsuite just uses "-g" by default.  It doesn't have any tests with
> optimization by default, though this can be done.  I tried it.  I modified
> an x86_64-linux toolchain to emit stabs by default, and used it on the gdb
> testsuite.  I get about 40k passes, the same number with or without your
> patch.  When I try adding -O2, I get about 28k passes, and there are about
> 26 more with your patch than without.  I do see a few assembler errors
> without the patch, and none with.

Thanks.

> > @@ -3664,6 +3724,26 @@ dbx_output_rbrac (const char *label,
> >       dbxout_stab_value_label (label);
> >   }
> > +/* Return true is at least one block among BLOCK, its children or siblings
> 
> is -> if

Will fix.

> Otherwise this looks OK to me, though I think you went to more trouble than
> necessary.  I can think of 3 simpler ways to fix this.
> 1) In opts.c, check for DBX_DEBUG and -freorder-blocks-and-partition and
> disable the optimization.

We try hard not to change code generation based on -g* flags, this would be
against that.

> 2) In the x86 port, #undef DBX_DEBUGGING_INFO so you can't generate it
> anymore and hence won't test it anymore.

If we deprecate it now, guess that is the way to go for GCC 9.

> 3) In the testsuite, drop stabs from the debug torture list.  And maybe add
> dwarf-5 at the same time, as we really should be testing that.

Well, we already had -fno-reorder-blocks-and-partition in the only test
in */debug/* where it FAILed.  Not testing doesn't mean the problem doesn't
exist and the fails to assemble problem means it can be severe for anyone
who enables -gstabs*.  With the changes, even if the debugging experience in
the cold parts of the function might not be that good (e.g. due to the weird
function names etc.), at least debugging in the hot parts will work as before
without hot/cold partitioning.

	Jakub

Re: [PATCH] Fix hot/cold partitioning with -gstabs{,+} (PR debug/81307)

[Jim Wilson]

On 11/27/2017 12:21 AM, Richard Biener wrote:
> Let's formally deprecate any non-DWARF debugging format support
> in GCC 8 changes.html?

I think we need to be careful about that.  I've been looking at the 
stabs support after I finished removing the sdb/coff debug info support. 
  We have 3 cpu targets that only support stabs (avr, pdp11, vax).  We 
have two OS targets that only support stabs, though in both cases the 
cpus do support dwarf and hence they could switch.  We have two embedded 
targets that support the --with-stabs configure option.  We have a few 
targets that might default to stabs if used with old obsolete assembler 
versions that probably don't work anymore anyways.  And we have AIX 
which continues to use XCOFF_DEBUG which is a variant of stabs, and 
shares some of the same code.  We also have vms which apparently uses a 
mixture of vms and dwarf2 debugging.  Does that count as a non-DWARF 
debugging format?

I think a good first step would be to emit a compile-time warning if gcc 
was configured to emit stabs by default.  E.g. maybe something like
#if PREFERRED_DEBUGGING_TYPE == DBX_DEBUG
#warning "dbx/stabs debug info support is being deprecated"
#endif
in a popular header file.  That gives maintainers a chance to fix their 
ports.  I was planning to propose this after gcc-8, but we could maybe 
add it to gcc-8.  Then maybe in the next version this changes to a 
#error, so you can't build a gcc that emits stabs by default, but users 
can still get stabs with -gstabs.  We can then emit a run-time warning 
when a user uses -gstabs, and then later a run-time error when -gstabs 
is used.  That gives users a chance to migrate before it breaks.  Then 
we can remove -gstabs, careful to avoid breaking xcoff support, until 
IBM agrees in the futility of continuing to maintain it and lets us 
remove xcoff too.

Jim

Re: [PATCH] Fix hot/cold partitioning with -gstabs{,+} (PR debug/81307)

[Jim Wilson]

On 11/27/2017 01:54 PM, Jim Wilson wrote:
> On 11/27/2017 12:21 AM, Richard Biener wrote:
>> Let's formally deprecate any non-DWARF debugging format support
>> in GCC 8 changes.html?
>
>   We 
> have two OS targets that only support stabs, though in both cases the 
> cpus do support dwarf and hence they could switch.

After prodding my memory a bit, this is m68k-openbsd and *-lynxos.

There is also darwin9 support that apparently no one really cares about 
anymore.

This was just a quick survey of stabs usage on my part, so I could have 
gotten some details wrong.

Jim

Re: [PATCH] Fix hot/cold partitioning with -gstabs{,+} (PR debug/81307)

[Richard Biener]

On Mon, 27 Nov 2017, Jim Wilson wrote:

> On 11/27/2017 12:21 AM, Richard Biener wrote:
> > Let's formally deprecate any non-DWARF debugging format support
> > in GCC 8 changes.html?
> 
> I think we need to be careful about that.  I've been looking at the stabs
> support after I finished removing the sdb/coff debug info support.  We have 3
> cpu targets that only support stabs (avr, pdp11, vax).  We have two OS targets
> that only support stabs, though in both cases the cpus do support dwarf and
> hence they could switch.  We have two embedded targets that support the
> --with-stabs configure option.  We have a few targets that might default to
> stabs if used with old obsolete assembler versions that probably don't work
> anymore anyways.  And we have AIX which continues to use XCOFF_DEBUG which is
> a variant of stabs, and shares some of the same code.  We also have vms which
> apparently uses a mixture of vms and dwarf2 debugging.  Does that count as a
> non-DWARF debugging format?
> 
> I think a good first step would be to emit a compile-time warning if gcc was
> configured to emit stabs by default.  E.g. maybe something like
> #if PREFERRED_DEBUGGING_TYPE == DBX_DEBUG
> #warning "dbx/stabs debug info support is being deprecated"
> #endif
> in a popular header file.  That gives maintainers a chance to fix their ports.
> I was planning to propose this after gcc-8, but we could maybe add it to
> gcc-8.  Then maybe in the next version this changes to a #error, so you can't
> build a gcc that emits stabs by default, but users can still get stabs with
> -gstabs.  We can then emit a run-time warning when a user uses -gstabs, and
> then later a run-time error when -gstabs is used.  That gives users a chance
> to migrate before it breaks.  Then we can remove -gstabs, careful to avoid
> breaking xcoff support, until IBM agrees in the futility of continuing to
> maintain it and lets us remove xcoff too.

I think we want to decomission -gstabs for all targets that do not
default to it (or variants of it) and adjust documentation to
reflect that -gstabs is unmaintained and deprecated.  We also want to
deprecate --with-stabs if there is an alternative, preferable emit
a warning for this configure option for GCC 8 and remove support for GCC 9.

I think embedded folks stick to STABS because of its smaller size, not
because it is anywhere useful.

Richard.

Re: [PATCH] Fix hot/cold partitioning with -gstabs{,+} (PR debug/81307)

[Jeff Law]

On 11/27/2017 06:07 PM, Jim Wilson wrote:
> On 11/27/2017 01:54 PM, Jim Wilson wrote:
>> On 11/27/2017 12:21 AM, Richard Biener wrote:
>>> Let's formally deprecate any non-DWARF debugging format support
>>> in GCC 8 changes.html?
>>
>>   We have two OS targets that only support stabs, though in both cases
>> the cpus do support dwarf and hence they could switch.
> 
> After prodding my memory a bit, this is m68k-openbsd and *-lynxos.
I'm pretty sure the OpenBSD guys have moved to dwarf2 by default at some
point.  Certainly appears that way looking at my OpenBSD 6.2 VM.

In the case of LynxOS, I haven't used it in about 25 years.  I have no
clue if they've moved away from stabs yet.  However, I don't think we
should let that stand in the way of moving towards deprecation.

My biggest worry WRT deprecation of stabs is AIX.

jeff

Re: [PATCH] Fix hot/cold partitioning with -gstabs{,+} (PR debug/81307)

[Mike Stump]

On Nov 27, 2017, at 5:07 PM, Jim Wilson <jimw@sifive.com> wrote:
> There is also darwin9 support that apparently no one really cares about anymore.

I'm fine with removing stabs support from the compiler.