diff mbox

[RFC] Modify -g1 to produce line tables

Message ID CAHACq4pO37aZtdiBGzbCW6TYGCLdboU6SYfgVjHVcrOf+FQa2Q@mail.gmail.com
State New
Headers show

Commit Message

Cary Coutant Feb. 6, 2013, 7:51 p.m. UTC 
A long time ago, I proposed a -gmlt option to generate "minimal line
tables" (basically -g1 + line tables):

http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02075.html

The consensus from that thread was that instead of a new option, we
should just modify -g1 to produce line tables (the documented purpose
of -g1 is to enable stack traces, and stack traces are much more
useful with line numbers). I also proposed, and got no objection, that
-g by itself should set the debugging level to 2 (i.e., -g1 -g should
behave the same as -g1 -g2).

Here, finally, is that patch again, reworked to generate line tables
at -g1. I plan to commit this when Stage 1 reopens, but I'd like to
verify that earlier consensus. I also plan to commit this to the
google/main branch, and future merges will go more smoothly if what I
put in google/main matches what eventually goes into trunk.

-cary

2013-02-06   Cary Coutant  <ccoutant@google.com>

gcc/
        * dwarf2out.c (want_pubnames): Don't do pubnames for -g1.
        (add_linkage_name): Don't add linkage name for -g1.
        (decls_for_scope): Process subblocks for -g1.
        (dwarf2out_source_line): Output line tables for -g1.
        (dwarf2out_finish): Likewise.
        * tree-ssa-live.c (remove_unused_scope_block_p): Don't prune
        unused scopes for -g1.
        * opts.c (common_handle_option): Handle -g same as -g2.
        * doc/invoke.texi: Update description for -g1.

gcc/testsuite/
        * gcc.dg/debug/dwarf2/mlt1.c: New test.
        * gcc.dg/debug/dwarf2/mlt2.c: New test.
2013-02-06   Cary Coutant  <ccoutant@google.com>

gcc/
	* dwarf2out.c (want_pubnames): Don't do pubnames for -g1.
	(add_linkage_name): Don't add linkage name for -g1.
	(decls_for_scope): Process subblocks for -g1.
	(dwarf2out_source_line): Output line tables for -g1.
	(dwarf2out_finish): Likewise.
	* tree-ssa-live.c (remove_unused_scope_block_p): Don't prune
	unused scopes for -g1.
	* opts.c (common_handle_option): Handle -g same as -g2.
	* doc/invoke.texi: Update description for -g1.

gcc/testsuite/
	* gcc.dg/debug/dwarf2/mlt1.c: New test.
	* gcc.dg/debug/dwarf2/mlt2.c: New test.

Comments

Andrew Pinski Nov. 20, 2013, 11:18 p.m. UTC | #1 
On Wed, Feb 6, 2013 at 11:51 AM, Cary Coutant <ccoutant@google.com> wrote:
> A long time ago, I proposed a -gmlt option to generate "minimal line
> tables" (basically -g1 + line tables):
>
> http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02075.html
>
> The consensus from that thread was that instead of a new option, we
> should just modify -g1 to produce line tables (the documented purpose
> of -g1 is to enable stack traces, and stack traces are much more
> useful with line numbers). I also proposed, and got no objection, that
> -g by itself should set the debugging level to 2 (i.e., -g1 -g should
> behave the same as -g1 -g2).
>
> Here, finally, is that patch again, reworked to generate line tables
> at -g1. I plan to commit this when Stage 1 reopens, but I'd like to
> verify that earlier consensus. I also plan to commit this to the
> google/main branch, and future merges will go more smoothly if what I
> put in google/main matches what eventually goes into trunk.

Hmm,  Stage 1 has been opened for a while now but I could not find
this patch has been committed yet.  Is there any plans to include this
patch?  It would be useful for Sanitizer and other uses.

Thanks,
Andrew Pinski


>
> -cary
>
> 2013-02-06   Cary Coutant  <ccoutant@google.com>
>
> gcc/
>         * dwarf2out.c (want_pubnames): Don't do pubnames for -g1.
>         (add_linkage_name): Don't add linkage name for -g1.
>         (decls_for_scope): Process subblocks for -g1.
>         (dwarf2out_source_line): Output line tables for -g1.
>         (dwarf2out_finish): Likewise.
>         * tree-ssa-live.c (remove_unused_scope_block_p): Don't prune
>         unused scopes for -g1.
>         * opts.c (common_handle_option): Handle -g same as -g2.
>         * doc/invoke.texi: Update description for -g1.
>
> gcc/testsuite/
>         * gcc.dg/debug/dwarf2/mlt1.c: New test.
>         * gcc.dg/debug/dwarf2/mlt2.c: New test.
Cary Coutant Nov. 20, 2013, 11:30 p.m. UTC | #2 
>> Here, finally, is that patch again, reworked to generate line tables
>> at -g1. I plan to commit this when Stage 1 reopens, but I'd like to
>> verify that earlier consensus. I also plan to commit this to the
>> google/main branch, and future merges will go more smoothly if what I
>> put in google/main matches what eventually goes into trunk.
>
> Hmm,  Stage 1 has been opened for a while now but I could not find
> this patch has been committed yet.  Is there any plans to include this
> patch?  It would be useful for Sanitizer and other uses.

Sorry, I never saw any feedback, positive or negative, on that, and it
kind of fell off my radar. I think it should still be ready to go in
-- Stage 1 is still open for another day, right? Let me rebase the
patch, kick off a bootstrap and regression tests, and I think I can be
ready to submit it if there are no objections.

-cary
Jeff Law Nov. 20, 2013, 11:39 p.m. UTC | #3 
On 11/20/13 16:30, Cary Coutant wrote:
>>> Here, finally, is that patch again, reworked to generate line tables
>>> at -g1. I plan to commit this when Stage 1 reopens, but I'd like to
>>> verify that earlier consensus. I also plan to commit this to the
>>> google/main branch, and future merges will go more smoothly if what I
>>> put in google/main matches what eventually goes into trunk.
>>
>> Hmm,  Stage 1 has been opened for a while now but I could not find
>> this patch has been committed yet.  Is there any plans to include this
>> patch?  It would be useful for Sanitizer and other uses.
>
> Sorry, I never saw any feedback, positive or negative, on that, and it
> kind of fell off my radar. I think it should still be ready to go in
> -- Stage 1 is still open for another day, right? Let me rebase the
> patch, kick off a bootstrap and regression tests, and I think I can be
> ready to submit it if there are no objections.
Yea, you've still got another day.  So definitely rebase and resubmit.

 From a review standpoint, things got really bad.  If you had something 
get dropped, definitely ping it.  As a whole, we're doing better now 
than probably anytime this year, but as always we could do better.


jeff
Cary Coutant Nov. 20, 2013, 11:48 p.m. UTC | #4 
>> Sorry, I never saw any feedback, positive or negative, on that, and it
>> kind of fell off my radar. I think it should still be ready to go in
>> -- Stage 1 is still open for another day, right? Let me rebase the
>> patch, kick off a bootstrap and regression tests, and I think I can be
>> ready to submit it if there are no objections.
>
> Yea, you've still got another day.  So definitely rebase and resubmit.
>
> From a review standpoint, things got really bad.  If you had something get
> dropped, definitely ping it.  As a whole, we're doing better now than
> probably anytime this year, but as always we could do better.

Sorry, I didn't mean to make it sound like I was blaming any potential
reviewers -- it fell off of my radar (even though I got reminded
occasionally about it by Dehao Chen, who really wants this in trunk).
Totally my failure.

-cary
Jeff Law Nov. 21, 2013, 3:22 a.m. UTC | #5 
On 11/20/13 16:48, Cary Coutant wrote:
>>> Sorry, I never saw any feedback, positive or negative, on that, and it
>>> kind of fell off my radar. I think it should still be ready to go in
>>> -- Stage 1 is still open for another day, right? Let me rebase the
>>> patch, kick off a bootstrap and regression tests, and I think I can be
>>> ready to submit it if there are no objections.
>>
>> Yea, you've still got another day.  So definitely rebase and resubmit.
>>
>>  From a review standpoint, things got really bad.  If you had something get
>> dropped, definitely ping it.  As a whole, we're doing better now than
>> probably anytime this year, but as always we could do better.
>
> Sorry, I didn't mean to make it sound like I was blaming any potential
> reviewers -- it fell off of my radar (even though I got reminded
> occasionally about it by Dehao Chen, who really wants this in trunk).
> Totally my failure.
No worries.  My views on our patch review backlog are based on a variety 
of observations and a few complaints :-)  I didn't see your message as a 
complaint.

jeff
diff mbox

Patch

Index: dwarf2out.c
===================================================================
--- dwarf2out.c	(revision 195743)
+++ dwarf2out.c	(working copy)
@@ -8650,9 +8650,11 @@  output_comp_unit (dw_die_ref die, int ou
 static inline bool
 want_pubnames (void)
 {
-  return (debug_generate_pub_sections != -1
-	  ? debug_generate_pub_sections
-	  : targetm.want_debug_pub_sections);
+  if (debug_info_level <= DINFO_LEVEL_TERSE)
+    return false;
+  if (debug_generate_pub_sections != -1)
+    return debug_generate_pub_sections;
+  return targetm.want_debug_pub_sections;
 }
 
 /* Add the DW_AT_GNU_pubnames and DW_AT_GNU_pubtypes attributes.  */
@@ -16222,11 +16224,12 @@  add_src_coords_attributes (dw_die_ref di
 static void
 add_linkage_name (dw_die_ref die, tree decl)
 {
-  if ((TREE_CODE (decl) == FUNCTION_DECL || TREE_CODE (decl) == VAR_DECL)
-       && TREE_PUBLIC (decl)
-       && !DECL_ABSTRACT (decl)
-       && !(TREE_CODE (decl) == VAR_DECL && DECL_REGISTER (decl))
-       && die->die_tag != DW_TAG_member)
+  if (debug_info_level > DINFO_LEVEL_TERSE
+      && (TREE_CODE (decl) == FUNCTION_DECL || TREE_CODE (decl) == VAR_DECL)
+      && TREE_PUBLIC (decl)
+      && !DECL_ABSTRACT (decl)
+      && !(TREE_CODE (decl) == VAR_DECL && DECL_REGISTER (decl))
+      && die->die_tag != DW_TAG_member)
     {
       /* Defer until we have an assembler name set.  */
       if (!DECL_ASSEMBLER_NAME_SET_P (decl))
@@ -19609,16 +19612,19 @@  decls_for_scope (tree stmt, dw_die_ref c
   /* Output the DIEs to represent all of the data objects and typedefs
      declared directly within this block but not within any nested
      sub-blocks.  Also, nested function and tag DIEs have been
-     generated with a parent of NULL; fix that up now.  */
-  for (decl = BLOCK_VARS (stmt); decl != NULL; decl = DECL_CHAIN (decl))
-    process_scope_var (stmt, decl, NULL_TREE, context_die);
-  for (i = 0; i < BLOCK_NUM_NONLOCALIZED_VARS (stmt); i++)
-    process_scope_var (stmt, NULL, BLOCK_NONLOCALIZED_VAR (stmt, i),
-    		       context_die);
+     generated with a parent of NULL; fix that up now.  We don't
+     have to do this if we're at -g1.  */
+  if (debug_info_level > DINFO_LEVEL_TERSE)
+    {
+      for (decl = BLOCK_VARS (stmt); decl != NULL; decl = DECL_CHAIN (decl))
+	process_scope_var (stmt, decl, NULL_TREE, context_die);
+      for (i = 0; i < BLOCK_NUM_NONLOCALIZED_VARS (stmt); i++)
+	process_scope_var (stmt, NULL, BLOCK_NONLOCALIZED_VAR (stmt, i),
+			   context_die);
+    }
 
-  /* If we're at -g1, we're not interested in subblocks.  */
-  if (debug_info_level <= DINFO_LEVEL_TERSE)
-    return;
+  /* Even if we're at -g1, we need to process the subblocks in order to get
+     inlined call information.  */
 
   /* Output the DIEs to represent all sub-blocks (and the items declared
      therein) of this block.  */
@@ -20966,7 +20972,7 @@  dwarf2out_source_line (unsigned int line
   unsigned int file_num;
   dw_line_info_table *table;
 
-  if (debug_info_level < DINFO_LEVEL_NORMAL || line == 0)
+  if (debug_info_level < DINFO_LEVEL_TERSE || line == 0)
     return;
 
   /* The discriminator column was added in dwarf4.  Simplify the below
@@ -23460,7 +23466,7 @@  dwarf2out_finish (const char *filename)
 	}
     }
 
-  if (debug_info_level >= DINFO_LEVEL_NORMAL)
+  if (debug_info_level >= DINFO_LEVEL_TERSE)
     add_AT_lineptr (main_comp_unit_die, DW_AT_stmt_list,
 		    debug_line_section_label);
 
@@ -23508,7 +23514,7 @@  dwarf2out_finish (const char *filename)
       /* Add a pointer to the line table for the main compilation unit
          so that the debugger can make sense of DW_AT_decl_file
          attributes.  */
-      if (debug_info_level >= DINFO_LEVEL_NORMAL)
+      if (debug_info_level >= DINFO_LEVEL_TERSE)
         add_AT_lineptr (ctnode->root_die, DW_AT_stmt_list,
                         (!dwarf_split_debug_info
                          ? debug_line_section_label
Index: tree-ssa-live.c
===================================================================
--- tree-ssa-live.c	(revision 195743)
+++ tree-ssa-live.c	(working copy)
@@ -564,11 +564,11 @@  remove_unused_scope_block_p (tree scope)
       eliminated.  */
    else if (!nsubblocks)
      ;
-   /* For terse debug info we can eliminate info on unused variables.  */
-   else if (debug_info_level == DINFO_LEVEL_NONE
-	    || debug_info_level == DINFO_LEVEL_TERSE)
+   /* When not generating debug info we can eliminate info on unused
+      variables.  */
+   else if (debug_info_level == DINFO_LEVEL_NONE)
      {
-       /* Even for -g0/-g1 don't prune outer scopes from artificial
+       /* Even for -g0 don't prune outer scopes from artificial
 	  functions, otherwise diagnostics using tree_nonartificial_location
 	  will not be emitted properly.  */
        if (inlined_function_outer_scope_p (scope))
Index: opts.c
===================================================================
--- opts.c	(revision 195743)
+++ opts.c	(working copy)
@@ -1701,7 +1701,17 @@  common_handle_option (struct gcc_options
       break;
 
     case OPT_g:
-      set_debug_level (NO_DEBUG, DEFAULT_GDB_EXTENSIONS, arg, opts, opts_set,
- 		       loc);
+      /* -g by itself should force -g2.  */
+      if (*arg == '\0')
+	set_debug_level (NO_DEBUG, DEFAULT_GDB_EXTENSIONS, "2", opts, opts_set,
+			 loc);
+      else
+	set_debug_level (NO_DEBUG, DEFAULT_GDB_EXTENSIONS, arg, opts, opts_set,
+			 loc);
+      break;
 
Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 195743)
+++ doc/invoke.texi	(working copy)
@@ -5098,8 +5098,8 @@  Level 0 produces no debug information at
 
 Level 1 produces minimal information, enough for making backtraces in
 parts of the program that you don't plan to debug.  This includes
-descriptions of functions and external variables, but no information
-about local variables and no line numbers.
+descriptions of functions and external variables, and line number
+tables, but no information about local variables.
 
 Level 3 includes extra information, such as all the macro definitions
 present in the program.  Some debuggers support macro expansion when
Index: testsuite/gcc.dg/debug/dwarf2/mlt1.c
===================================================================
--- testsuite/gcc.dg/debug/dwarf2/mlt1.c	(revision 0)
+++ testsuite/gcc.dg/debug/dwarf2/mlt1.c	(revision 0)
@@ -0,0 +1,32 @@ 
+/* Test that -g1 includes line tables and inlined subroutine entries,
+   and excludes types and variables.  */
+/* Origin: Cary Coutant  <ccoutant@google.com> */
+/* { dg-do compile } */
+/* { dg-options "-O2 -gdwarf-2 -dA -g1" } */
+/* { dg-final { scan-assembler "DW_AT_stmt_list" } } */
+/* { dg-final { scan-assembler "DW_TAG_subprogram" } } */
+/* { dg-final { scan-assembler "DW_TAG_inlined_subroutine" } } */
+/* { dg-final { scan-assembler-not "DW_TAG_variable" } } */
+/* { dg-final { scan-assembler-not "DW_TAG_formal_parameter" } } */
+/* { dg-final { scan-assembler-not "DW_TAG_base_type" } } */
+
+static inline __attribute__((always_inline)) int
+a(int i, int j)
+{
+  return (i << 5) + j;
+}
+
+int
+b(int i, int j)
+{
+  return (i >> 5) + (j << 27);
+}
+
+int
+c(int i, int j)
+{
+  int r = a(i, j);
+  r = b(r, i);
+  r = b(r, j);
+  return r;
+}
Index: testsuite/gcc.dg/debug/dwarf2/mlt2.c
===================================================================
--- testsuite/gcc.dg/debug/dwarf2/mlt2.c	(revision 0)
+++ testsuite/gcc.dg/debug/dwarf2/mlt2.c	(revision 0)
@@ -0,0 +1,31 @@ 
+/* Test that -g overrides -g1.  */
+/* Origin: Cary Coutant  <ccoutant@google.com> */
+/* { dg-do compile } */
+/* { dg-options "-O2 -gdwarf-2 -dA -g1 -g" } */
+/* { dg-final { scan-assembler "DW_AT_stmt_list" } } */
+/* { dg-final { scan-assembler "DW_TAG_subprogram" } } */
+/* { dg-final { scan-assembler "DW_TAG_inlined_subroutine" } } */
+/* { dg-final { scan-assembler "DW_TAG_variable" } } */
+/* { dg-final { scan-assembler "DW_TAG_formal_parameter" } } */
+/* { dg-final { scan-assembler "DW_TAG_base_type" } } */
+
+static inline __attribute__((always_inline)) int
+a(int i, int j)
+{
+  return (i << 5) + j;
+}
+
+int
+b(int i, int j)
+{
+  return (i >> 5) + (j << 27);
+}
+
+int
+c(int i, int j)
+{
+  int r = a(i, j);
+  r = b(r, i);
+  r = b(r, j);
+  return r;
+}