[RFA Darwin v2]: Add push_dummy_call for i386

[RFA Darwin v2]: Add push_dummy_call for i386

[Tristan Gingold]

Hi,

updated version of the patch.  All comments have been addressed.

Also, I downloaded the last version of the Darwin ABI document (2009-02-04).
It now properly how structures/unions are passed and I therefore handle this
case (In the previous revision the behaviour was so strange that I prefered
not to handle this case).

Tristan.

2009-07-06  Tristan Gingold  <gingold@adacore.com>

	* i386-darwin-tdep.c (i386_m128_p): New function.
	(i386_darwin_arg_type_alignment): Ditto.
	(i386_darwin_push_dummy_call): Ditto.
	(i386_darwin_init_abi): Define Darwin specific push_dummy_call.
	Adjust long_double size.  Adjust pc offset in setjump buffer.

Index: i386-darwin-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-darwin-tdep.c,v
retrieving revision 1.6
diff -u -p -r1.6 i386-darwin-tdep.c
--- i386-darwin-tdep.c	3 Jul 2009 12:06:36 -0000	1.6
+++ i386-darwin-tdep.c	6 Jul 2009 07:32:48 -0000
@@ -109,6 +109,144 @@ darwin_dwarf_signal_frame_p (struct gdba
   return i386_sigtramp_p (this_frame);
 }
 
+/* Check wether TYPE is a 128-bit vector (__m128, __m128d or __m128i).  */
+
+static int
+i386_m128_p (struct type *type)
+{
+  return TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_VECTOR (type)
+    && TYPE_LENGTH (type) == 16;
+}
+
+/* Return the alignment for TYPE when passed as an argument.  */
+
+static int
+i386_darwin_arg_type_alignment (struct type *type)
+{
+  type = check_typedef (type);
+  /* Passing arguments.
+     6.  The caller places 64-bit vectors (__m64) on the parameter area,
+         aligned to 8-byte boundaries.
+     7.  [...]  The caller aligns 128-bit vectors in the parameter area to
+         16-byte boundaries.  */
+  if (TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_VECTOR (type))
+    return TYPE_LENGTH (type);
+  /* 4.  The caller places all the fields of structures (or unions) with no
+         vector elements in the parameter area.  These structures are 4-byte
+         aligned.
+     5.  The caller places structures with vector elements on the stack,
+         16-byte aligned.  */
+  if (TYPE_CODE (type) == TYPE_CODE_STRUCT
+      || TYPE_CODE (type) == TYPE_CODE_UNION)
+    {
+      int i;
+      int res = 4;
+      for (i = 0; i < TYPE_NFIELDS (type); i++)
+        res = max (res,
+                   i386_darwin_arg_type_alignment (TYPE_FIELD_TYPE (type, i)));
+      return res;
+    }
+  /* 2.  The caller aligns nonvector arguments to 4-byte boundaries.  */
+  return 4;
+}
+
+static CORE_ADDR
+i386_darwin_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
+			     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 (gdbarch);
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  gdb_byte buf[4];
+  int i;
+  int write_pass;
+
+  /* Determine the total space required for arguments and struct
+     return address in a first pass, then push arguments in a second pass.  */
+
+  for (write_pass = 0; write_pass < 2; write_pass++)
+    {
+      int args_space = 0;
+      int num_m128 = 0;
+
+      if (struct_return)
+	{
+	  if (write_pass)
+	    {
+	      /* Push value address.  */
+	      store_unsigned_integer (buf, 4, byte_order, struct_addr);
+	      write_memory (sp, buf, 4);
+	    }
+          args_space += 4;
+	}
+
+      for (i = 0; i < nargs; i++)
+	{
+          struct type *arg_type = value_enclosing_type (args[i]);
+
+          if (i386_m128_p (arg_type) && num_m128 < 4)
+            {
+              if (write_pass)
+                {
+                  const gdb_byte *val = value_contents_all (args[i]);
+                  regcache_raw_write
+                    (regcache, I387_MM0_REGNUM(tdep) + num_m128, val);
+                }
+              num_m128++;
+            }
+          else
+            {
+              int len = TYPE_LENGTH (arg_type);
+              int align = i386_darwin_arg_type_alignment (arg_type);
+
+              args_space = align_up (args_space, align);
+              if (write_pass)
+                write_memory (sp + args_space,
+                              value_contents_all (args[i]), len);
+
+              /* The System V ABI says that:
+                 
+                 "An argument's size is increased, if necessary, to make it a
+                 multiple of [32-bit] words.  This may require tail padding,
+                 depending on the size of the argument."
+                 
+                 This makes sure the stack stays word-aligned.  */
+              args_space += align_up (len, 4);
+            }
+        }
+
+      /* Darwin i386 ABI:
+	 1.  The caller ensures that the stack is 16-byte aligned at the point
+	     of the function call.  */
+      if (!write_pass)
+	sp = align_down (sp - args_space, 16);
+    }
+
+  /* Store return address.  */
+  sp -= 4;
+  store_unsigned_integer (buf, 4, byte_order, bp_addr);
+  write_memory (sp, buf, 4);
+
+  /* Finally, update the stack pointer...  */
+  store_unsigned_integer (buf, 4, byte_order, sp);
+  regcache_cooked_write (regcache, I386_ESP_REGNUM, buf);
+
+  /* ...and fake a frame pointer.  */
+  regcache_cooked_write (regcache, I386_EBP_REGNUM, buf);
+
+  /* MarkK wrote: This "+ 8" is all over the place:
+     (i386_frame_this_id, i386_sigtramp_frame_this_id,
+     i386_dummy_id).  It's there, since all frame unwinders for
+     a given target have to agree (within a certain margin) on the
+     definition of the stack address of a frame.  Otherwise frame id
+     comparison might not work correctly.  Since DWARF2/GCC uses the
+     stack address *before* the function call as a frame's CFA.  On
+     the i386, when %ebp is used as a frame pointer, the offset
+     between the contents %ebp and the CFA as defined by GCC.  */
+  return sp + 8;
+}
+
 static void
 i386_darwin_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -119,6 +257,7 @@ i386_darwin_init_abi (struct gdbarch_inf
   set_gdbarch_num_regs (gdbarch, I386_SSE_NUM_REGS);
 
   dwarf2_frame_set_signal_frame_p (gdbarch, darwin_dwarf_signal_frame_p);
+  set_gdbarch_push_dummy_call (gdbarch, i386_darwin_push_dummy_call);
 
   tdep->struct_return = reg_struct_return;
 
@@ -127,7 +266,12 @@ i386_darwin_init_abi (struct gdbarch_inf
   tdep->sc_reg_offset = i386_darwin_thread_state_reg_offset;
   tdep->sc_num_regs = i386_darwin_thread_state_num_regs;
 
-  tdep->jb_pc_offset = 20;
+  tdep->jb_pc_offset = 48;
+
+  /* Although the i387 extended floating-point has only 80 significant
+     bits, a `long double' actually takes up 128, probably to enforce
+     alignment.  */
+  set_gdbarch_long_double_bit (gdbarch, 128);
 
   set_solib_ops (gdbarch, &darwin_so_ops);
 }

Re: [RFA Darwin v2]: Add push_dummy_call for i386

[Joel Brobecker]

> 2009-07-06  Tristan Gingold  <gingold@adacore.com>
> 
> 	* i386-darwin-tdep.c (i386_m128_p): New function.
> 	(i386_darwin_arg_type_alignment): Ditto.
> 	(i386_darwin_push_dummy_call): Ditto.
> 	(i386_darwin_init_abi): Define Darwin specific push_dummy_call.
> 	Adjust long_double size.  Adjust pc offset in setjump buffer.

This looks OK to me. 

> +static int
> +i386_m128_p (struct type *type)
> +{
> +  return TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_VECTOR (type)
> +    && TYPE_LENGTH (type) == 16;
> +}

Just one tiny comment: The GNU Coding Standard likes it when we use
parens around the returned expression in this case, even if it is
unecessary. It helps automatic formatting. Would you mind rewriting
it as follow:

   return (TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_VECTOR
           && TYPE_LENGTH (type) == 16);

?

-- 
Joel

Re: [RFA Darwin v2]: Add push_dummy_call for i386

[Tristan Gingold]

On Jul 16, 2009, at 9:07 PM, Joel Brobecker wrote:

>> 2009-07-06  Tristan Gingold  <gingold@adacore.com>
>>
>> 	* i386-darwin-tdep.c (i386_m128_p): New function.
>> 	(i386_darwin_arg_type_alignment): Ditto.
>> 	(i386_darwin_push_dummy_call): Ditto.
>> 	(i386_darwin_init_abi): Define Darwin specific push_dummy_call.
>> 	Adjust long_double size.  Adjust pc offset in setjump buffer.
>
> This looks OK to me.
>
>> +static int
>> +i386_m128_p (struct type *type)
>> +{
>> +  return TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_VECTOR (type)
>> +    && TYPE_LENGTH (type) == 16;
>> +}
>
> Just one tiny comment: The GNU Coding Standard likes it when we use
> parens around the returned expression in this case, even if it is
> unecessary. It helps automatic formatting. Would you mind rewriting
> it as follow:
>
>   return (TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_VECTOR
>           && TYPE_LENGTH (type) == 16);

Thanks, committed with this change.

tristan.