Re: [rfa:ppc64] Fix 64-bit PPC ELF function calls

[Andrew Cagney <ac131313@redhat.com>]

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

> On Oct 6,  6:19pm, Andrew Cagney wrote:
> 
> 
>> > I notice some 80+ character lines in ppc64_sysv_abi_push_dummy_call().
>> > Could you adjust these so that they're 80 characters or less?
> 
>> 
>> I'll run the file through gdb_indent.sh, as a separate commit.
> 
> 
> I see that you've already done it.

Yes, obvious.

> Don't forget to do it again
> after you commit your ppc64_sysv_abi_push_dummy_call() changes.

The attached has already been re-indented.  Otherwize the diff ends up 
containing unnecessary white space changes.

> Here's another typo:
[...]
> s/ment/meant/
[...]
> s/Fortunatly/Fortunately/

Fixed, along with a few others.

> Not yet.
> 
> Please add
> 
>   gdb_assert (tdep->wordsize == 8)
> 
> somewhere early in ppc64_sysv_abi_push_dummy_call().

Added, along with your comments.

> Anyway, in this instance, I think it's advisable to avoid the names
> "top" and "bottom" altogether.  Perhaps ``param_end'' is a more
> appropriate name?

I replaced that variable with vparam_size and gparam_size.

> +  /* Address, on the stack, of the next general (integer, struct,
> +     float, ...) parameter.  */
> +  CORE_ADDR gparam = 0;
> +  /* Address, on the stack, of the next vector.  */
> +  CORE_ADDR vparam = 0;
> 
> These are addresses when write_pass == 1, but merely a running
> total of the number of words needed when write_pass == 0.  Calling
> them addresses is misleading.
> 
> +  /* Go through the argument list twice.
> +
> +     Pass 1: Figure out how much stack space is required for arguments
> +     and pushed values.
> +
> +     Pass 2: Replay the same computation but this time also write the
> +     values out to the target.  */
> 
> Perhaps you could explain the dual roles of gparam and vparam in the
> above comment?

Yes but slightly further down where they are computed.

> +      if (!write_pass)
> +	{
> +	  vparam = align_down (param_top - vparam, 16);
> +	  gparam = align_down (vparam - gparam, 16);
> +	  sp = align_down (gparam - 48, 16);
> +	}
> 
> The ABI says:
> 
>     The parameter save area, which is located at a fixed offset of 48
>     bytes from the stack pointer, is reserved in each stack frame for use
>     as an argument list.  A minimum of 8 doublewords is always reserved.
> 
> I don't see where you've accounted for this 8 doubleword minimum. 
> Perhaps revise this to something along the following lines?

Good point.

>        if (!write_pass)
>  	{
>  	  vparam = align_down (param_top - vparam, 16);
> 	  if (gparam < 8 * tdep->wordsize)
> 	    gparam = 8 * tdep->wordsize;
>  	  gparam = align_down (vparam - gparam, 16);
>  	  sp = align_down (gparam - 48, 16);
>  	}
> 
> I'm not sure this is right either since that alters the vector save
> area.  I don't happen to have a copy of the Altivec ABI though.

Done (but also rearanged).  The vector save area can be zero in size.

> +		  if (greg <= 10)
> +		    {
> +		      /* It goes into the register, left aligned (as
> +                         far as I can tell).  */
> 
> I can't find anything in the ABI to contradict this, but I found it
> surprising that floating point arguments would be left aligned.  If
> I'm not mistaken (for big endian), this'll mean that the the float
> is stored in the most significant half of the register.

It now reads:

		      /* The ABI states "Single precision floating
		         point values are mapped to the first word in
		         a single doubleword" and "... floating point
		         values mapped to the first eight doublewords
		         of the parameter save area are also passed in
		         general registers").

			 This code interprets that to mean: store it,
			 left aligned, in the general register.  */


Andrew

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

2003-10-09  Andrew Cagney  <cagney@redhat.com>

	* Makefile.in (ppc-sysv-tdep.o): Add $(gdb_assert_h).
	* rs6000-tdep.c (rs6000_gdbarch_init): When 64 bit SysV ABI, set
	push_dummy_call to ppc64_sysv_abi_push_dummy_call.
	* ppc-sysv-tdep.c: Include "gdb_assert.h".
	(ppc64_sysv_abi_push_dummy_call): New function.
	(ppc64_sysv_abi_broken_push_dummy_call): New function.
	* ppc-tdep.h (ppc64_sysv_abi_push_dummy_call): Declare.
	(ppc64_sysv_abi_broken_push_dummy_call): Declare.

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.452
diff -u -r1.452 Makefile.in
--- Makefile.in	6 Oct 2003 21:56:40 -0000	1.452
+++ Makefile.in	10 Oct 2003 01:25:03 -0000
@@ -2114,7 +2114,7 @@
 	$(target_h) $(breakpoint_h) $(value_h) $(osabi_h) $(ppc_tdep_h) \
 	$(ppcnbsd_tdep_h) $(nbsd_tdep_h) $(solib_svr4_h)
 ppc-sysv-tdep.o: ppc-sysv-tdep.c $(defs_h) $(gdbcore_h) $(inferior_h) \
-	$(regcache_h) $(value_h) $(gdb_string_h) $(ppc_tdep_h)
+	$(regcache_h) $(value_h) $(gdb_string_h) $(gdb_assert_h) $(ppc_tdep_h)
 printcmd.o: printcmd.c $(defs_h) $(gdb_string_h) $(frame_h) $(symtab_h) \
 	$(gdbtypes_h) $(value_h) $(language_h) $(expression_h) $(gdbcore_h) \
 	$(gdbcmd_h) $(target_h) $(breakpoint_h) $(demangle_h) $(valprint_h) \
Index: ppc-sysv-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/ppc-sysv-tdep.c,v
retrieving revision 1.14
diff -u -r1.14 ppc-sysv-tdep.c
--- ppc-sysv-tdep.c	6 Oct 2003 22:23:47 -0000	1.14
+++ ppc-sysv-tdep.c	10 Oct 2003 01:25:03 -0000
@@ -26,7 +26,7 @@
 #include "regcache.h"
 #include "value.h"
 #include "gdb_string.h"
-
+#include "gdb_assert.h"
 #include "ppc-tdep.h"
 
 /* Pass the arguments in either registers, or in the stack. Using the
@@ -315,6 +315,295 @@
     return 0;
 
   return (TYPE_LENGTH (value_type) > 8);
+}
+
+/* Pass the arguments in either registers, or in the stack. Using the
+   ppc 64 bit SysV ABI.
+
+   This implements a dumbed down version of the ABI.  It always writes
+   values to memory, GPR and FPR, even when not necessary.  Doing this
+   greatly simplifies the logic. */
+
+CORE_ADDR
+ppc64_sysv_abi_push_dummy_call (struct gdbarch *gdbarch, CORE_ADDR func_addr,
+				struct regcache *regcache, CORE_ADDR bp_addr,
+				int nargs, struct value **args, CORE_ADDR sp,
+				int struct_return, CORE_ADDR struct_addr)
+{
+  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
+  /* By this stage in the proceedings, SP has been decremented by "red
+     zone size" + "struct return size".  Fetch the stack-pointer from
+     before this and use that as the BACK_CHAIN.  */
+  const CORE_ADDR back_chain = read_sp ();
+  /* See for-loop comment below.  */
+  int write_pass;
+  /* Size of the Altivec's vector parameter region, the final value is
+     computed in the for-loop below.  */
+  LONGEST vparam_size = 0;
+  /* Size of the general parameter region, the final value is computed
+     in the for-loop below.  */
+  LONGEST gparam_size = 0;
+  /* Kevin writes ... I don't mind seeing tdep->wordsize used in the
+     calls to align_up(), align_down(), etc.  because this makes it
+     easier to reuse this code (in a copy/paste sense) in the future,
+     but it is a 64-bit ABI and asserting that the wordsize is 8 bytes
+     at some point makes it easier to verify that this function is
+     correct without having to do a non-local analysis to figure out
+     the possible values of tdep->wordsize.  */
+  gdb_assert (tdep->wordsize == 8);
+
+  /* Go through the argument list twice.
+
+     Pass 1: Compute the function call's stack space and register
+     requirements.
+
+     Pass 2: Replay the same computation but this time also write the
+     values out to the target.  */
+
+  for (write_pass = 0; write_pass < 2; write_pass++)
+    {
+      int argno;
+      /* Next available floating point register for float and double
+         arguments.  */
+      int freg = 1;
+      /* Next available general register for non-vector (but possibly
+         float) arguments.  */
+      int greg = 3;
+      /* Next available vector register for vector arguments.  */
+      int vreg = 2;
+      /* The address, at which the next general purpose parameter
+         (integer, struct, float, ...) should be saved.  */
+      CORE_ADDR gparam;
+      /* Address, at which the next Altivec vector parameter should be
+         saved.  */
+      CORE_ADDR vparam;
+
+      if (!write_pass)
+	{
+	  /* During the first pass, GPARAM and VPARAM are more like
+	     offsets (start address zero) than addresses.  That way
+	     the accumulate the total stack space each region
+	     requires.  */
+	  gparam = 0;
+	  vparam = 0;
+	}
+      else
+	{
+	  /* Decrement the stack pointer making space for the Altivec
+	     and general on-stack parameters.  Set vparam and gparam
+	     to their corresponding regions.  */
+	  vparam = align_down (sp - vparam_size, 16);
+	  gparam = align_down (vparam - gparam_size, 16);
+	  /* Add in space for the TOC, link editor double word,
+	     compiler double word, LR save area, CR save area.  */
+	  sp = align_down (gparam - 48, 16);
+	}
+
+      /* If the function is returning a `struct', then there is an
+         extra hidden parameter (which will be passed in r3)
+         containing the address of that struct..  In that case we
+         should advance one word and start from r4 register to copy
+         parameters.  This also consumes one on-stack parameter slot.  */
+      if (struct_return)
+	{
+	  if (write_pass)
+	    regcache_cooked_write_signed (regcache,
+					  tdep->ppc_gp0_regnum + greg,
+					  struct_addr);
+	  greg++;
+	  gparam = align_up (gparam + tdep->wordsize, tdep->wordsize);
+	}
+
+      for (argno = 0; argno < nargs; argno++)
+	{
+	  struct value *arg = args[argno];
+	  struct type *type = check_typedef (VALUE_TYPE (arg));
+	  char *val = VALUE_CONTENTS (arg);
+	  if (TYPE_CODE (type) == TYPE_CODE_FLT && TYPE_LENGTH (type) <= 8)
+	    {
+	      /* Floats and Doubles go in f1 .. f13.  They also
+	         consume a left aligned GREG,, and can end up in
+	         memory.  */
+	      if (write_pass)
+		{
+		  if (ppc_floating_point_unit_p (current_gdbarch)
+		      && freg <= 13)
+		    {
+		      char regval[MAX_REGISTER_SIZE];
+		      struct type *regtype = register_type (gdbarch,
+							    FP0_REGNUM);
+		      convert_typed_floating (val, type, regval, regtype);
+		      regcache_cooked_write (regcache, FP0_REGNUM + freg,
+					     regval);
+		    }
+		  if (greg <= 10)
+		    {
+		      /* The ABI states "Single precision floating
+		         point values are mapped to the first word in
+		         a single doubleword" and "... floating point
+		         values mapped to the first eight doublewords
+		         of the parameter save area are also passed in
+		         general registers").
+
+		         This code interprets that to mean: store it,
+		         left aligned, in the general register.  */
+		      char regval[MAX_REGISTER_SIZE];
+		      memset (regval, 0, sizeof regval);
+		      memcpy (regval, val, TYPE_LENGTH (type));
+		      regcache_cooked_write (regcache,
+					     tdep->ppc_gp0_regnum + greg,
+					     regval);
+		    }
+		  write_memory (gparam, val, TYPE_LENGTH (type));
+		}
+	      /* Always consume parameter stack space.  */
+	      freg++;
+	      greg++;
+	      gparam = align_up (gparam + TYPE_LENGTH (type), tdep->wordsize);
+	    }
+	  else if (TYPE_LENGTH (type) == 16 && TYPE_VECTOR (type)
+		   && TYPE_CODE (type) == TYPE_CODE_ARRAY
+		   && tdep->ppc_vr0_regnum >= 0)
+	    {
+	      /* In the Altivec ABI, vectors go in the vector
+	         registers v2 .. v13, or when that runs out, a vector
+	         annex which goes above all the normal parameters.
+	         NOTE: cagney/2003-09-21: This is a guess based on the
+	         PowerOpen Altivec ABI.  */
+	      if (vreg <= 13)
+		{
+		  if (write_pass)
+		    regcache_cooked_write (regcache,
+					   tdep->ppc_vr0_regnum + vreg, val);
+		  vreg++;
+		}
+	      else
+		{
+		  if (write_pass)
+		    write_memory (vparam, val, TYPE_LENGTH (type));
+		  vparam = align_up (vparam + TYPE_LENGTH (type), 16);
+		}
+	    }
+	  else if ((TYPE_CODE (type) == TYPE_CODE_INT
+		    || TYPE_CODE (type) == TYPE_CODE_ENUM)
+		   && TYPE_LENGTH (type) <= 8)
+	    {
+	      /* Scalars get sign[un]extended and go in gpr3 .. gpr10.
+	         They can also end up in memory.  */
+	      if (write_pass)
+		{
+		  /* Sign extend the value, then store it unsigned.  */
+		  ULONGEST word = unpack_long (type, val);
+		  if (greg <= 10)
+		    regcache_cooked_write_unsigned (regcache,
+						    tdep->ppc_gp0_regnum +
+						    greg, word);
+		  write_memory_unsigned_integer (gparam, tdep->wordsize,
+						 word);
+		}
+	      greg++;
+	      gparam = align_up (gparam + TYPE_LENGTH (type), tdep->wordsize);
+	    }
+	  else
+	    {
+	      int byte;
+	      for (byte = 0; byte < TYPE_LENGTH (type);
+		   byte += tdep->wordsize)
+		{
+		  if (write_pass && greg <= 10)
+		    {
+		      char regval[MAX_REGISTER_SIZE];
+		      int len = TYPE_LENGTH (type) - byte;
+		      if (len > tdep->wordsize)
+			len = tdep->wordsize;
+		      memset (regval, 0, sizeof regval);
+		      /* WARNING: cagney/2003-09-21: As best I can
+		         tell, the ABI specifies that the value should
+		         be left aligned.  Unfortunately, GCC doesn't
+		         do this - it instead right aligns even sized
+		         values and puts odd sized values on the
+		         stack.  Work around that by putting both a
+		         left and right aligned value into the
+		         register (hopefully no one notices :-^).
+		         Arrrgh!  */
+		      /* Left aligned (8 byte values such as pointers
+		         fill the buffer).  */
+		      memcpy (regval, val + byte, len);
+		      /* Right aligned (but only if even).  */
+		      if (len == 1 || len == 2 || len == 4)
+			memcpy (regval + tdep->wordsize - len,
+				val + byte, len);
+		      regcache_cooked_write (regcache, greg, regval);
+		    }
+		  greg++;
+		}
+	      if (write_pass)
+		/* WARNING: cagney/2003-09-21: Strictly speaking, this
+		   isn't necessary, unfortunately, GCC appears to get
+		   "struct convention" parameter passing wrong putting
+		   odd sized structures in memory instead of in a
+		   register.  Work around this by always writing the
+		   value to memory.  Fortunately, doing this
+		   simplifies the code.  */
+		write_memory (gparam, val, TYPE_LENGTH (type));
+	      /* Always consume parameter stack space.  */
+	      gparam = align_up (gparam + TYPE_LENGTH (type), tdep->wordsize);
+	    }
+	}
+
+      if (!write_pass)
+	{
+	  /* Save the true region sizes ready for the second pass.  */
+	  vparam_size = vparam;
+	  /* Make certain that the general parameter save area is at
+	     least the minimum 8 registers (or doublewords) in size.  */
+	  if (greg < 8)
+	    gparam_size = 8 * tdep->wordsize;
+	  else
+	    gparam_size = gparam;
+	}
+    }
+
+  /* Update %sp.   */
+  regcache_cooked_write_signed (regcache, SP_REGNUM, sp);
+
+  /* Write the backchain (it occupies WORDSIZED bytes).  */
+  write_memory_signed_integer (sp, tdep->wordsize, back_chain);
+
+  /* Point the inferior function call's return address at the dummy's
+     breakpoint.  */
+  regcache_cooked_write_signed (regcache, tdep->ppc_lr_regnum, bp_addr);
+
+  /* Find a value for the TOC register.  Every symbol should have both
+     ".FN" and "FN" in the minimal symbol table.  "FN" points at the
+     FN's descriptor, while ".FN" points at the entry point (which
+     matches FUNC_ADDR).  Need to reverse from FUNC_ADDR back to the
+     FN's descriptor address.  */
+  {
+    /* Find the minimal symbol that corresponds to FUNC_ADDR (should
+       have the name ".FN").  */
+    struct minimal_symbol *dot_fn = lookup_minimal_symbol_by_pc (func_addr);
+    if (dot_fn != NULL && SYMBOL_LINKAGE_NAME (dot_fn)[0] == '.')
+      {
+	/* Now find the corresponding "FN" (dropping ".") minimal
+	   symbol's address.  */
+	struct minimal_symbol *fn =
+	  lookup_minimal_symbol (SYMBOL_LINKAGE_NAME (dot_fn) + 1, NULL,
+				 NULL);
+	if (fn != NULL)
+	  {
+	    /* Got the address of that descriptor.  The TOC is the
+	       second double word.  */
+	    CORE_ADDR toc =
+	      read_memory_unsigned_integer (SYMBOL_VALUE_ADDRESS (fn) +
+					    tdep->wordsize, tdep->wordsize);
+	    regcache_cooked_write_unsigned (regcache,
+					    tdep->ppc_gp0_regnum + 2, toc);
+	  }
+      }
+  }
+
+  return sp;
 }
 
 
Index: ppc-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/ppc-tdep.h,v
retrieving revision 1.19
diff -u -r1.19 ppc-tdep.h
--- ppc-tdep.h	3 Oct 2003 21:11:39 -0000	1.19
+++ ppc-tdep.h	10 Oct 2003 01:25:03 -0000
@@ -42,6 +42,13 @@
 					struct value **args, CORE_ADDR sp,
 					int struct_return,
 					CORE_ADDR struct_addr);
+CORE_ADDR ppc64_sysv_abi_push_dummy_call (struct gdbarch *gdbarch,
+					  CORE_ADDR func_addr,
+					  struct regcache *regcache,
+					  CORE_ADDR bp_addr, int nargs,
+					  struct value **args, CORE_ADDR sp,
+					  int struct_return,
+					  CORE_ADDR struct_addr);
 int ppc_linux_memory_remove_breakpoint (CORE_ADDR addr, char *contents_cache);
 struct link_map_offsets *ppc_linux_svr4_fetch_link_map_offsets (void);
 void ppc_linux_supply_gregset (char *buf);
Index: rs6000-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.166
diff -u -r1.166 rs6000-tdep.c
--- rs6000-tdep.c	3 Oct 2003 21:11:39 -0000	1.166
+++ rs6000-tdep.c	10 Oct 2003 01:25:04 -0000
@@ -2959,6 +2959,8 @@
      revisited.  */
   if (sysv_abi && wordsize == 4)
     set_gdbarch_push_dummy_call (gdbarch, ppc_sysv_abi_push_dummy_call);
+  else if (sysv_abi && wordsize == 8)
+    set_gdbarch_push_dummy_call (gdbarch, ppc64_sysv_abi_push_dummy_call);
   else
     set_gdbarch_push_dummy_call (gdbarch, rs6000_push_dummy_call);