Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [ppc64le] Use skip_entrypoint for skip_trampoline_code
@ 2015-09-07 20:47 Jan Kratochvil
  2015-09-07 22:01 ` [ppc64le patch v2] " Jan Kratochvil
  0 siblings, 1 reply; 8+ messages in thread
From: Jan Kratochvil @ 2015-09-07 20:47 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1678 bytes --]

Hi,

ppc64le loses control when stepping between two PLT-called functions inside
a shared library:

29        shlib_second (); /* first-hit */^M
(gdb) PASS: gdb.base/solib-intra-step.exp: first-hit
step^M
^M
Program received signal SIGABRT, Aborted.^M
0x00003fffb7cbe578 in __GI_raise (sig=<optimized out>) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56^M
56        return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);^M
(gdb) FAIL: gdb.base/solib-intra-step.exp: second-hit
->
29        shlib_second (); /* first-hit */^M
(gdb) PASS: gdb.base/solib-intra-step.exp: first-hit
step^M
shlib_second () at ./gdb.base/solib-intra-step-lib.c:23^M
23        abort (); /* second-hit */^M
(gdb) PASS: gdb.base/solib-intra-step.exp: second-hit

This is because gdbarch_skip_trampoline_code() will resolve the final function
as shlib_second+0 and place there the breakpoint, but ld.so will jump after
the breakpoint - at shlib_second+8 - as it is ELFv2 local symbol optimization:

Dump of assembler code for function shlib_second:
   0x0000000000000804 <+0>:	addis   r2,r12,2
   0x0000000000000808 <+4>:	addi    r2,r2,30668
   0x000000000000080c <+8>:	mflr    r0

Currently gdbarch_skip_entrypoint() has been called in skip_prologue_sal() and
fill_in_stop_func() but that is not enough.  I believe
gdbarch_skip_entrypoint() should be called after every
gdbarch_skip_trampoline_code(), shouldn't it?

The attached patch is a bit hack but I am not sure what is a clean solution.
Maybe create a gdbarch_skip_trampoline_code() wrapper function calling also
gdbarch_skip_entrypoint() and forbid calling gdbarch_skip_trampoline_code()
directly?

No regressions on ppc64le-rhel7-linux-gnu.


Jan

[-- Attachment #2: 2 --]
[-- Type: text/plain, Size: 5673 bytes --]

gdb/ChangeLog
2015-09-07  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* ppc64-tdep.c (ppc64_skip_trampoline_code): Rename to ...
	(ppc64_skip_trampoline_code_1): ... here.
	(ppc64_skip_trampoline_code): New wrapper function.

gdb/testsuite/ChangeLog
2015-09-07  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/solib-intra-step-lib.c: New file.
	* gdb.base/solib-intra-step-main.c: New file.
	* gdb.base/solib-intra-step.exp: New file.

diff --git a/gdb/ppc64-tdep.c b/gdb/ppc64-tdep.c
index bb23b6a..d3c0ece 100644
--- a/gdb/ppc64-tdep.c
+++ b/gdb/ppc64-tdep.c
@@ -454,8 +454,8 @@ ppc64_standard_linkage4_target (struct frame_info *frame,
    When the execution direction is EXEC_REVERSE, scan backward to
    check whether we are in the middle of a PLT stub.  */
 
-CORE_ADDR
-ppc64_skip_trampoline_code (struct frame_info *frame, CORE_ADDR pc)
+static CORE_ADDR
+ppc64_skip_trampoline_code_1 (struct frame_info *frame, CORE_ADDR pc)
 {
 #define MAX(a,b) ((a) > (b) ? (a) : (b))
   unsigned int insns[MAX (MAX (MAX (ARRAY_SIZE (ppc64_standard_linkage1),
@@ -530,6 +530,18 @@ ppc64_skip_trampoline_code (struct frame_info *frame, CORE_ADDR pc)
   return 0;
 }
 
+/* Wrapper of ppc64_skip_trampoline_code_1 checking also
+   ppc_elfv2_skip_entrypoint.  */
+
+CORE_ADDR
+ppc64_skip_trampoline_code (struct frame_info *frame, CORE_ADDR pc)
+{
+  pc = ppc64_skip_trampoline_code_1 (frame, pc);
+  if (pc != 0)
+    pc = gdbarch_skip_entrypoint (get_frame_arch (frame), pc);
+  return pc;
+}
+
 /* Support for convert_from_func_ptr_addr (ARCH, ADDR, TARG) on PPC64
    GNU/Linux.
 
diff --git a/gdb/testsuite/gdb.base/solib-intra-step-lib.c b/gdb/testsuite/gdb.base/solib-intra-step-lib.c
new file mode 100644
index 0000000..07b72cf
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-intra-step-lib.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/>.  */
+
+#include <stdlib.h>
+
+void
+shlib_second (void)
+{
+  abort (); /* second-hit */
+}
+
+void
+shlib_first (void)
+{
+  shlib_second (); /* first-hit */
+}
diff --git a/gdb/testsuite/gdb.base/solib-intra-step-main.c b/gdb/testsuite/gdb.base/solib-intra-step-main.c
new file mode 100644
index 0000000..186bd5f
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-intra-step-main.c
@@ -0,0 +1,25 @@
+/* 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/>.  */
+
+extern void shlib_first (void);
+
+int
+main (void)
+{
+  shlib_first ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/solib-intra-step.exp b/gdb/testsuite/gdb.base/solib-intra-step.exp
new file mode 100644
index 0000000..5666ede
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-intra-step.exp
@@ -0,0 +1,52 @@
+# 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/>.
+
+standard_testfile
+
+if {[skip_shlib_tests]} {
+    return 0
+}
+
+# Library file.
+set libname "${testfile}-lib"
+set srcfile_lib ${srcdir}/${subdir}/${libname}.c
+set binfile_lib [standard_output_file ${libname}.so]
+set lib_flags [list debug]
+# Binary file.
+set testfile "${testfile}-main"
+set srcfile ${srcdir}/${subdir}/${testfile}.c
+set binfile [standard_output_file ${testfile}]
+set bin_flags [list debug shlib=${binfile_lib}]
+
+if [get_compiler_info] {
+    return -1
+}
+
+if { [gdb_compile_shlib ${srcfile_lib} ${binfile_lib} $lib_flags] != ""
+     || [gdb_compile ${srcfile} ${binfile} executable $bin_flags] != "" } {
+  untested "Could not compile $binfile_lib or $binfile."
+  return -1
+}
+
+clean_restart ${binfile}
+gdb_load_shlibs $binfile_lib
+
+if ![runto_main] then {
+  return 0
+}
+
+gdb_test "step" " first-hit .*" "first-hit"
+
+gdb_test "step" " second-hit .*" "second-hit"

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [ppc64le patch v2] Use skip_entrypoint for skip_trampoline_code
  2015-09-07 20:47 [ppc64le] Use skip_entrypoint for skip_trampoline_code Jan Kratochvil
@ 2015-09-07 22:01 ` Jan Kratochvil
  2015-09-10 15:11   ` [ping for Cc] " Jan Kratochvil
  2015-09-10 21:17   ` [ppc64le patch v3] " Jan Kratochvil
  0 siblings, 2 replies; 8+ messages in thread
From: Jan Kratochvil @ 2015-09-07 22:01 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 53 bytes --]

An update to handle also breakpoints in -O2 -g code.

[-- Attachment #2: 3 --]
[-- Type: text/plain, Size: 7549 bytes --]

gdb/ChangeLog
2015-09-07  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* linespec.c (minsym_found): Call gdbarch_skip_entrypoint.
	* ppc64-tdep.c (ppc64_skip_trampoline_code): Rename to ...
	(ppc64_skip_trampoline_code_1): ... here.
	(ppc64_skip_trampoline_code): New wrapper function.
	* symtab.c (find_function_start_sal): Call gdbarch_skip_entrypoint.

gdb/testsuite/ChangeLog
2015-09-07  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/solib-intra-step-lib.c: New file.
	* gdb.base/solib-intra-step-main.c: New file.
	* gdb.base/solib-intra-step.exp: New file.

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 8f102fa..4c29c12 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -3570,6 +3570,8 @@ minsym_found (struct linespec_state *self, struct objfile *objfile,
 	  sal.pc = MSYMBOL_VALUE_ADDRESS (objfile, msymbol);
 	  sal.pc = gdbarch_convert_from_func_ptr_addr (gdbarch, sal.pc,
 						       &current_target);
+	  if (gdbarch_skip_entrypoint_p (gdbarch))
+	    sal.pc = gdbarch_skip_entrypoint (gdbarch, sal.pc);
 	}
       else
 	skip_prologue_sal (&sal);
diff --git a/gdb/ppc64-tdep.c b/gdb/ppc64-tdep.c
index bb23b6a..4a0b93a 100644
--- a/gdb/ppc64-tdep.c
+++ b/gdb/ppc64-tdep.c
@@ -454,8 +454,8 @@ ppc64_standard_linkage4_target (struct frame_info *frame,
    When the execution direction is EXEC_REVERSE, scan backward to
    check whether we are in the middle of a PLT stub.  */
 
-CORE_ADDR
-ppc64_skip_trampoline_code (struct frame_info *frame, CORE_ADDR pc)
+static CORE_ADDR
+ppc64_skip_trampoline_code_1 (struct frame_info *frame, CORE_ADDR pc)
 {
 #define MAX(a,b) ((a) > (b) ? (a) : (b))
   unsigned int insns[MAX (MAX (MAX (ARRAY_SIZE (ppc64_standard_linkage1),
@@ -530,6 +530,20 @@ ppc64_skip_trampoline_code (struct frame_info *frame, CORE_ADDR pc)
   return 0;
 }
 
+/* Wrapper of ppc64_skip_trampoline_code_1 checking also
+   ppc_elfv2_skip_entrypoint.  */
+
+CORE_ADDR
+ppc64_skip_trampoline_code (struct frame_info *frame, CORE_ADDR pc)
+{
+  struct gdbarch *gdbarch = get_frame_arch (frame);
+
+  pc = ppc64_skip_trampoline_code_1 (frame, pc);
+  if (pc != 0 && gdbarch_skip_entrypoint_p (gdbarch))
+    pc = gdbarch_skip_entrypoint (gdbarch, pc);
+  return pc;
+}
+
 /* Support for convert_from_func_ptr_addr (ARCH, ADDR, TARG) on PPC64
    GNU/Linux.
 
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 1ba691e..f65e809 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -3629,7 +3629,11 @@ find_function_start_sal (struct symbol *sym, int funfirstline)
       && (COMPUNIT_LOCATIONS_VALID (SYMTAB_COMPUNIT (sal.symtab))
 	  || SYMTAB_LANGUAGE (sal.symtab) == language_asm))
     {
+      struct gdbarch *gdbarch = symbol_arch (sym);
+
       sal.pc = BLOCK_START (SYMBOL_BLOCK_VALUE (sym));
+      if (gdbarch_skip_entrypoint_p (gdbarch))
+	sal.pc = gdbarch_skip_entrypoint (gdbarch, sal.pc);
       return sal;
     }
 
diff --git a/gdb/testsuite/gdb.base/solib-intra-step-lib.c b/gdb/testsuite/gdb.base/solib-intra-step-lib.c
new file mode 100644
index 0000000..9ab1211
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-intra-step-lib.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/>.  */
+
+#include <stdlib.h>
+
+void
+shlib_second (int dummy)
+{ /* second-retry */
+  abort (); /* second-hit */
+}
+
+void
+shlib_first (void)
+{ /* first-retry */
+  shlib_second (0); /* first-hit */
+}
diff --git a/gdb/testsuite/gdb.base/solib-intra-step-main.c b/gdb/testsuite/gdb.base/solib-intra-step-main.c
new file mode 100644
index 0000000..186bd5f
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-intra-step-main.c
@@ -0,0 +1,25 @@
+/* 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/>.  */
+
+extern void shlib_first (void);
+
+int
+main (void)
+{
+  shlib_first ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/solib-intra-step.exp b/gdb/testsuite/gdb.base/solib-intra-step.exp
new file mode 100644
index 0000000..4255c5d
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-intra-step.exp
@@ -0,0 +1,75 @@
+# 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/>.
+
+standard_testfile
+
+if {[skip_shlib_tests]} {
+    return 0
+}
+
+# Library file.
+set libname "${testfile}-lib"
+set srcfile_lib ${srcdir}/${subdir}/${libname}.c
+set binfile_lib [standard_output_file ${libname}.so]
+set lib_flags [list debug optimize=-O2]
+# Binary file.
+set testfile "${testfile}-main"
+set srcfile ${srcdir}/${subdir}/${testfile}.c
+set binfile [standard_output_file ${testfile}]
+set bin_flags [list debug shlib=${binfile_lib}]
+
+if [get_compiler_info] {
+    return -1
+}
+
+if { [gdb_compile_shlib ${srcfile_lib} ${binfile_lib} $lib_flags] != ""
+     || [gdb_compile ${srcfile} ${binfile} executable $bin_flags] != "" } {
+  untested "Could not compile $binfile_lib or $binfile."
+  return -1
+}
+
+clean_restart ${binfile}
+gdb_load_shlibs $binfile_lib
+
+if ![runto_main] then {
+  return 0
+}
+
+gdb_test "step" " first-hit .*" "first-hit"
+
+set test "second-hit"
+gdb_test_multiple "step" $test {
+    -re " second-hit .*\r\n$gdb_prompt $" {
+	pass $test
+    }
+    -re " first-retry .*\r\n$gdb_prompt $" {
+	set test "second-hit (optimized 1)"
+	gdb_test_multiple "step" $test {
+	    -re " second-hit .*\r\n$gdb_prompt $" {
+		pass $test
+	    }
+	    -re " first-hit .*\r\n$gdb_prompt $" {
+		gdb_test "step" " second-hit .*" "second-hit (optimized 2)"
+	    }
+	}
+    }
+}
+
+if ![runto_main] then {
+  return 0
+}
+
+gdb_breakpoint "shlib_second"
+gdb_continue_to_breakpoint "second-hit" ".* (second-hit|second-retry) .*"

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [ping for Cc] [ppc64le patch v2] Use skip_entrypoint for skip_trampoline_code
  2015-09-07 22:01 ` [ppc64le patch v2] " Jan Kratochvil
@ 2015-09-10 15:11   ` Jan Kratochvil
  2015-09-10 21:17   ` [ppc64le patch v3] " Jan Kratochvil
  1 sibling, 0 replies; 8+ messages in thread
From: Jan Kratochvil @ 2015-09-10 15:11 UTC (permalink / raw)
  To: gdb-patches; +Cc: Ulrich Weigand

Hello Ulrich,

just Ccing you as you implemented gdbarch_skip_entrypoint.

description:
	https://sourceware.org/ml/gdb-patches/2015-09/msg00080.html
	Message-ID: <20150907204720.GA535@host1.jankratochvil.net>
updated patch:
	https://sourceware.org/ml/gdb-patches/2015-09/msg00081.html
	Message-ID: <20150907220131.GA2811@host1.jankratochvil.net>


Thanks,
Jan


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [ppc64le patch v3] Use skip_entrypoint for skip_trampoline_code
  2015-09-07 22:01 ` [ppc64le patch v2] " Jan Kratochvil
  2015-09-10 15:11   ` [ping for Cc] " Jan Kratochvil
@ 2015-09-10 21:17   ` Jan Kratochvil
  2015-09-15 16:39     ` Ulrich Weigand
  1 sibling, 1 reply; 8+ messages in thread
From: Jan Kratochvil @ 2015-09-10 21:17 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 53 bytes --]

An update to PASS the testcase on non-ppc64le archs.

[-- Attachment #2: 4 --]
[-- Type: text/plain, Size: 7842 bytes --]

gdb/ChangeLog
2015-09-07  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* linespec.c (minsym_found): Call gdbarch_skip_entrypoint.
	* ppc64-tdep.c (ppc64_skip_trampoline_code): Rename to ...
	(ppc64_skip_trampoline_code_1): ... here.
	(ppc64_skip_trampoline_code): New wrapper function.
	* symtab.c (find_function_start_sal): Call gdbarch_skip_entrypoint.

gdb/testsuite/ChangeLog
2015-09-07  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.opt/solib-intra-step-lib.c: New file.
	* gdb.opt/solib-intra-step-main.c: New file.
	* gdb.opt/solib-intra-step.exp: New file.

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 8f102fa..4c29c12 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -3570,6 +3570,8 @@ minsym_found (struct linespec_state *self, struct objfile *objfile,
 	  sal.pc = MSYMBOL_VALUE_ADDRESS (objfile, msymbol);
 	  sal.pc = gdbarch_convert_from_func_ptr_addr (gdbarch, sal.pc,
 						       &current_target);
+	  if (gdbarch_skip_entrypoint_p (gdbarch))
+	    sal.pc = gdbarch_skip_entrypoint (gdbarch, sal.pc);
 	}
       else
 	skip_prologue_sal (&sal);
diff --git a/gdb/ppc64-tdep.c b/gdb/ppc64-tdep.c
index bb23b6a..4a0b93a 100644
--- a/gdb/ppc64-tdep.c
+++ b/gdb/ppc64-tdep.c
@@ -454,8 +454,8 @@ ppc64_standard_linkage4_target (struct frame_info *frame,
    When the execution direction is EXEC_REVERSE, scan backward to
    check whether we are in the middle of a PLT stub.  */
 
-CORE_ADDR
-ppc64_skip_trampoline_code (struct frame_info *frame, CORE_ADDR pc)
+static CORE_ADDR
+ppc64_skip_trampoline_code_1 (struct frame_info *frame, CORE_ADDR pc)
 {
 #define MAX(a,b) ((a) > (b) ? (a) : (b))
   unsigned int insns[MAX (MAX (MAX (ARRAY_SIZE (ppc64_standard_linkage1),
@@ -530,6 +530,20 @@ ppc64_skip_trampoline_code (struct frame_info *frame, CORE_ADDR pc)
   return 0;
 }
 
+/* Wrapper of ppc64_skip_trampoline_code_1 checking also
+   ppc_elfv2_skip_entrypoint.  */
+
+CORE_ADDR
+ppc64_skip_trampoline_code (struct frame_info *frame, CORE_ADDR pc)
+{
+  struct gdbarch *gdbarch = get_frame_arch (frame);
+
+  pc = ppc64_skip_trampoline_code_1 (frame, pc);
+  if (pc != 0 && gdbarch_skip_entrypoint_p (gdbarch))
+    pc = gdbarch_skip_entrypoint (gdbarch, pc);
+  return pc;
+}
+
 /* Support for convert_from_func_ptr_addr (ARCH, ADDR, TARG) on PPC64
    GNU/Linux.
 
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 1ba691e..f65e809 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -3629,7 +3629,11 @@ find_function_start_sal (struct symbol *sym, int funfirstline)
       && (COMPUNIT_LOCATIONS_VALID (SYMTAB_COMPUNIT (sal.symtab))
 	  || SYMTAB_LANGUAGE (sal.symtab) == language_asm))
     {
+      struct gdbarch *gdbarch = symbol_arch (sym);
+
       sal.pc = BLOCK_START (SYMBOL_BLOCK_VALUE (sym));
+      if (gdbarch_skip_entrypoint_p (gdbarch))
+	sal.pc = gdbarch_skip_entrypoint (gdbarch, sal.pc);
       return sal;
     }
 
diff --git a/gdb/testsuite/gdb.opt/solib-intra-step-lib.c b/gdb/testsuite/gdb.opt/solib-intra-step-lib.c
new file mode 100644
index 0000000..9ab1211
--- /dev/null
+++ b/gdb/testsuite/gdb.opt/solib-intra-step-lib.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/>.  */
+
+#include <stdlib.h>
+
+void
+shlib_second (int dummy)
+{ /* second-retry */
+  abort (); /* second-hit */
+}
+
+void
+shlib_first (void)
+{ /* first-retry */
+  shlib_second (0); /* first-hit */
+}
diff --git a/gdb/testsuite/gdb.opt/solib-intra-step-main.c b/gdb/testsuite/gdb.opt/solib-intra-step-main.c
new file mode 100644
index 0000000..186bd5f
--- /dev/null
+++ b/gdb/testsuite/gdb.opt/solib-intra-step-main.c
@@ -0,0 +1,25 @@
+/* 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/>.  */
+
+extern void shlib_first (void);
+
+int
+main (void)
+{
+  shlib_first ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.opt/solib-intra-step.exp b/gdb/testsuite/gdb.opt/solib-intra-step.exp
new file mode 100644
index 0000000..044c4bd
--- /dev/null
+++ b/gdb/testsuite/gdb.opt/solib-intra-step.exp
@@ -0,0 +1,86 @@
+# 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/>.
+
+standard_testfile
+
+if {[skip_shlib_tests]} {
+    return 0
+}
+
+# Library file.
+set libname "${testfile}-lib"
+set srcfile_lib ${srcdir}/${subdir}/${libname}.c
+set binfile_lib [standard_output_file ${libname}.so]
+set lib_flags [list debug optimize=-O2]
+# Binary file.
+set testfile "${testfile}-main"
+set srcfile ${srcdir}/${subdir}/${testfile}.c
+set binfile [standard_output_file ${testfile}]
+set bin_flags [list debug shlib=${binfile_lib}]
+
+if [get_compiler_info] {
+    return -1
+}
+
+if { [gdb_compile_shlib ${srcfile_lib} ${binfile_lib} $lib_flags] != ""
+     || [gdb_compile ${srcfile} ${binfile} executable $bin_flags] != "" } {
+  untested "Could not compile $binfile_lib or $binfile."
+  return -1
+}
+
+clean_restart ${binfile}
+gdb_load_shlibs $binfile_lib
+
+if ![runto_main] then {
+  return 0
+}
+
+set test "first-hit"
+gdb_test_multiple "step" $test {
+    -re " first-hit .*\r\n$gdb_prompt $" {
+	pass $test
+    }
+    -re " first-retry .*\r\n$gdb_prompt $" {
+	gdb_test "step" " first-hit .*" "first-hit (optimized)"
+    }
+}
+
+set test "second-hit"
+gdb_test_multiple "step" $test {
+    -re " second-hit .*\r\n$gdb_prompt $" {
+	pass $test
+    }
+    -re " first-retry .*\r\n$gdb_prompt $" {
+	set test "second-hit (optimized 1)"
+	gdb_test_multiple "step" $test {
+	    -re " second-hit .*\r\n$gdb_prompt $" {
+		pass $test
+	    }
+	    -re " first-hit .*\r\n$gdb_prompt $" {
+		gdb_test "step" " second-hit .*" "second-hit (optimized 2)"
+	    }
+	}
+    }
+    -re " second-retry .*\r\n$gdb_prompt $" {
+	gdb_test "step" " second-hit .*" "second-hit (optimized 3)"
+    }
+}
+
+if ![runto_main] then {
+  return 0
+}
+
+gdb_breakpoint "shlib_second"
+gdb_continue_to_breakpoint "second-hit" ".* (second-hit|second-retry) .*"

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [ppc64le patch v3] Use skip_entrypoint for skip_trampoline_code
  2015-09-10 21:17   ` [ppc64le patch v3] " Jan Kratochvil
@ 2015-09-15 16:39     ` Ulrich Weigand
  2015-09-15 16:50       ` Jan Kratochvil
  0 siblings, 1 reply; 8+ messages in thread
From: Ulrich Weigand @ 2015-09-15 16:39 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

Jan Kratochvil wrote:

> gdb/ChangeLog
> 2015-09-07  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* linespec.c (minsym_found): Call gdbarch_skip_entrypoint.
> 	* ppc64-tdep.c (ppc64_skip_trampoline_code): Rename to ...
> 	(ppc64_skip_trampoline_code_1): ... here.
> 	(ppc64_skip_trampoline_code): New wrapper function.
> 	* symtab.c (find_function_start_sal): Call gdbarch_skip_entrypoint.
> 
> gdb/testsuite/ChangeLog
> 2015-09-07  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdb.opt/solib-intra-step-lib.c: New file.
> 	* gdb.opt/solib-intra-step-main.c: New file.
> 	* gdb.opt/solib-intra-step.exp: New file.

This is OK.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [ppc64le patch v3] Use skip_entrypoint for skip_trampoline_code
  2015-09-15 16:39     ` Ulrich Weigand
@ 2015-09-15 16:50       ` Jan Kratochvil
  2015-09-15 16:55         ` Ulrich Weigand
  0 siblings, 1 reply; 8+ messages in thread
From: Jan Kratochvil @ 2015-09-15 16:50 UTC (permalink / raw)
  To: Ulrich Weigand; +Cc: gdb-patches

On Tue, 15 Sep 2015 18:39:17 +0200, Ulrich Weigand wrote:
> This is OK.

Thanks for the ppc64le arch review but I am not sure if it is also approval of
the coding style which I asked about (not specifically you) by:

On Mon, 07 Sep 2015 22:47:20 +0200, Jan Kratochvil wrote:
# Currently gdbarch_skip_entrypoint() has been called in skip_prologue_sal() and
# fill_in_stop_func() but that is not enough.  I believe
# gdbarch_skip_entrypoint() should be called after every
# gdbarch_skip_trampoline_code(), shouldn't it?
# 
# The attached patch is a bit hack but I am not sure what is a clean solution.
# Maybe create a gdbarch_skip_trampoline_code() wrapper function calling also
# gdbarch_skip_entrypoint() and forbid calling gdbarch_skip_trampoline_code()
# directly?

I did not want to refactor it before asking as there are too many
possibilities how to do it and I expect I would not guess the right one.

OTOH the refactorization can be also considered a future task, if you mean it
that way.


Thanks,
Jan


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [ppc64le patch v3] Use skip_entrypoint for skip_trampoline_code
  2015-09-15 16:50       ` Jan Kratochvil
@ 2015-09-15 16:55         ` Ulrich Weigand
  2015-09-15 17:10           ` [commit] " Jan Kratochvil
  0 siblings, 1 reply; 8+ messages in thread
From: Ulrich Weigand @ 2015-09-15 16:55 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

Jan Kratochvil wrote:
> On Tue, 15 Sep 2015 18:39:17 +0200, Ulrich Weigand wrote:
> > This is OK.
> 
> Thanks for the ppc64le arch review but I am not sure if it is also approval of
> the coding style which I asked about (not specifically you) by:
> 
> On Mon, 07 Sep 2015 22:47:20 +0200, Jan Kratochvil wrote:
> # Currently gdbarch_skip_entrypoint() has been called in skip_prologue_sal() and
> # fill_in_stop_func() but that is not enough.  I believe
> # gdbarch_skip_entrypoint() should be called after every
> # gdbarch_skip_trampoline_code(), shouldn't it?
> # 
> # The attached patch is a bit hack but I am not sure what is a clean solution.
> # Maybe create a gdbarch_skip_trampoline_code() wrapper function calling also
> # gdbarch_skip_entrypoint() and forbid calling gdbarch_skip_trampoline_code()
> # directly?
> 
> I did not want to refactor it before asking as there are too many
> possibilities how to do it and I expect I would not guess the right one.
> 
> OTOH the refactorization can be also considered a future task, if you mean it
> that way.

Actually, I thought it was fine to have this just be PowerPC-specific.

The need to skip entrypoints in skip_trampoline_code is only due to the
quite PowerPC-specific dynamic linker optimization to redirect the PLT
slot to the local entry point.  I'm not sure if any other target that may
want to use the skip_entrypoint logic in the future will have the same
requirement, so it seemed preferable to have that platform-specific.

In any case, it is probably better to wait with refactoring this into
common code until such time as there actually *is* a second platform
that uses skip_entrypoint, so we're clearer about the actual requirements.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [commit] [ppc64le patch v3] Use skip_entrypoint for skip_trampoline_code
  2015-09-15 16:55         ` Ulrich Weigand
@ 2015-09-15 17:10           ` Jan Kratochvil
  0 siblings, 0 replies; 8+ messages in thread
From: Jan Kratochvil @ 2015-09-15 17:10 UTC (permalink / raw)
  To: Ulrich Weigand; +Cc: gdb-patches

On Tue, 15 Sep 2015 18:55:14 +0200, Ulrich Weigand wrote:
> Actually, I thought it was fine to have this just be PowerPC-specific.
> 
> The need to skip entrypoints in skip_trampoline_code is only due to the
> quite PowerPC-specific dynamic linker optimization to redirect the PLT
> slot to the local entry point.  I'm not sure if any other target that may
> want to use the skip_entrypoint logic in the future will have the same
> requirement, so it seemed preferable to have that platform-specific.
> 
> In any case, it is probably better to wait with refactoring this into
> common code until such time as there actually *is* a second platform
> that uses skip_entrypoint, so we're clearer about the actual requirements.

OK.

Checked in:
	141c5cc4c44a6ce1a5c628c0f4849a8b1c91d383


Thanks,
Jan


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2015-09-15 17:10 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-07 20:47 [ppc64le] Use skip_entrypoint for skip_trampoline_code Jan Kratochvil
2015-09-07 22:01 ` [ppc64le patch v2] " Jan Kratochvil
2015-09-10 15:11   ` [ping for Cc] " Jan Kratochvil
2015-09-10 21:17   ` [ppc64le patch v3] " Jan Kratochvil
2015-09-15 16:39     ` Ulrich Weigand
2015-09-15 16:50       ` Jan Kratochvil
2015-09-15 16:55         ` Ulrich Weigand
2015-09-15 17:10           ` [commit] " Jan Kratochvil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox