Re: [RFC][PATCH] fix gdb segv when objfile can't be opened

[Mike Gulick <mike dot gulick at mathworks dot com>]

Here is a new version of the patch.  It changes the following from the original
patch:

1) It adds a testsuite which locks down:
  a) The stack frame is printed when the breakpoint is hit
  b) 'next' runs without causing a segv
2) No longer set descriptor->size in gdb_bfd_map_section when an error is hit.
   It doesn't look like this is needed as gdb_bfd_map_section is the only place
   that consumes that descriptor->size field.
3) Update the description in gdb_bfd.h to indicate that if there is an error
   mapping the section, a warning is generated, *SIZE is set to 0, and NULL is
   returned.

I considered catching the error in the caller of gdb_bfd_map_section, which is
dwarf2_read_section in dwarf2read.c.  It didn't seem to make any more sense to
catch the error here than it would directly in gdb_bfd_map_section.  If
anything, it would make more sense to catch the error in the caller of
dwarf2_read_section, which in this case is dwarf2_get_section_info.  However
there are many callers of dwarf2_read_section.  It doesn't make sense to force
all of them to add a TRY/CATCH around calling dwarf2_read_section.

Please let me know thoughts/feedback.

Also see the note in the test case about not being able to make 'shlib=...'
work when compiling a shared library that links against another shared
library.  The workaround I used is kind of ugly.

Thanks,
Mike
---

>From d59bb69db0f10f8cb7d91568907b505f05424df7 Mon Sep 17 00:00:00 2001
From: Mike Gulick <mgulick@mathworks.com>
Date: Wed, 18 Oct 2017 16:04:27 -0400
Subject: [PATCH v2] fix gdb segv when objfile can't be opened

This fixes PR 16577.

This patch changes gdb_bfd_map_section to issue a warning rather than an
error if it is unable to read the object file, and sets the size of the
section/frame that it attempted to read to 0 on error.

The description of gdb_bfd_map_section states that it will try to read
or map the contents of the section SECT, and if successful, the section
data is returned and *SIZE is set to the size of the section data.  This
function was throwing an error and leaving *SIZE as-is.  Setting the
section size to 0 indicates to dwarf2_build_frame_info that there is no
data to read, otherwise it will try to read from an invalid frame
pointer.

Changing the error to a warning allows this to be handled gracefully.
Additionally, the error was clobbering the breakpoint output indicating
the current frame (function name, arguments, source file, and line number).
E.g.

Thread 3 "foo" hit Breakpoint 1, BFD: reopening /tmp/jna-1013829440/jna2973250704389291330.tmp: No such file or directory

BFD: reopening /tmp/jna-1013829440/jna2973250704389291330.tmp: No such file or directory

(gdb)

While the "BFD: reopening ..." messages will still appear interspersed in the
breakpoint output, the current frame info is now displayed:

Thread 3 "foo" hit Breakpoint 1, BFD: reopening /tmp/jna-1013829440/jna1875755897659885075.tmp: No such file or directory

BFD: reopening /tmp/jna-1013829440/jna1875755897659885075.tmp: No such file or directory

warning: Can't read data for section '.eh_frame' in file '/tmp/jna-1013829440/jna1875755897659885075.tmp'
do_something () at file.cpp:80
80	{
(gdb)
---
 gdb/gdb_bfd.c                              |  11 ++-
 gdb/gdb_bfd.h                              |  17 +++--
 gdb/testsuite/gdb.base/solib-vanish-lib1.c |  22 ++++++
 gdb/testsuite/gdb.base/solib-vanish-lib2.c |  20 ++++++
 gdb/testsuite/gdb.base/solib-vanish-main.c |  62 +++++++++++++++++
 gdb/testsuite/gdb.base/solib-vanish.exp    | 108 +++++++++++++++++++++++++++++
 6 files changed, 228 insertions(+), 12 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/solib-vanish-lib1.c
 create mode 100644 gdb/testsuite/gdb.base/solib-vanish-lib2.c
 create mode 100644 gdb/testsuite/gdb.base/solib-vanish-main.c
 create mode 100644 gdb/testsuite/gdb.base/solib-vanish.exp

diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index cc02740..b33de07 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -702,9 +702,14 @@ gdb_bfd_map_section (asection *sectp, bfd_size_type *size)
 
   data = NULL;
   if (!bfd_get_full_section_contents (abfd, sectp, &data))
-    error (_("Can't read data for section '%s' in file '%s'"),
-	   bfd_get_section_name (abfd, sectp),
-	   bfd_get_filename (abfd));
+    {
+      warning (_("Can't read data for section '%s' in file '%s'"),
+	       bfd_get_section_name (abfd, sectp),
+	       bfd_get_filename (abfd));
+      /* Section is invalid -- set size to 0 and return NULL */
+      *size = 0;
+      return (const gdb_byte *) NULL;
+    }
   descriptor->data = data;
 
  done:
diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index b1ff857..e427781 100644
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -115,15 +115,14 @@ void gdb_bfd_mark_parent (bfd *child, bfd *parent);
 
 void gdb_bfd_record_inclusion (bfd *includer, bfd *includee);
 
-/* Try to read or map the contents of the section SECT.  If
-   successful, the section data is returned and *SIZE is set to the
-   size of the section data; this may not be the same as the size
-   according to bfd_get_section_size if the section was compressed.
-   The returned section data is associated with the BFD and will be
-   destroyed when the BFD is destroyed.  There is no other way to free
-   it; for temporary uses of section data, see
-   bfd_malloc_and_get_section.  SECT may not have relocations.  This
-   function will throw on error.  */
+/* Try to read or map the contents of the section SECT.  If successful, the
+   section data is returned and *SIZE is set to the size of the section data;
+   this may not be the same as the size according to bfd_get_section_size if the
+   section was compressed.  The returned section data is associated with the BFD
+   and will be destroyed when the BFD is destroyed.  There is no other way to
+   free it; for temporary uses of section data, see bfd_malloc_and_get_section.
+   SECT may not have relocations.  If there is an error reading the section,
+   this issues a warning, sets *SIZE to 0, and returns NULL.  */
 
 const gdb_byte *gdb_bfd_map_section (asection *section, bfd_size_type *size);
 
diff --git a/gdb/testsuite/gdb.base/solib-vanish-lib1.c b/gdb/testsuite/gdb.base/solib-vanish-lib1.c
new file mode 100644
index 0000000..f8e8182
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-vanish-lib1.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2017 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/>.  */
+
+extern int bar (int);
+
+int foo (int x) {
+  return bar (x);
+}
diff --git a/gdb/testsuite/gdb.base/solib-vanish-lib2.c b/gdb/testsuite/gdb.base/solib-vanish-lib2.c
new file mode 100644
index 0000000..7799d92
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-vanish-lib2.c
@@ -0,0 +1,20 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2017 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/>.  */
+
+int bar (int y) {
+  return y + 1; /* break here */
+}
diff --git a/gdb/testsuite/gdb.base/solib-vanish-main.c b/gdb/testsuite/gdb.base/solib-vanish-main.c
new file mode 100644
index 0000000..0e21e9c
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-vanish-main.c
@@ -0,0 +1,62 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2017 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/>.  */
+
+#include <stdio.h>
+#include <dlfcn.h>
+#include <stdlib.h>
+
+#ifndef VANISH_LIB
+#define VANISH_LIB "./solib-vanish-lib1.so"
+#endif
+
+int main()
+{
+  void *handle;
+  int (*foo)(int);
+  char *error;
+
+  /* Open library */
+  handle = dlopen(VANISH_LIB, RTLD_NOW);
+  if (!handle) {
+    fprintf(stderr, "%s\n", dlerror());
+    exit(EXIT_FAILURE);
+  }
+
+  /* Clear any existing error */
+  dlerror();
+
+  /* Remove library */
+  if (remove(VANISH_LIB) == -1) {
+    perror("remove " VANISH_LIB);
+    exit(EXIT_FAILURE);
+  }
+
+  /* Get function pointer */
+  foo = dlsym(handle, "foo");
+  error = dlerror();
+  if (error != NULL) {
+    fprintf(stderr, "%s\n", error);
+    exit(EXIT_FAILURE);
+  }
+
+  /* Call function */
+  (*foo)(1);
+
+  /* Close and exit */
+  dlclose(handle);
+  exit(EXIT_SUCCESS);
+}
diff --git a/gdb/testsuite/gdb.base/solib-vanish.exp b/gdb/testsuite/gdb.base/solib-vanish.exp
new file mode 100644
index 0000000..de92d8d
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-vanish.exp
@@ -0,0 +1,108 @@
+# Copyright 2017 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 test case verifies that GDB gracefully handles a shared library file
+# vanishing after being dlopen'ed.  This consists of three nested function calls:
+#
+# main() -> foo() -> bar()
+#
+# where:
+# - foo exists in solib-vanish-lib1.so, which is dlopen'ed by main()
+# - bar exists in solib-vanish-lib2.so, which is dynamically linked into
+#   solib-vanish-lib1.so
+#
+# Immediately after dlopen'ing solib-vanish-lib1.so, the so file is deleted.
+# The main executable and solib-vanish-lib2.so are still accessible.
+#
+# If a breakpoint is set on bar(), gdb throws an error when this breakpoint is
+# hit:
+#
+#   (gdb) r
+#   Starting program: /local/gdb/git/pr_16577_repro/simple/solib-vanish-main
+#
+#   Breakpoint 1, BFD: reopening ./solib-vanish-lib1.so: No such file or directory
+#
+#   BFD: reopening ./solib-vanish-lib1.so: No such file or directory
+#
+#   (gdb)
+#
+# Notice that this does not print the current frame, i.e.:
+#   bar (y=1) at solib-vanish-lib2.c:19
+#   19	  return y + 1; /* break here */
+#   (gdb)
+#
+# The current gdb git tip segfaults if we then try to step:
+#   (gdb) n
+#   Segmentation fault
+
+# This test verifies that:
+# 1) GDB does not segfault when stepping
+# 2) The stack frame is printed
+
+if { [skip_shlib_tests] } {
+    return 0
+}
+
+# Library 2
+set lib2name "solib-vanish-lib2"
+set srcfile_lib2 ${srcdir}/${subdir}/${lib2name}.c
+set binfile_lib2 [standard_output_file ${lib2name}.so]
+set lib2_flags {debug}
+
+# Library 1
+set lib1name "solib-vanish-lib1"
+set srcfile_lib1 ${srcdir}/${subdir}/${lib1name}.c
+set binfile_lib1 [standard_output_file ${lib1name}.so]
+# I can't make this work for some reason:
+#set lib1_flags [list debug shlib=${binfile_lib2}]
+set lib1_flags [list debug ldflags=-l:${lib2name}.so ldflags=-Wl,-rpath=[standard_output_file ""] libdir=[standard_output_file ""]]
+
+# Main program
+set testfile "solib-vanish-main"
+set srcfile ${srcdir}/${subdir}/${testfile}.c
+set executable ${testfile}
+set binfile [standard_output_file ${executable}]
+set bin_flags [list debug shlib_load additional_flags=-DVANISH_LIB=\"${binfile_lib1}\"]
+
+if { [gdb_compile_shlib ${srcfile_lib2} ${binfile_lib2} $lib2_flags] != ""
+     || [gdb_compile_shlib ${srcfile_lib1} ${binfile_lib1} $lib1_flags] != ""
+     || [gdb_compile ${srcfile} ${binfile} executable $bin_flags] != "" } {
+    untested "failed to compile"
+    return -1
+}
+
+clean_restart $testfile
+
+if { ![runto_main] } {
+    fail "can't run to main"
+    return
+}
+
+delete_breakpoints
+
+set lib2_lineno [gdb_get_line_number "break here" ${srcfile_lib2}]
+
+gdb_breakpoint "${lib2name}.c:${lib2_lineno}" {allow-pending}
+
+gdb_test "continue" \
+    "Breakpoint .*at .*/${lib2name}.c:${lib2_lineno}.*break here.*" \
+    "breakpoint prints location"
+
+# This should not segfault
+gdb_test "next" \
+    "" \
+    "next succeeds"
+
-- 
2.1.4