[rfc] Simplify ppc64_sysv_abi_adjust_breakpoint_address

[rfc] Simplify ppc64_sysv_abi_adjust_breakpoint_address

[Ulrich Weigand]

Hello,

this is another patch to simplify powerpc64-linux code.  We used to 
handle minimal symbols pointing to function descriptors by installing
a ppc64_sysv_abi_adjust_breakpoint_address routine.  This always had
a number of unpleasant side effects, for example it broke prologue
skipping, and it tended to print out annoying warning messages.

With my recent changes to handle function descriptors directly in
linespec.c:minsym_found, the underlying problem is already handled
(and without those side effects) before we ever get into the last-
ditch adjust_breakpoint_address code, so this no longer has any
effect.

Thus I'd propose to remove that code.  Any objections?

Tested on powerpc64-linux with no change in behaviour.

Thanks,
Ulrich


ChangeLog:

	* ppc-linux-tdep.c (ppc_linux_init_abi): Do not install
	ppc64_sysv_abi_adjust_breakpoint_address.
	* ppc-sysv-tdep.c (ppc64_sysv_abi_adjust_breakpoint_address): Remove.
	* ppc-tdep.h (ppc64_sysv_abi_adjust_breakpoint_address): Remove.


diff -urNp gdb-orig/gdb/ppc-linux-tdep.c gdb-head/gdb/ppc-linux-tdep.c
--- gdb-orig/gdb/ppc-linux-tdep.c	2008-05-14 20:28:48.244451000 +0200
+++ gdb-head/gdb/ppc-linux-tdep.c	2008-05-14 21:13:05.634689598 +0200
@@ -1024,15 +1024,6 @@ ppc_linux_init_abi (struct gdbarch_info 
   
   if (tdep->wordsize == 8)
     {
-       /* Handle the 64-bit SVR4 minimal-symbol convention of using "FN"
-	  for the descriptor and ".FN" for the entry-point -- a user
-	  specifying "break FN" will unexpectedly end up with a breakpoint
-	  on the descriptor and not the function.  This architecture method
-	  transforms any breakpoints on descriptors into breakpoints on the
-	  corresponding entry point.  */
-      set_gdbarch_adjust_breakpoint_address
-	(gdbarch, ppc64_sysv_abi_adjust_breakpoint_address);
-
       /* Handle PPC GNU/Linux 64-bit function pointers (which are really
 	 function descriptors).  */
       set_gdbarch_convert_from_func_ptr_addr
diff -urNp gdb-orig/gdb/ppc-sysv-tdep.c gdb-head/gdb/ppc-sysv-tdep.c
--- gdb-orig/gdb/ppc-sysv-tdep.c	2008-05-11 23:02:59.000000000 +0200
+++ gdb-head/gdb/ppc-sysv-tdep.c	2008-05-14 21:13:18.323862623 +0200
@@ -1510,20 +1510,3 @@ ppc64_sysv_abi_return_value (struct gdba
   return RETURN_VALUE_STRUCT_CONVENTION;
 }
 
-CORE_ADDR
-ppc64_sysv_abi_adjust_breakpoint_address (struct gdbarch *gdbarch,
-					  CORE_ADDR bpaddr)
-{
-  /* PPC64 SYSV specifies that the minimal-symbol "FN" should point at
-     a function-descriptor while the corresponding minimal-symbol
-     ".FN" should point at the entry point.  Consequently, a command
-     like "break FN" applied to an object file with only minimal
-     symbols, will insert the breakpoint into the descriptor at "FN"
-     and not the function at ".FN".  Avoid this confusion by adjusting
-     any attempt to set a descriptor breakpoint into a corresponding
-     function breakpoint.  Note that GDB warns the user when this
-     adjustment is applied - that's ok as otherwise the user will have
-     no way of knowing why their breakpoint at "FN" resulted in the
-     program stopping at ".FN".  */
-  return gdbarch_convert_from_func_ptr_addr (gdbarch, bpaddr, &current_target);
-}
diff -urNp gdb-orig/gdb/ppc-tdep.h gdb-head/gdb/ppc-tdep.h
--- gdb-orig/gdb/ppc-tdep.h	2008-05-11 23:02:59.000000000 +0200
+++ gdb-head/gdb/ppc-tdep.h	2008-05-14 21:13:42.342003078 +0200
@@ -54,9 +54,6 @@ CORE_ADDR ppc64_sysv_abi_push_dummy_call
 					  struct value **args, CORE_ADDR sp,
 					  int struct_return,
 					  CORE_ADDR struct_addr);
-CORE_ADDR ppc64_sysv_abi_adjust_breakpoint_address (struct gdbarch *gdbarch,
-						    CORE_ADDR bpaddr);
-
 enum return_value_convention ppc64_sysv_abi_return_value (struct gdbarch *gdbarch,
 							  struct type *func_type,
 							  struct type *valtype,
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] Simplify ppc64_sysv_abi_adjust_breakpoint_address

[Daniel Jacobowitz]

On Thu, May 15, 2008 at 01:02:00AM +0200, Ulrich Weigand wrote:
> Thus I'd propose to remove that code.  Any objections?

Hooray!  I like this.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc] Simplify ppc64_sysv_abi_adjust_breakpoint_address

[Ulrich Weigand]

Daniel Jacobowitz wrote:
> On Thu, May 15, 2008 at 01:02:00AM +0200, Ulrich Weigand wrote:
> > Thus I'd propose to remove that code.  Any objections?
> 
> Hooray!  I like this.

Checked in as well.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] Simplify ppc64_sysv_abi_adjust_breakpoint_address

[Luis Machado]

Hi folks,

Resurrecting this one...

It seems we have a situation in which
"ppc64_sysv_abi_adjust_breakpoint_address" is still required, in a way.

Before removing this function, GDB was smart enough to know that the
entry point of a 64-bit PPC binary is, in reality, a function
descriptor, thus grabbing the correct breakpoint location from within
that address and setting it correctly.

After removing this function, GDB no longer knows that a specific
address is a function descriptor, and places a breakpoint at a data
section. The binary's code tries to fetch the correct address from the
function descriptor's address and ends up fetching the breakpoint
instruction, which makes no sense.

So, i see two ways:

1 - Make GDB smart again, being able to determine if the address is of a
function descriptor or not, basically the way i was before this patch.

2 - Assume the user knows what he's doing and that he knows where to
place a breakpoint when using the address of a function descriptor.

Regards,
Luis

On Fri, 2008-05-16 at 14:53 +0200, Ulrich Weigand wrote:
> Daniel Jacobowitz wrote:
> > On Thu, May 15, 2008 at 01:02:00AM +0200, Ulrich Weigand wrote:
> > > Thus I'd propose to remove that code.  Any objections?
> > 
> > Hooray!  I like this.
> 
> Checked in as well.
> 
> Thanks,
> Ulrich
> 

Re: [rfc] Simplify ppc64_sysv_abi_adjust_breakpoint_address

[Ulrich Weigand]

Luis Machado wrote:

> It seems we have a situation in which
> "ppc64_sysv_abi_adjust_breakpoint_address" is still required, in a way.
> 
> Before removing this function, GDB was smart enough to know that the
> entry point of a 64-bit PPC binary is, in reality, a function
> descriptor, thus grabbing the correct breakpoint location from within
> that address and setting it correctly.
> 
> After removing this function, GDB no longer knows that a specific
> address is a function descriptor, and places a breakpoint at a data
> section. The binary's code tries to fetch the correct address from the
> function descriptor's address and ends up fetching the breakpoint
> instruction, which makes no sense.
> 
> So, i see two ways:
> 
> 1 - Make GDB smart again, being able to determine if the address is of a
> function descriptor or not, basically the way i was before this patch.
> 
> 2 - Assume the user knows what he's doing and that he knows where to
> place a breakpoint when using the address of a function descriptor.

I'm not sure exactly what situation you're refering to ... is this
about
   break *0x.....
where the address that was manually specified refers to a function
descriptor instead of the function code?

In that case, I'd tend to consider this user error -- if you use
hard-coded addresses, you should know what you're doing on the 
platform.

If this is about something else, could you show a command line
that does the wrong thing for you?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] Simplify ppc64_sysv_abi_adjust_breakpoint_address

[Luis Machado]

On Thu, 2008-10-09 at 19:51 +0200, Ulrich Weigand wrote:
> Luis Machado wrote:
> 
> > It seems we have a situation in which
> > "ppc64_sysv_abi_adjust_breakpoint_address" is still required, in a way.
> > 
> > Before removing this function, GDB was smart enough to know that the
> > entry point of a 64-bit PPC binary is, in reality, a function
> > descriptor, thus grabbing the correct breakpoint location from within
> > that address and setting it correctly.
> > 
> > After removing this function, GDB no longer knows that a specific
> > address is a function descriptor, and places a breakpoint at a data
> > section. The binary's code tries to fetch the correct address from the
> > function descriptor's address and ends up fetching the breakpoint
> > instruction, which makes no sense.
> > 
> > So, i see two ways:
> > 
> > 1 - Make GDB smart again, being able to determine if the address is of a
> > function descriptor or not, basically the way i was before this patch.
> > 
> > 2 - Assume the user knows what he's doing and that he knows where to
> > place a breakpoint when using the address of a function descriptor.
> 
> I'm not sure exactly what situation you're refering to ... is this
> about
>    break *0x.....
> where the address that was manually specified refers to a function
> descriptor instead of the function code?
> 
> In that case, I'd tend to consider this user error -- if you use
> hard-coded addresses, you should know what you're doing on the 
> platform.
> 
> If this is about something else, could you show a command line
> that does the wrong thing for you?
> 
> Bye,
> Ulrich
> 

This is exactly about placing breakpoint on numberic addresses like
"break *0x...". GDB used to do that correctly before that patch, but it
doesn't anymore. So, maybe it's OK to consider this is something the
user should be aware of...

Luis

Re: [rfc] Simplify ppc64_sysv_abi_adjust_breakpoint_address

[Ulrich Weigand]

Luis Machado wrote:

> This is exactly about placing breakpoint on numberic addresses like
> "break *0x...". GDB used to do that correctly before that patch, but it
> doesn't anymore. So, maybe it's OK to consider this is something the
> user should be aware of...

In that case, I'm wondering where you get those addresses from --
why are those those pointing the descriptor instead of the code?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] Simplify ppc64_sysv_abi_adjust_breakpoint_address

[Luis Machado]

On Thu, 2008-10-09 at 19:59 +0200, Ulrich Weigand wrote:
> Luis Machado wrote:
> 
> > This is exactly about placing breakpoint on numberic addresses like
> > "break *0x...". GDB used to do that correctly before that patch, but it
> > doesn't anymore. So, maybe it's OK to consider this is something the
> > user should be aware of...
> 
> In that case, I'm wondering where you get those addresses from --
> why are those those pointing the descriptor instead of the code?
> 
> Bye,
> Ulrich

This is a very specific case where i fetch the so called entry-point
from the auxv table in a 64-bit ppc binary. It shows the function
descriptor's address.

9    AT_ENTRY             Entry point of program         0x100111d0

(gdb) x/g 0x100111d0
0x100111d0 <_start>:    0x000000001000053c

Luis

Re: [rfc] Simplify ppc64_sysv_abi_adjust_breakpoint_address

[Ulrich Weigand]

Luis Machado wrote:

> This is a very specific case where i fetch the so called entry-point
> from the auxv table in a 64-bit ppc binary. It shows the function
> descriptor's address.
> 
> 9    AT_ENTRY             Entry point of program         0x100111d0
> 
> (gdb) x/g 0x100111d0
> 0x100111d0 <_start>:    0x000000001000053c

I see.  Couldn't you just use
  b _start
and everything should work as expected?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] Simplify ppc64_sysv_abi_adjust_breakpoint_address

[Luis Machado]

On Thu, 2008-10-09 at 20:04 +0200, Ulrich Weigand wrote:
> Luis Machado wrote:
> 
> > This is a very specific case where i fetch the so called entry-point
> > from the auxv table in a 64-bit ppc binary. It shows the function
> > descriptor's address.
> > 
> > 9    AT_ENTRY             Entry point of program         0x100111d0
> > 
> > (gdb) x/g 0x100111d0
> > 0x100111d0 <_start>:    0x000000001000053c
> 
> I see.  Couldn't you just use
>   b _start
> and everything should work as expected?
> 
> Bye,
> Ulrich

That would work OK. It's just that this was something GDB did in the
past, and it's not doing anymore. So i'm just checking if we want to
keep it this way and consider the user knows what he's doing or not.

Luis