[patch/rfc/hppa] handle setting gp for calling shlib functions

[patch/rfc/hppa] handle setting gp for calling shlib functions

[Randolph Chung]

We need to be careful about setting the gp when doing a function call in
several cases:

(1) when calling a function through a function descriptor
(2) when calling a shared library function (e.g. malloc)

This patch tries to fix these problems --

(1) is relatively easy
On the 32-bit ABI, function descriptors (called plabels) have a bit set
in their address. We implement the convert_from_func_ptr_addr method to
convert a plabel to a function address, and set the gp.

(2) is a bit more tricky. For hppa-linux we can use almost the same
method as ia64-linux -- by looking through the elf .dynamic section we
can determine the appropriate PLTGOT offset to use. Just have to be
careful that, if we get a pointer that is inside the plt we don't try to
do this, since the function pointer is not yet fixed up.  

For SOM and hppa64-hpux, I am not quite sure how to do this. pa64solib.c
has some code that reads the dp of a shlib into object private data.
Possibly we can use this.  For now both of these targets have a dummy
find_global_pointer method that returns 0, so they retain the previous
broken behavior.

On hppa-linux this fixes many failures related to calling inferior
shlib functions from gdb. It also fixes the C++ virtfunc tests.

Is this ok? Or can somebody suggest a better method?

thanks
randolph


2004-04-28  Randolph Chung  <tausq@debian.org>

	* hppa-tdep.h (gdbarch_tdep): Add find_global_pointer method.
	(FIND_GLOBAL_POINTER): Define.
	* hppa-tdep.c (hppa32_push_dummy_call): Try to find and set the gp
	if we are calling a function in a shared library.
	(hppa32_convert_from_func_ptr_addr): New.
	(hppa_find_global_pointer): New.
	(hppa_gdbarch_init): Set find_global_pointer and  
	convert_from_func_ptr_addr.
	* hppa-linux-tdep.c (hppa_linux_find_global_pointer): New function to
	find the gp in an elf object.
	(hppa_linux_init_abi): Set find_global_pointer

Index: hppa-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/hppa-tdep.h,v
retrieving revision 1.4
diff -u -p -r1.4 hppa-tdep.h
--- hppa-tdep.h	23 Apr 2004 02:54:21 -0000	1.4
+++ hppa-tdep.h	29 Apr 2004 05:50:00 -0000
@@ -33,8 +33,15 @@ struct gdbarch_tdep
   /* Is this an ELF target? This can be 64-bit HP-UX, or a 32/64-bit GNU/Linux
      system.  */
   int is_elf;
+
+  /* Given a function address, try to find the global pointer for the 
+     corresponding shared object.  */
+  CORE_ADDR (*find_global_pointer) (CORE_ADDR);
 };
 
+#define FIND_GLOBAL_POINTER \
+  (gdbarch_tdep (current_gdbarch)->find_global_pointer)
+
 /*
  * Unwind table and descriptor.
  */
Index: hppa-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-tdep.c,v
retrieving revision 1.153
diff -u -p -r1.153 hppa-tdep.c
--- hppa-tdep.c	29 Apr 2004 03:36:49 -0000	1.153
+++ hppa-tdep.c	29 Apr 2004 05:49:59 -0000
@@ -768,6 +768,10 @@ hppa32_push_dummy_call (struct gdbarch *
   /* Two passes.  First pass computes the location of everything,
      second pass writes the bytes out.  */
   int write_pass;
+
+  /* Global pointer (r19) of the function we are trying to call.  */
+  CORE_ADDR gp;
+
   for (write_pass = 0; write_pass < 2; write_pass++)
     {
       CORE_ADDR struct_ptr = 0;
@@ -888,6 +892,11 @@ hppa32_push_dummy_call (struct gdbarch *
   if (struct_return)
     write_register (28, struct_addr);
 
+  gp = FIND_GLOBAL_POINTER (func_addr);
+
+  if (gp != 0)
+    write_register (19, gp);
+
   /* Set the return address.  */
   regcache_cooked_write_unsigned (regcache, RP_REGNUM, bp_addr);
 
@@ -1021,6 +1030,25 @@ hppa64_push_dummy_call (struct gdbarch *
 }
 
 static CORE_ADDR
+hppa32_convert_from_func_ptr_addr (struct gdbarch *gdbarch,
+				   CORE_ADDR addr,
+				   struct target_ops *targ)
+{
+  if (addr & 2)
+    {
+      ULONGEST gp;
+
+      addr &= ~3;
+
+      gp = read_memory_unsigned_integer (addr + 4, 4);
+      write_register (19, gp);
+      addr = read_memory_unsigned_integer (addr, 4);
+    }
+
+  return addr;
+}
+
+static CORE_ADDR
 hppa32_frame_align (struct gdbarch *gdbarch, CORE_ADDR addr)
 {
   /* HP frames are 64-byte (or cache line) aligned (yes that's _byte_
@@ -2230,6 +2258,12 @@ hppa_pseudo_register_read (struct gdbarc
     store_unsigned_integer (buf, sizeof(tmp), tmp);
 }
 
+static CORE_ADDR
+hppa_find_global_pointer (CORE_ADDR faddr)
+{
+  return 0;
+}
+
 /* Here is a table of C type sizes on hppa with various compiles
    and options.  I measured this on PA 9000/800 with HP-UX 11.11
    and these compilers:
@@ -2297,6 +2331,8 @@ hppa_gdbarch_init (struct gdbarch_info i
   else
     tdep->bytes_per_address = 4;
 
+  tdep->find_global_pointer = hppa_find_global_pointer;
+
   /* Some parts of the gdbarch vector depend on whether we are running
      on a 32 bits or 64 bits target.  */
   switch (tdep->bytes_per_address)
@@ -2356,6 +2392,8 @@ hppa_gdbarch_init (struct gdbarch_info i
     case 4:
       set_gdbarch_push_dummy_call (gdbarch, hppa32_push_dummy_call);
       set_gdbarch_frame_align (gdbarch, hppa32_frame_align);
+      set_gdbarch_convert_from_func_ptr_addr
+        (gdbarch, hppa32_convert_from_func_ptr_addr);
       break;
     case 8:
       set_gdbarch_push_dummy_call (gdbarch, hppa64_push_dummy_call);
Index: hppa-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-linux-tdep.c,v
retrieving revision 1.1
diff -u -p -r1.1 hppa-linux-tdep.c
--- hppa-linux-tdep.c	29 Apr 2004 03:36:49 -0000	1.1
+++ hppa-linux-tdep.c	29 Apr 2004 05:49:59 -0000
@@ -30,6 +30,8 @@ Foundation, Inc., 59 Temple Place - Suit
 #include "dwarf2-frame.h"
 #include "hppa-tdep.h"
 
+#include "elf/common.h"
+
 #if 0
 /* Convert DWARF register number REG to the appropriate register
    number used by GDB.  */
@@ -456,6 +458,77 @@ hppa_linux_sigtramp_unwind_sniffer (stru
   return NULL;
 }
 
+/* Attempt to find (and return) the global pointer for the given
+   function.
+
+   This is a rather nasty bit of code searchs for the .dynamic section
+   in the objfile corresponding to the pc of the function we're trying
+   to call.  Once it finds the addresses at which the .dynamic section
+   lives in the child process, it scans the Elf32_Dyn entries for a
+   DT_PLTGOT tag.  If it finds one of these, the corresponding
+   d_un.d_ptr value is the global pointer.  */
+
+static CORE_ADDR
+hppa_linux_find_global_pointer (CORE_ADDR faddr)
+{
+  struct obj_section *faddr_sect;
+
+  /* If the address is in the plt section, then the real function hasn't 
+     yet been fixed up by the linker so we cannot determine the gp of 
+     that function.  */
+  if (in_plt_section (faddr, NULL))
+    return 0;
+
+  faddr_sect = find_pc_section (faddr);
+  if (faddr_sect != NULL)
+    {
+      struct obj_section *osect;
+
+      ALL_OBJFILE_OSECTIONS (faddr_sect->objfile, osect)
+	{
+	  if (strcmp (osect->the_bfd_section->name, ".dynamic") == 0)
+	    break;
+	}
+
+      if (osect < faddr_sect->objfile->sections_end)
+	{
+	  CORE_ADDR addr;
+
+	  addr = osect->addr;
+	  while (addr < osect->endaddr)
+	    {
+	      int status;
+	      LONGEST tag;
+	      char buf[4];
+
+	      status = target_read_memory (addr, buf, sizeof (buf));
+	      if (status != 0)
+		break;
+	      tag = extract_signed_integer (buf, sizeof (buf));
+
+	      if (tag == DT_PLTGOT)
+		{
+		  CORE_ADDR global_pointer;
+
+		  status = target_read_memory (addr + 4, buf, sizeof (buf));
+		  if (status != 0)
+		    break;
+		  global_pointer = extract_unsigned_integer (buf, sizeof (buf));
+
+		  /* The payoff... */
+		  return global_pointer;
+		}
+
+	      if (tag == DT_NULL)
+		break;
+
+	      addr += 8;
+	    }
+	}
+    }
+  return 0;
+}
+
 /* Forward declarations.  */
 extern initialize_file_ftype _initialize_hppa_linux_tdep;
 
@@ -466,6 +539,8 @@ hppa_linux_init_abi (struct gdbarch_info
 
   /* Linux is always ELF.  */
   tdep->is_elf = 1;
+
+  tdep->find_global_pointer = hppa_linux_find_global_pointer;
 
   set_gdbarch_write_pc (gdbarch, hppa_linux_target_write_pc);
 



-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/rfc/hppa] handle setting gp for calling shlib functions

[Andrew Cagney]

> +#define FIND_GLOBAL_POINTER \
> +  (gdbarch_tdep (current_gdbarch)->find_global_pointer)
> +
Just not this (macro's are bad m'kay :-).  The code should instead do 
something like:

	struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
	...
	... = tdep->find_global_pointer (...);

You'll find most architecture functions have that near the start.

The function also needs to add gdbarch|tdep|frame parameters and then 
use them instead of relying on globals such as current_gdbarch (I need 
to deprecate that)(1).

>  static CORE_ADDR
> +hppa32_convert_from_func_ptr_addr (struct gdbarch *gdbarch,
> +				   CORE_ADDR addr,
> +				   struct target_ops *targ)
> +{
> +  if (addr & 2)
> +    {
> +      ULONGEST gp;
> +
> +      addr &= ~3;
> +
> +      gp = read_memory_unsigned_integer (addr + 4, 4);
> +      write_register (19, gp);
> +      addr = read_memory_unsigned_integer (addr, 4);
> +    }
> +
> +  return addr;
> +}
> +
> +static CORE_ADDR

This should be using TARG methods read memory(2), and should not be 
writing GP to register 19.

Andrew

PS: The direction is strongly away from global state.  Already GDB has 
instances where two targets and/or architectures exist simultaneously:
(1) When handling 32 on 64-bit debugging (e.g., i386 on amd64) both 
architectures are active.
(2) Computing the entry point address, both the executable 
(un-relocated) and process targets are active.

Re: [patch/rfc/hppa] handle setting gp for calling shlib functions

[Randolph Chung]

> > static CORE_ADDR
> >+hppa32_convert_from_func_ptr_addr (struct gdbarch *gdbarch,
> >+				   CORE_ADDR addr,
> >+				   struct target_ops *targ)
> >+{
> >+  if (addr & 2)
> >+    {
> >+      ULONGEST gp;
> >+
> >+      addr &= ~3;
> >+
> >+      gp = read_memory_unsigned_integer (addr + 4, 4);
> >+      write_register (19, gp);
> >+      addr = read_memory_unsigned_integer (addr, 4);
> >+    }
> >+
> >+  return addr;
> >+}
> >+
> >+static CORE_ADDR
> 
> This should be using TARG methods read memory(2), and should not be 
> writing GP to register 19.

First point is understood, but about writing to register --

How else can this be handled? i.e. where else in the code path of
call_function_by_hand () will I be able to get ahold of the function
descriptor and set the gp? Unfortunately the func_addr that is passed
into push_dummy_call is already the canonicalized function address,
so i cannot do this as part of e.g. find_global_pointer.

thanks,
randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/rfc/hppa] handle setting gp for calling shlib functions

[Andrew Cagney]

>>>> > static CORE_ADDR
>>>> >+hppa32_convert_from_func_ptr_addr (struct gdbarch *gdbarch,
>>>> >+				   CORE_ADDR addr,
>>>> >+				   struct target_ops *targ)
>>>> >+{
>>>> >+  if (addr & 2)
>>>> >+    {
>>>> >+      ULONGEST gp;
>>>> >+
>>>> >+      addr &= ~3;
>>>> >+
>>>> >+      gp = read_memory_unsigned_integer (addr + 4, 4);
>>>> >+      write_register (19, gp);
>>>> >+      addr = read_memory_unsigned_integer (addr, 4);
>>>> >+    }
>>>> >+
>>>> >+  return addr;
>>>> >+}
>>>> >+
>>>> >+static CORE_ADDR
>>
>>> 
>>> This should be using TARG methods read memory(2), and should not be 
>>> writing GP to register 19.
> 
> 
> First point is understood, but about writing to register --
> 
> How else can this be handled? i.e. where else in the code path of
> call_function_by_hand () will I be able to get ahold of the function
> descriptor and set the gp?

Unfortunatly not here -> it's called with an exec-target for which 
writing register values is meaningless.  It's also ment to have no side 
effects :-/

 > Unfortunately the func_addr that is passed
> into push_dummy_call is already the canonicalized function address,
> so i cannot do this as part of e.g. find_global_pointer.

For PPC64 I used ABI knowledge to do a code address -> descriptor 
address reverse lookup.  Can you do something similar?

You've come across one of GDB's weak points - GDB currently isn't the 
best when it comes to juggling function descriptors and code addresses. 
  Function pointer parameters, for instance, have a similar problem - 
PPC64 push-dummy-call finds itself pushing code-address and not the 
descriptor address and hence messes them up :-(

Anyway, a thing-to-do-one day is to pass the function's ``value'' 
(descriptor and type) to push-dummy-call (there has been talk of doing 
this before).  Another is to clean up what's passed as the parameter 
list -> again descriptors.

Andrew

Re: [patch/rfc/hppa] handle setting gp for calling shlib functions

[Randolph Chung]

> > Unfortunately the func_addr that is passed
> >into push_dummy_call is already the canonicalized function address,
> >so i cannot do this as part of e.g. find_global_pointer.
> 
> For PPC64 I used ABI knowledge to do a code address -> descriptor 
> address reverse lookup.  Can you do something similar?

you mean look through a opd section? unfortunately there isn't such a
thing for the 32-bit abi. it's there in hppa64. 

for hppa32, the plabel likely resides in the got/plt, and i don't think
we can easily find it there.

> Anyway, a thing-to-do-one day is to pass the function's ``value'' 
> (descriptor and type) to push-dummy-call (there has been talk of doing 
> this before).  Another is to clean up what's passed as the parameter 
> list -> again descriptors.

well, i can already do this, by *not* implementing the conversion
function. However, when the actual call is made, we need to do the
conversion.

how about adding a new gdbarch method breakpoint_from_func_ptr 
and potentially not calling convert_from_func_ptr_address() from
call_function_by_hand()? other than ia64, ppc64 and hppa, how many
function-descriptor archs do we have?

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/rfc/hppa] handle setting gp for calling shlib functions

[Andrew Cagney]

>>>Unfortunately the func_addr that is passed
>>>> >into push_dummy_call is already the canonicalized function address,
>>>> >so i cannot do this as part of e.g. find_global_pointer.
>>
>>> 
>>> For PPC64 I used ABI knowledge to do a code address -> descriptor 
>>> address reverse lookup.  Can you do something similar?
> 
> 
> you mean look through a opd section? unfortunately there isn't such a
> thing for the 32-bit abi. it's there in hppa64. 

Anything.

For pa64, given "<symname>", the descriptor is at ".<symname>" so the 
operation is pretty easy.

> for hppa32, the plabel likely resides in the got/plt, and i don't think
> we can easily find it there.
> 
> 
>>> Anyway, a thing-to-do-one day is to pass the function's ``value'' 
>>> (descriptor and type) to push-dummy-call (there has been talk of doing 
>>> this before).  Another is to clean up what's passed as the parameter 
>>> list -> again descriptors.
> 
> 
> well, i can already do this, by *not* implementing the conversion
> function. However, when the actual call is made, we need to do the
> conversion.
> 
> how about adding a new gdbarch method breakpoint_from_func_ptr 
> and potentially not calling convert_from_func_ptr_address() from
> call_function_by_hand()? other than ia64, ppc64 and hppa, how many
> function-descriptor archs do we have?

(also xstormy16 at least)

The function/variable is (well was, it appears to have fallen off the 
radar for the moment) also wanted for a second reason.  When it comes to 
implementing ABI variants the full function type info is needed.  Hence, 
a ``struct value'' function descriptor (or descriptor pointer / type) 
are going to eventually be needed anyway.  Hence, I think an updated 
push-dummy-call with the new parameter would be better.

Andrew

Re: [patch/rfc/hppa] handle setting gp for calling shlib functions

[Randolph Chung]

> The function/variable is (well was, it appears to have fallen off the 
> radar for the moment) also wanted for a second reason.  When it comes to 
> implementing ABI variants the full function type info is needed.  Hence, 
> a ``struct value'' function descriptor (or descriptor pointer / type) 
> are going to eventually be needed anyway.  Hence, I think an updated 
> push-dummy-call with the new parameter would be better.

i can certainly try to look into this, but in lieu of the above (at
least in the meantime) would it be ok if i stash the func descriptor 
passed into convert_from_func_ptr_addr into a "cache" in the
tdep structure, and in push_dummy_call check the passed in funcaddr
against the cached value?

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/rfc/hppa] handle setting gp for calling shlib functions

[Andrew Cagney]

>>The function/variable is (well was, it appears to have fallen off the 
>>> radar for the moment) also wanted for a second reason.  When it comes to 
>>> implementing ABI variants the full function type info is needed.  Hence, 
>>> a ``struct value'' function descriptor (or descriptor pointer / type) 
>>> are going to eventually be needed anyway.  Hence, I think an updated 
>>> push-dummy-call with the new parameter would be better.
> 
> 
> i can certainly try to look into this, but in lieu of the above (at
> least in the meantime) would it be ok if i stash the func descriptor 
> passed into convert_from_func_ptr_addr into a "cache" in the
> tdep structure, and in push_dummy_call check the passed in funcaddr
> against the cached value?

Er, no, we're going to fix it.

Andrew

Re: [patch/rfc/hppa] handle setting gp for calling shlib functions

[Andrew Cagney]

>>> The function/variable is (well was, it appears to have fallen off the
>>>
>>>> radar for the moment) also wanted for a second reason.  When it comes to implementing ABI variants the full function type info is needed.  Hence, a ``struct value'' function descriptor (or descriptor pointer / type) are going to eventually be needed anyway.  Hence, I think an updated push-dummy-call with the new parameter would be better.
>>
>>
>>
>> i can certainly try to look into this, but in lieu of the above (at
>> least in the meantime) would it be ok if i stash the func descriptor passed into convert_from_func_ptr_addr into a "cache" in the
>> tdep structure, and in push_dummy_call check the passed in funcaddr
>> against the cached value?
> 
> 
> Er, no, we're going to fix it.

What happens if the function pointer value, rather than code start 
address is passed into push dummy call?  The entire value should be 
passed in, but this is a simple step in that direction.

Andrew

Re: [patch/rfc/hppa] handle setting gp for calling shlib functions

[Randolph Chung]

> What happens if the function pointer value, rather than code start 
> address is passed into push dummy call?  The entire value should be 
> passed in, but this is a simple step in that direction.

This will certainly help, as long as we also have a hook later in
call_function_by_hand () that can twiddle the funcaddr before setting
the breakpoint. maybe just moving the call that converts a funcptr to an
address to after the push dummy call?

btw, a related but slightly different question -- i don't see any code
in this path which will preserve and restore the call-clobbered 
registers when doing a call. how does gdb ensure that the register 
state before and after the call are the same? (i do see that there is
a testcase for this though.)

thanks,
randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/rfc/hppa] handle setting gp for calling shlib functions

[Andrew Cagney]

>>What happens if the function pointer value, rather than code start 
>>> address is passed into push dummy call?  The entire value should be 
>>> passed in, but this is a simple step in that direction.
> 
> 
> This will certainly help, as long as we also have a hook later in
> call_function_by_hand () that can twiddle the funcaddr before setting
> the breakpoint. maybe just moving the call that converts a funcptr to an
> address to after the push dummy call?

Yes, or find_function addr could return both.

> btw, a related but slightly different question -- i don't see any code
> in this path which will preserve and restore the call-clobbered 
> registers when doing a call. how does gdb ensure that the register 
> state before and after the call are the same? (i do see that there is
> a testcase for this though.)

See generic_push_dummy_frame (the name is historic) which saves all 
registers in save_reggroup (which by default is all registers).

callfuncs.exp checks that this works, namely tests like:

PASS: gdb.base/callfuncs.exp: gdb function calls preserve register contents
PASS: gdb.base/callfuncs.exp: continue after stop in call dummy 
preserves register contents
PASS: gdb.base/callfuncs.exp: finish after stop in call dummy preserves 
register contents
PASS: gdb.base/callfuncs.exp: return after stop in call dummy preserves 
register contents
PASS: gdb.base/callfuncs.exp: nested call dummies preserve register contents

Andrew