diff mbox

[RFC/RFA] Add support for the --readnever command-line option (DWARF only)

Message ID 1467838463-15786-1-git-send-email-brobecker@adacore.com
State New, archived
Headers

Commit Message

Joel Brobecker July 6, 2016, 8:54 p.m. UTC 
  Hello,

One of our customers asked us about this option, which they could
see as being available in the version of GDB shipped by RedHat but
not in the version that AdaCore supports.

The purpose of this option is to turn the load of debugging information
off. The implementation proposed here is mostly a copy of the patch
distributed with RedHat Fedora, and looking at the patch itself and
the history, I can see some reasons why it was never submitted:
  - The patch appears to have been introduced as a workaround, at
    least initially;
  - The patch is far from perfect, as it simply shunts the load of
    DWARF debugging information, without really worrying about the
    other debug format.
  - Who really does non-symbolic debugging anyways?

One use of this when a user simply wants to do the following
sequence: attach, dump core, detach. Loading the debugging information
in this case is an unnecessary cause of delay.

I started looking at a more general way of implementing this feature.
For instance, I was hoping for a patch maybe at the objfile reader
level, or even better at the generic part of the objfile reader.
But it turns out this is not trivial at all.  The loading of debugging
information appears to be done as part of either the sym_fns->sym_read
(most cases), or during the sym_fns->sym_read_psymbols phase ("lazy
loading", ELF). Reading the code there, it wasn't obvious to me
what the consequence would be to add some code to ignore debugging
information, and since I would not be able to test those changes,
I opted towards touching only the targets that use DWARF.

This is why I ended up choosing the same approach as RedHat.
It's far from perfect, but has the benefit of working for
what I hope is the vast majority of people, and being fairly
unintrusive. I thought, since it's useful to AdaCore, and it's
been useful to RedHat, maybe others might find it useful, so
here it is.

The changes I made to the patch from Fedora are:

      - dwarf2_has_info: Return immediately if readnever_symbol_files
                         is set - faster return, and easier to read, IMO;
      - main.c: Update the --help output to mention that this feature
                only supports DWARF;
      - gdb.texinfo: Add a paragraph to explain that this option only
        supports DWARF.

If you guys don't think it's a good idea, then I'll understand
(hence the RFC).  At least we'll have it in the archives, in case
someone else wants it.

gdb/ChangeLog:

        Andrew Cagney  <cagney@redhat.com>
        Joel Brobecker  <brobecker@adacore.com>
        * symfile.c (readnever_symbol_files): New global.
        * top.h (readnever_symbol_files): New extern global.
        * main.c (captured_main): Add support for --readnever.
        (print_gdb_help): Document --readnever.
        * dwarf2read.c: #include "top.h".
        (dwarf2_has_info): Return 0 if READNEVER_SYMBOL_FILES is set.

gdb/doc/ChangeLog:

        Andrew Cagney  <cagney@redhat.com>
        Joel Brobecker  <brobecker@adacore.com>
        * gdb.texinfo (File Options): Document --readnever.

gdb/testsuite/ChangeLog:

        Joel Brobecker  <brobecker@adacore.com>
        * gdb.base/readnever.c, gdb.base/readnever.exp: New files.

Tested on x86_64-linux.


---
 gdb/doc/gdb.texinfo                  |  8 ++++++
 gdb/dwarf2read.c                     |  4 +++
 gdb/main.c                           |  4 +++
 gdb/symfile.c                        |  1 +
 gdb/testsuite/gdb.base/readnever.c   | 39 +++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/readnever.exp | 48 ++++++++++++++++++++++++++++++++++++
 gdb/top.h                            |  1 +
 7 files changed, 105 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/readnever.c
 create mode 100644 gdb/testsuite/gdb.base/readnever.exp

Comments

Yao Qi July 12, 2016, 2:27 p.m. UTC | #1 
Hi Joel,

On Wed, Jul 6, 2016 at 9:54 PM, Joel Brobecker <brobecker@adacore.com> wrote:
> Hello,
>
> One of our customers asked us about this option, which they could
> see as being available in the version of GDB shipped by RedHat but
> not in the version that AdaCore supports.
>
> The purpose of this option is to turn the load of debugging information
> off. The implementation proposed here is mostly a copy of the patch
> distributed with RedHat Fedora, and looking at the patch itself and
> the history, I can see some reasons why it was never submitted:

Andrew Cagney posted the patch
https://www.sourceware.org/ml/gdb/2004-11/msg00216.html

>   - The patch appears to have been introduced as a workaround, at
>     least initially;
>   - The patch is far from perfect, as it simply shunts the load of
>     DWARF debugging information, without really worrying about the
>     other debug format.
>   - Who really does non-symbolic debugging anyways?

The reason, IMO, it was posted is that people want GDB avoid reading
debug info and efficiently dump stack backtrace.  I think Red Hat people
must know why Fedora is shipping this patch.

I don't object to this approach.
Pedro Alves Oct. 4, 2016, 6:06 p.m. UTC | #2 
On 07/06/2016 09:54 PM, Joel Brobecker wrote:

>  #include <fcntl.h>
>  #include <sys/types.h>
> @@ -2062,6 +2063,9 @@ int
>  dwarf2_has_info (struct objfile *objfile,
>                   const struct dwarf2_debug_sections *names)
>  {
> +  if (readnever_symbol_files)
> +    return 0;

Guess that means '--readnever --readnow' is the same as
--readnever in practice?


> +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
> +    untested "Couldn't compile ${srcfile}"
> +    return -1
> +}

Maybe use build_executable.

> +set saved_gdbflags $GDBFLAGS
> +set GDBFLAGS "$GDBFLAGS --readnever"
> +clean_restart ${binfile}
> +set GDBFLAGS $saved_gdbflags

Nowadays we have save_vars:

save_vars { GDBFLAGS } {
  append GDBFLAGS " --readnever"
  clean_restart ${binfile}
}

Thanks,
Pedro Alves
Pedro Alves Oct. 4, 2016, 6:06 p.m. UTC | #3 
On 07/12/2016 03:27 PM, Yao Qi wrote:
> Hi Joel,
> 
> On Wed, Jul 6, 2016 at 9:54 PM, Joel Brobecker <brobecker@adacore.com> wrote:
>> Hello,
>>
>> One of our customers asked us about this option, which they could
>> see as being available in the version of GDB shipped by RedHat but
>> not in the version that AdaCore supports.
>>
>> The purpose of this option is to turn the load of debugging information
>> off. The implementation proposed here is mostly a copy of the patch
>> distributed with RedHat Fedora, and looking at the patch itself and
>> the history, I can see some reasons why it was never submitted:
> 
> Andrew Cagney posted the patch
> https://www.sourceware.org/ml/gdb/2004-11/msg00216.html
> 
>>   - The patch appears to have been introduced as a workaround, at
>>     least initially;
>>   - The patch is far from perfect, as it simply shunts the load of
>>     DWARF debugging information, without really worrying about the
>>     other debug format.
>>   - Who really does non-symbolic debugging anyways?
> 
> The reason, IMO, it was posted is that people want GDB avoid reading
> debug info and efficiently dump stack backtrace.  I think Red Hat people
> must know why Fedora is shipping this patch.
> 
> I don't object to this approach.
> 

This predates my gdb involvement, I don't really know the history.
Maybe Jan knows.

In any case, I don't object to the approach.

Is this skipping _unwind_ info as well though?  I think the
documentation should be clear on that, because if it does
skip dwarf info for unwinding as well, then you
may get a faster, but incorrect backtrace.

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a068622..f2910e1 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -1032,6 +1032,14 @@  Read each symbol file's entire symbol table immediately, rather than
 the default, which is to read it incrementally as it is needed.
 This makes startup slower, but makes future operations faster.
 
+@item --readnever
+@cindex @code{--readnever}
+Do not read each symbol file's symbolic debug information.  This makes
+startup faster but at the expense of not being able to perform
+symbolic debugging.
+
+This option is currently limited to debug information in DWARF format.
+For all other format, this option has no effect.
 @end table
 
 @node Mode Options
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 6658a38..b34b06f 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -70,6 +70,7 @@ 
 #include "filestuff.h"
 #include "build-id.h"
 #include "namespace.h"
+#include "top.h"
 
 #include <fcntl.h>
 #include <sys/types.h>
@@ -2062,6 +2063,9 @@  int
 dwarf2_has_info (struct objfile *objfile,
                  const struct dwarf2_debug_sections *names)
 {
+  if (readnever_symbol_files)
+    return 0;
+
   dwarf2_per_objfile = ((struct dwarf2_per_objfile *)
 			objfile_data (objfile, dwarf2_objfile_data_key));
   if (!dwarf2_per_objfile)
diff --git a/gdb/main.c b/gdb/main.c
index 5477379..3e8701b 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -601,6 +601,7 @@  captured_main (void *data)
       {"tui", no_argument, 0, OPT_TUI},
       {"dbx", no_argument, &dbx_commands, 1},
       {"readnow", no_argument, &readnow_symbol_files, 1},
+      {"readnever", no_argument, &readnever_symbol_files, 1},
       {"r", no_argument, &readnow_symbol_files, 1},
       {"quiet", no_argument, &quiet, 1},
       {"q", no_argument, &quiet, 1},
@@ -1199,6 +1200,9 @@  Selection of debuggee and its files:\n\n\
   --se=FILE          Use FILE as symbol file and executable file.\n\
   --symbols=SYMFILE  Read symbols from SYMFILE.\n\
   --readnow          Fully read symbol files on first access.\n\
+  --readnever        Do not read symbol files.\n\
+                     There is currently a known limitation that this only\n\
+                     has an effect on symbol files provided in DWARF format.\n\
   --write            Set writing into executable and core files.\n\n\
 "), stream);
   fputs_unfiltered (_("\
diff --git a/gdb/symfile.c b/gdb/symfile.c
index d29e96c..bbc2c24 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -80,6 +80,7 @@  static void clear_symtab_users_cleanup (void *ignore);
 
 /* Global variables owned by this file.  */
 int readnow_symbol_files;	/* Read full symbols immediately.  */
+int readnever_symbol_files;	/* Never read full symbols.  */
 
 /* Functions this file defines.  */
 
diff --git a/gdb/testsuite/gdb.base/readnever.c b/gdb/testsuite/gdb.base/readnever.c
new file mode 100644
index 0000000..803a554
--- /dev/null
+++ b/gdb/testsuite/gdb.base/readnever.c
@@ -0,0 +1,39 @@ 
+/* Copyright 2016 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+void
+fun_three (void)
+{
+  /* Do nothing.  */
+}
+
+void
+fun_two (void)
+{
+  fun_three ();
+}
+
+void
+fun_one (void)
+{
+  fun_two ();
+}
+
+int
+main (void)
+{
+  fun_one ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/readnever.exp b/gdb/testsuite/gdb.base/readnever.exp
new file mode 100644
index 0000000..5bf31a8
--- /dev/null
+++ b/gdb/testsuite/gdb.base/readnever.exp
@@ -0,0 +1,48 @@ 
+# Copyright 2016 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.  It is intended to test that
+# gdb can correctly print arrays with indexes for each element of the
+# array.
+
+standard_testfile .c
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+    untested "Couldn't compile ${srcfile}"
+    return -1
+}
+
+set saved_gdbflags $GDBFLAGS
+set GDBFLAGS "$GDBFLAGS --readnever"
+clean_restart ${binfile}
+set GDBFLAGS $saved_gdbflags
+
+
+if ![runto_main] then {
+    perror "couldn't run to breakpoint"
+    continue
+}
+
+gdb_test "break fun_three" \
+         "Breakpoint $decimal at $hex"
+
+gdb_test "continue" \
+         "Breakpoint $decimal, $hex in fun_three \\(\\)"
+
+gdb_test "backtrace" \
+         [multi_line "#0  $hex in fun_three \\(\\)" \
+                     "#1  $hex in fun_two \\(\\)" \
+                     "#2  $hex in fun_one \\(\\)" \
+                     "#3  $hex in main \\(\\)" ]
diff --git a/gdb/top.h b/gdb/top.h
index 64f7211..6f75eea 100644
--- a/gdb/top.h
+++ b/gdb/top.h
@@ -241,6 +241,7 @@  extern int gdb_in_secondary_prompt_p (struct ui *ui);
 
 /* From random places.  */
 extern int readnow_symbol_files;
+extern int readnever_symbol_files;
 
 /* Perform _initialize initialization.  */
 extern void gdb_init (char *);