This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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
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);
 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]