[PATCH v2 20/23] PPC64: symbol-file + exec-file results in broken displaced stepping

[Pedro Alves <palves at redhat dot com>]

Running gdb.base/dbx.exp with "set displaced-stepping on" on PPC64
fails like this:

 (gdb) run
 Starting program: build/gdb/testsuite/gdb.base/dbx

 Program received signal SIGSEGV, Segmentation fault.
 __GI__dl_debug_state () at dl-debug.c:75
 75      {
 (gdb) FAIL: gdb.base/dbx.exp: running to main

The problem is that GDB ended up with the wrong entry point address.
Displaced stepping puts the scratch pad at the entry point, thus the
crash.

Most other tests load the binary into GDB with "file", which is just a
wrapper for "exec-file + symbol-file".  However, this particular test
does the opposite: first "symbol-file", and then "exec-file".

symfile.c:init_entry_point_info makes certain that the entry point
address points at real code, and not a function descriptor, by calling
gdbarch_convert_from_func_ptr_addr.  However, ppc64's implementation
of gdbarch_convert_from_func_ptr_addr relies on target sections.  IOW,
on the exec_file being loaded already.  Thus if we use just
"symbol-file PROG", ppc64_convert_from_func_ptr_addr can't check of
ADDR points to a function descriptor, and
symfile.c:init_entry_point_info ends up caching the wrong entry point
address in the OBJFILE.  Even if the user uses "exec-file" afterwards,
GDB doesn't fix up the objfile's entry point address, as the address
had already been initialized:

 static void
 init_entry_point_info (struct objfile *objfile)
 {
   struct entry_info *ei = &objfile->per_bfd->ei;

   if (ei->initialized)
     return;

I think that it's expected that "symbol-file" alone doesn't work,
however, I think "symbol-file" followed by "exec-file" should behave
the same as "exec-file + symbol-file".

This patch fixes it by making GDB forget about the previously cached
entry point address when a new exec file is loaded.  I didn't make
this gdbarch-dependent in order to keep it simple.

New test that doesn't depend on --dbx included.  Fails without the GDB
fix.

gdb/ChangeLog:
2015-04-07  Pedro Alves  <palves@redhat.com>

	* symfile.c (invalidate_symfile_entry_point_info): New function.
	(_initialize_symfile): Attach it as executable_changed observer.

gdb/testsuite/ChangeLog:
2015-04-07  Pedro Alves  <palves@redhat.com>

	* gdb.base/symbol-file-exec-file.c: New file.
	* gdb.base/symbol-file-exec-file.exp: New file.
---
 gdb/symfile.c                                    |  21 ++++
 gdb/testsuite/gdb.base/symbol-file-exec-file.c   |  30 ++++++
 gdb/testsuite/gdb.base/symbol-file-exec-file.exp | 126 +++++++++++++++++++++++
 3 files changed, 177 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/symbol-file-exec-file.c
 create mode 100644 gdb/testsuite/gdb.base/symbol-file-exec-file.exp

diff --git a/gdb/symfile.c b/gdb/symfile.c
index 0c35ffa..81244f7 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -900,6 +900,22 @@ read_symbols (struct objfile *objfile, int add_flags)
     require_partial_symbols (objfile, 0);
 }
 
+/* Forget about any previous computation of the main symfile's entry
+   point.  */
+
+static void
+invalidate_symfile_entry_point_info (void)
+{
+  struct entry_info *ei;
+
+  if (symfile_objfile == NULL)
+    return;
+
+  ei = &symfile_objfile->per_bfd->ei;
+  ei->initialized = 0;
+  ei->entry_point_p = 0;
+}
+
 /* Initialize entry point information for this objfile.  */
 
 static void
@@ -3957,6 +3973,11 @@ _initialize_symfile (void)
 
   observer_attach_free_objfile (symfile_free_objfile);
 
+  /* Some target's like PPC64 (see ppc64_convert_from_func_ptr_addr),
+     rely on the exec_file's target sections to compute the symfile's
+     entry point.  */
+  observer_attach_executable_changed (invalidate_symfile_entry_point_info);
+
   c = add_cmd ("symbol-file", class_files, symbol_file_command, _("\
 Load symbol table from executable file FILE.\n\
 The `file' command can also load symbol tables, as well as setting the file\n\
diff --git a/gdb/testsuite/gdb.base/symbol-file-exec-file.c b/gdb/testsuite/gdb.base/symbol-file-exec-file.c
new file mode 100644
index 0000000..0b37f46
--- /dev/null
+++ b/gdb/testsuite/gdb.base/symbol-file-exec-file.c
@@ -0,0 +1,30 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015 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
+foo (int i)
+{
+}
+
+int
+main (void)
+{
+  foo (1);
+  foo (2);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/symbol-file-exec-file.exp b/gdb/testsuite/gdb.base/symbol-file-exec-file.exp
new file mode 100644
index 0000000..aebf794
--- /dev/null
+++ b/gdb/testsuite/gdb.base/symbol-file-exec-file.exp
@@ -0,0 +1,126 @@
+# Copyright 2015 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/>.
+
+# Test that displaced stepping works if the program is loaded with
+# "symbol-file + exec-file", in that order.  GDB used to end up with a
+# broken entry point address on PPC64 in that case.  (The displaced
+# stepping scratch pad is put at the entry point.)
+
+standard_testfile
+
+if {[build_executable "failed to build" $testfile $srcfile debug] == -1} {
+    return -1
+}
+
+# Override gdb_file_cmd, allowing gdb_load to do its normal thing.
+# This way remotes and simulators work, too.
+
+set old_gdb_file_cmd_args [info args gdb_file_cmd]
+set old_gdb_file_cmd_body [info body gdb_file_cmd]
+
+set our_gdb_file_cmd_called 0
+
+set symbol_file_first 0
+
+proc gdb_file_cmd {arg} {
+    upvar timeout timeout
+    global last_loaded_file
+    global our_gdb_file_cmd_called
+    global symbol_file_first
+
+    set our_gdb_file_cmd_called 1
+
+    set last_loaded_file $arg
+
+    if [is_remote host] {
+        set arg [remote_download host $arg]
+        if { $arg == "" } {
+            error "download failed"
+            return -1
+        }
+    }
+
+    proc symbol_file {} {
+	upvar arg arg
+
+	if {[gdb_test "symbol-file $arg" \
+		 "Reading symbols from.*done\\." \
+		 "symbol-file \$arg"] != 0 } {
+	    return -code return -1
+	}
+    }
+
+    proc exec_file {} {
+	upvar arg arg
+
+	if {[gdb_test_no_output "exec-file $arg" \
+		 "exec-file \$arg"] != 0} {
+	    return -code return -1
+	}
+    }
+
+    if $symbol_file_first {
+	symbol_file
+	exec_file
+    } else {
+	exec_file
+	symbol_file
+    }
+
+    return 0
+}
+
+# The test proper.  DISPLACED indicates whether to use displaced
+# stepping to step over the breakpoint.
+proc do_test { displaced } {
+    global our_gdb_file_cmd_called
+    global binfile
+
+    set our_gdb_file_cmd_called 0
+
+    clean_restart $binfile
+
+    gdb_test "set displaced-stepping $displaced"
+
+    # Don't rely on runto_main leaving the breakpoint behind.
+    gdb_breakpoint "main"
+
+    gdb_run_cmd
+
+    gdb_test "" "Breakpoint 1, main .* foo \\(1\\);" "run to main"
+
+    gdb_assert $our_gdb_file_cmd_called "overridden gdb_file_cmd called"
+
+    gdb_test "next" "foo \\(2\\);" "step over breakpoint"
+}
+
+foreach displaced { "off" "on" } {
+    foreach symfile_first { 0 1 } {
+	set symbol_file_first $symfile_first
+
+	if { $symfile_first } {
+	    set prefix "symbol-file + exec-file"
+	} else {
+	    set prefix "exec-file + symbol-file"
+	}
+
+	with_test_prefix "displaced=$displaced: $prefix" {
+	    do_test $displaced
+	}
+    }
+}
+
+# Restore gdb_file_cmd.
+eval proc gdb_file_cmd {$old_gdb_file_cmd_args} {$old_gdb_file_cmd_body}
-- 
1.9.3