diff mbox

Add new -gmlt option for min. debug info with line tables (issue4440072)

Message ID 20110426224430.D1BC8481B3@thebrac.mtv.corp.google.com
State New
Headers show

Commit Message

Cary Coutant April 26, 2011, 10:44 p.m. UTC 
This patch adds a new option, -gmlt, that produces level 1 debug info
plus line number tables and inlined subroutine information. (The option
is short for "minimum line tables," taken from a similar feature of
HP's compilers.)

We've been using this option at Google for about a year now, and have
found it useful for collecting stack traces with file names and line
numbers without having to pay the much larger overhead for full debug
info. It's also useful for sample-based profiling, as discriminator
information is also available in the line number tables.

For optimized code, we've measured binaries built with -g0, -gmlt,
and -g2, and found that the total size of a binary compiled with -gmlt
is about 2.5x larger than one compiled without debug, while a binary
compiled with -g2 is about 6.7x larger. 

OK for trunk?

-cary


M	gcc/common.opt
M	gcc/doc/invoke.texi
M	gcc/dwarf2out.c
M	gcc/opts.c
A	gcc/testsuite/gcc.dg/debug/dwarf2/mlt1.c
A	gcc/testsuite/gcc.dg/debug/dwarf2/mlt2.c
M	gcc/tree-ssa-live.c

Tested:
  bootstrapped on x86_64.
  Added two new test cases.

gcc/ChangeLog:

	* common.opt (generate_debug_line_table): New global var.
	(gmlt): New option
	* dwarf2out.c (GENERATE_MINIMUM_LINE_TABLE): New macro.
	(add_pubname_string): Test for -gmlt.
	(add_pubname): Likewise.
	(add_src_coords_attributes): Likewise.
	(decls_for_scope): Likewise.
	(dwarf2out_source_line): Likewise.
	(dwarf2out_finish): Likewise.
	* opts.c (finish_options): Force debug info to at least level 1
	if -gmlt specified.
	(common_handle_option): Add OPT_gmlt.
	(set_debug_level): Set generate_debug_line_table flag.
	* tree-ssa-live.c (remove_unused_scope_block_p): Test for -gmlt.

	* doc/invoke.texi (-gmlt): New options

gcc/testsuite/ChangeLog:

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


--
This patch is available for review at http://codereview.appspot.com/4440072

Comments

Andrew Pinski April 26, 2011, 10:47 p.m. UTC | #1 
On Tue, Apr 26, 2011 at 3:44 PM, Cary Coutant <ccoutant@google.com> wrote:
> This patch adds a new option, -gmlt, that produces level 1 debug info
> plus line number tables and inlined subroutine information. (The option
> is short for "minimum line tables," taken from a similar feature of
> HP's compilers.)

What is the difference between -gmlt and -g1?  And why can't this just
be enabled for -g1?

Thanks,
Andrew Pinski
Cary Coutant April 26, 2011, 10:57 p.m. UTC | #2 
>> This patch adds a new option, -gmlt, that produces level 1 debug info
>> plus line number tables and inlined subroutine information. (The option
>> is short for "minimum line tables," taken from a similar feature of
>> HP's compilers.)
>
> What is the difference between -gmlt and -g1?  And why can't this just
> be enabled for -g1?

With -g1, you don't get line number tables or any inlined subroutine
information. Here's the description of -g1 in the docs:

"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."

I considered just changing -g1 to do this, with the argument that
"enough for making backtraces" ought to include line numbers and
inlined function calls, but I wasn't familiar enough with existing
uses of -g1 to go with that alternative. I'm certainly willing to go
that route, though, if that's preferable.

If we do go with extending -g1, though. we want to be able to turn it
on in our build scripts, yet allow a later "-g" option in the user's
compiler options to enable full debug info. In this patch, I've made
sure that "-gmlt -g" is equivalent to "-g2", whereas "-g1 -g" is the
same as "-g1" by itself. (It's too much to train people to use "-g2"
instead of "-g" when they want full debug.)

-cary
Andrew Pinski April 26, 2011, 11 p.m. UTC | #3 
On Tue, Apr 26, 2011 at 3:57 PM, Cary Coutant <ccoutant@google.com> wrote:
> If we do go with extending -g1, though. we want to be able to turn it
> on in our build scripts, yet allow a later "-g" option in the user's
> compiler options to enable full debug info. In this patch, I've made
> sure that "-gmlt -g" is equivalent to "-g2", whereas "-g1 -g" is the
> same as "-g1" by itself. (It's too much to train people to use "-g2"
> instead of "-g" when they want full debug.)
>

Well I think -g1 -g should act like how -O2 -O works.  That is should be -g2.

Thanks,
Andrew Pinski
Cary Coutant April 26, 2011, 11:09 p.m. UTC | #4 
> Well I think -g1 -g should act like how -O2 -O works.  That is should be -g2.

I think so, too, but I guess I was too timid to assume such a change
might be acceptable. If the maintainers would go along with that, I'd
have no problem revising the patch to make -g1 do line tables and
inline info. (I guess I'd have to add -gmlt as an alias for -g1 on the
google/main branch for compatibility, though -- not that that's a big
deal.)

-cary
Joseph Myers April 27, 2011, 10:07 a.m. UTC | #5 
On Tue, 26 Apr 2011, Cary Coutant wrote:

> @@ -1856,6 +1871,8 @@ set_debug_level (enum debug_info_type type, int extended, const char *arg,
>        else
>  	opts->x_debug_info_level = (enum debug_info_levels) argval;
>      }
> +
> +  generate_debug_line_table = debug_info_level >= DINFO_LEVEL_NORMAL;

set_debug_level should not use global state; this needs to check 
opts->x_debug_info_level (not the global debug_info_level) and set 
opts->x_generate_debug_line_table.
Cary Coutant April 27, 2011, 5:45 p.m. UTC | #6 
>> +  generate_debug_line_table = debug_info_level >= DINFO_LEVEL_NORMAL;
>
> set_debug_level should not use global state; this needs to check
> opts->x_debug_info_level (not the global debug_info_level) and set
> opts->x_generate_debug_line_table.

Oops, missed that. Thanks!

-cary
Jason Merrill May 3, 2011, 8:24 p.m. UTC | #7 
On 04/26/2011 06:57 PM, Cary Coutant wrote:
> I considered just changing -g1 to do this, with the argument that
> "enough for making backtraces" ought to include line numbers and
> inlined function calls, but I wasn't familiar enough with existing
> uses of -g1 to go with that alternative. I'm certainly willing to go
> that route, though, if that's preferable.

That makes sense to me; it seems appropriate for -g1 to have information 
that makes a backtrace more informative, but not information for 
interactive debugging.  Jim, do you have an opinion?

Jason
Cary Coutant May 6, 2011, 5:43 p.m. UTC | #8 
Ping.

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

Should I prepare a patch that does this at -g1, and makes "-g1 -g" the
same as "-g2"?

-cary


On Tue, May 3, 2011 at 1:24 PM, Jason Merrill <jason@redhat.com> wrote:
> On 04/26/2011 06:57 PM, Cary Coutant wrote:
>>
>> I considered just changing -g1 to do this, with the argument that
>> "enough for making backtraces" ought to include line numbers and
>> inlined function calls, but I wasn't familiar enough with existing
>> uses of -g1 to go with that alternative. I'm certainly willing to go
>> that route, though, if that's preferable.
>
> That makes sense to me; it seems appropriate for -g1 to have information
> that makes a backtrace more informative, but not information for interactive
> debugging.  Jim, do you have an opinion?
>
> Jason
>
Jim Wilson May 9, 2011, 8:48 p.m. UTC | #9 
On Tue, 2011-05-03 at 16:24 -0400, Jason Merrill wrote:
> That makes sense to me; it seems appropriate for -g1 to have information 
> that makes a backtrace more informative, but not information for 
> interactive debugging.  Jim, do you have an opinion?

I'm not aware of any significant use of -g1.  It is very rare for anyone
to mention it in a bug report for instance.  Once upon a time (before
2002-03-19), it was used for compiling libgcc, but that was just to
ensure that it got tested somewhere.  From my Cisco experience, I would
agree that backtraces without line numbers are not very useful.  It
would be OK with me if these changes were added to -g1 instead of
creating a new -gmlt option.

Jim
diff mbox

Patch

diff --git a/gcc/common.opt b/gcc/common.opt
index 83a61fc..2964d61 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -120,6 +120,11 @@  enum debug_info_type write_symbols = NO_DEBUG
 Variable
 enum debug_info_levels debug_info_level = DINFO_LEVEL_NONE
 
+; Whether to generate line number table.  Normally set at DINFO_LEVEL_NORMAL
+; or above; can also be set for DINFO_LEVEL_TERSE with -gmlt.
+Variable
+bool generate_debug_line_table = false
+
 ; Nonzero means use GNU-only extensions in the generated symbolic
 ; debugging information.  Currently, this only has an effect when
 ; write_symbols is set to DBX_DEBUG, XCOFF_DEBUG, or DWARF_DEBUG.
@@ -2150,6 +2155,10 @@  ggdb
 Common JoinedOrMissing
 Generate debug information in default extended format
 
+gmlt
+Common RejectNegative
+Generate debug information at level 1 with minimal line table
+
 gstabs
 Common JoinedOrMissing Negative(gstabs+)
 Generate debug information in STABS format
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 4377f34..b199253 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -320,7 +320,7 @@  Objective-C and Objective-C++ Dialects}.
 -fstack-usage  -ftest-coverage  -ftime-report -fvar-tracking @gol
 -fvar-tracking-assignments  -fvar-tracking-assignments-toggle @gol
 -g  -g@var{level}  -gtoggle  -gcoff  -gdwarf-@var{version} @gol
--ggdb  -gstabs  -gstabs+  -gstrict-dwarf  -gno-strict-dwarf @gol
+-ggdb  -gmlt  -gstabs  -gstabs+  -gstrict-dwarf  -gno-strict-dwarf @gol
 -gvms  -gxcoff  -gxcoff+ @gol
 -fno-merge-debug-strings -fno-dwarf2-cfi-asm @gol
 -fdebug-prefix-map=@var{old}=@var{new} @gol
@@ -4679,6 +4679,11 @@  debug format is long obsolete, but the option cannot be changed now.
 Instead use an additional @option{-g@var{level}} option to change the
 debug level for DWARF.
 
+@item -gmlt
+@opindex gmlt
+Produce a minimal line table, with level 1 debugging information plus
+information about inlined functions and line numbers.
+
 @item -gtoggle
 @opindex gtoggle
 Turn off generation of debug info, if leaving out this option would have
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index f3c4c09..4506f7f 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -113,6 +113,10 @@  int vms_file_stats_name (const char *, long long *, long *, char *, int *);
 #define DWARF2_INDIRECT_STRING_SUPPORT_MISSING_ON_TARGET 0
 #endif
 
+/* True if generating only the minimum line table (-gmlt).  */
+#define GENERATE_MINIMUM_LINE_TABLE (debug_info_level == DINFO_LEVEL_TERSE \
+				     && generate_debug_line_table)
+
 /* ??? Poison these here until it can be done generically.  They've been
    totally replaced in this file; make sure it stays that way.  */
 #undef DWARF2_UNWIND_INFO
@@ -11639,7 +11643,7 @@  dwarf2_name (tree decl, int scope)
 static void
 add_pubname_string (const char *str, dw_die_ref die)
 {
-  if (targetm.want_debug_pub_sections)
+  if (!GENERATE_MINIMUM_LINE_TABLE && targetm.want_debug_pub_sections)
     {
       pubname_entry e;
 
@@ -11652,7 +11656,9 @@  add_pubname_string (const char *str, dw_die_ref die)
 static void
 add_pubname (tree decl, dw_die_ref die)
 {
-  if (targetm.want_debug_pub_sections && TREE_PUBLIC (decl))
+  if (!GENERATE_MINIMUM_LINE_TABLE
+      && targetm.want_debug_pub_sections
+      && TREE_PUBLIC (decl))
     {
       const char *name = dwarf2_name (decl, 1);
       if (name)
@@ -17837,11 +17843,12 @@  add_src_coords_attributes (dw_die_ref die, tree decl)
 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 (!GENERATE_MINIMUM_LINE_TABLE
+      && (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))
@@ -20911,14 +20918,18 @@  decls_for_scope (tree stmt, dw_die_ref context_die, int depth)
      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);
+  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)
+  /* If we're at -g1 and not generating minimal line tables,
+     we're not interested in subblocks.  */
+  if (!generate_debug_line_table && debug_info_level <= DINFO_LEVEL_TERSE)
     return;
 
   /* Output the DIEs to represent all sub-blocks (and the items declared
@@ -22189,7 +22200,7 @@  dwarf2out_source_line (unsigned int line, const char *filename,
   unsigned int file_num;
   dw_line_info_table *table;
 
-  if (debug_info_level < DINFO_LEVEL_NORMAL || line == 0)
+  if (!generate_debug_line_table || line == 0)
     return;
 
   /* The discriminator column was added in dwarf4.  Simplify the below
@@ -23719,7 +23730,7 @@  dwarf2out_finish (const char *filename)
 	}
     }
 
-  if (debug_info_level >= DINFO_LEVEL_NORMAL)
+  if (generate_debug_line_table)
     add_AT_lineptr (comp_unit_die (), DW_AT_stmt_list,
 		    debug_line_section_label);
 
@@ -23746,7 +23757,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 (generate_debug_line_table)
         add_AT_lineptr (ctnode->root_die, DW_AT_stmt_list,
 		        debug_line_section_label);
 
diff --git a/gcc/opts.c b/gcc/opts.c
index cd581f6..72f4d51 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -609,6 +609,11 @@  finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
 {
   enum unwind_info_type ui_except;
 
+  /* If -gmlt was specified, make sure debug level is at least 1.  */
+  if (opts->x_generate_debug_line_table
+      && opts->x_debug_info_level < DINFO_LEVEL_TERSE)
+    opts->x_debug_info_level = DINFO_LEVEL_TERSE;
+
   if (opts->x_dump_base_name && ! IS_ABSOLUTE_PATH (opts->x_dump_base_name))
     {
       /* First try to make OPTS->X_DUMP_BASE_NAME relative to the
@@ -1649,6 +1654,16 @@  common_handle_option (struct gcc_options *opts,
 		       loc);
       break;
 
+    case OPT_gmlt:
+      set_debug_level (NO_DEBUG, DEFAULT_GDB_EXTENSIONS, "", opts, opts_set,
+		       loc);
+      /* Clear the debug level to NONE so that a subsequent bare -g will
+	 set it to NORMAL (level 2).  If no subsequent option sets the
+	 level explicitly, we will set it to TERSE in finish_options().  */
+      opts->x_debug_info_level = DINFO_LEVEL_NONE;
+      opts->x_generate_debug_line_table = true;
+      break;
+
     case OPT_gvms:
       set_debug_level (VMS_DEBUG, false, arg, opts, opts_set, loc);
       break;
@@ -1856,6 +1871,8 @@  set_debug_level (enum debug_info_type type, int extended, const char *arg,
       else
 	opts->x_debug_info_level = (enum debug_info_levels) argval;
     }
+
+  generate_debug_line_table = debug_info_level >= DINFO_LEVEL_NORMAL;
 }
 
 /* Arrange to dump core on error for diagnostic context DC.  (The
diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/mlt1.c b/gcc/testsuite/gcc.dg/debug/dwarf2/mlt1.c
new file mode 100644
index 0000000..7b0b2e9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/mlt1.c
@@ -0,0 +1,32 @@ 
+/* Test that -gmlt 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 -gmlt" } */
+/* { 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;
+}
diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/mlt2.c b/gcc/testsuite/gcc.dg/debug/dwarf2/mlt2.c
new file mode 100644
index 0000000..bd7ad05
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/mlt2.c
@@ -0,0 +1,31 @@ 
+/* Test that -g overrides -gmlt.  */
+/* Origin: Cary Coutant  <ccoutant@google.com> */
+/* { dg-do compile } */
+/* { dg-options "-O2 -gdwarf-2 -dA -gmlt -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;
+}
diff --git a/gcc/tree-ssa-live.c b/gcc/tree-ssa-live.c
index 4216b22..6a88236 100644
--- a/gcc/tree-ssa-live.c
+++ b/gcc/tree-ssa-live.c
@@ -548,8 +548,9 @@  remove_unused_scope_block_p (tree scope)
    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)
+   else if (!generate_debug_line_table
+	    && (debug_info_level == DINFO_LEVEL_NONE
+		|| debug_info_level == DINFO_LEVEL_TERSE))
      {
        /* Even for -g0/-g1 don't prune outer scopes from artificial
 	  functions, otherwise diagnostics using tree_nonartificial_location