Borland fastcall support (i386)

Borland fastcall support (i386)

[Jonas Maebe]

Hello,

I've got a first version ready of a patch to add support for the  
Borland fastcall calling convention. It is the default calling  
convention of Borland/CodeGear/Embarcadero Delphi, and because of that  
also of the Free Pascal Compiler (FPC) on i386 platforms.

This probably cannot (yet) be committed as is, because
a) my FSF copyright assignment is not yet finalized (waiting for the  
contracts department of my employer to approve)
b) one of the modified headers (include/elf/dwarf2.h) appears to be  
imported from gcc, so I guess I should submit that change to gcc first  
or instead?
c) the patch is against Archer rather than against CVS head (although  
I don't know how different they are as far as the modified files are  
concerned)

I've verified its basic operation on Mac OS X/i386 by modifying FPC to  
add the appropriate calling convention attribute to the DWARF info and  
calling a bunch of different procedures and functions in a small test  
program (with/without structured function result, with/without  
parameters on the stack).

I'm mainly posting it now already to get comments on whether this the  
correct approach, and to see whether I didn't make any obvious  
mistakes against the coding guidelines or other conventions. The  
processing of the stack paramaters is basically a copy/paste from  
i386_push_dummy_call, except that it goes through the parameters in  
the opposite order and skips register parameters.

Regarding the constant chosen for the calling convention: as I  
mentioned on the gdb list, there is no procedure to reserve such new  
identifiers, so I've simply picked the next one available. My mail to  
the DWARF list on this topic did result in a call to document all  
extensions at http://wiki.dwarfstd.org/index.php? 
title=Vendor_Extensions though, so it is easier to avoid multiple  
vendors creating overlapping/conflicting extensions.

Thanks,


Jonas

PS: I'm not subscribed to this list, so please CC me in replies.
PS2: I'm not sure if the check_typedef() I used is necessary. It's  
used in some places and not in others, and I'm not really sure when it  
is required and when not.


2009-04-03  Jonas Maebe  <jonas.maebe@elis.ugent.be>

	Add support for the "Borland fastcall" calling convention.

	* elf/dwarf2.h: Add DW_CC_GNU_borland_fastcall_i386 constant.
	* i386-tdep.c: #include elf/dwarf2.h
	(i386_push_dummy_borland_fast_call): New.
	(i386_push_dummy_generic_call): New dispatch function that calls
	i386_push_dummy_call or i386_push_dummy_borland_fast_call depending
	on the calling convention.
	(i386_gdbarch_init): Install i386_push_dummy_generic_call as
	push_dummy_call handler.

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index b74270a..11e83c1 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -50,6 +50,8 @@
  #include "i386-tdep.h"
  #include "i387-tdep.h"

+#include "elf/dwarf2.h"
+
  /* Register names.  */

  static char *i386_register_names[] =
@@ -1753,6 +1755,151 @@ i386_push_dummy_call (struct gdbarch *gdbarch,  
struct value *function,
    return sp + 8;
  }

+/* Borland fastcall: register parameters are passed from left to
+   right, stack parameters from right to left. The first three  
unstructured
+   parameters <= 32 bits are passed in eax, edx and ecx. The others  
are passed
+   on the stack. Furthermore, in case of a struct return by  
reference, the
+   address of this struct is passed as the last parameter.  */
+static CORE_ADDR
+i386_push_dummy_borland_fast_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)
+{
+  static const int para_regs[3] = {I386_EAX_REGNUM, I386_EDX_REGNUM,  
I386_ECX_REGNUM};
+
+  gdb_byte buf[4];
+  int reg_paras[3] = {-1, -1, -1};
+  int i, j;
+  int write_pass;
+  int para_regnum = 0;
+
+  /* First assign the register parameters (left to right).  */
+  for (i = 0; i < nargs && para_regnum < 3; i++)
+    {
+      struct type *type = check_typedef(value_enclosing_type  
(args[i]));
+      int len = TYPE_LENGTH (type);
+
+      if (len <= 4
+	  && TYPE_CODE(type) != TYPE_CODE_ARRAY
+	  && TYPE_CODE(type) != TYPE_CODE_STRUCT
+	  && TYPE_CODE(type) != TYPE_CODE_FLT)
+	{
+	  regcache_cooked_write (regcache, para_regs[para_regnum],
+				 value_contents_all (args[i]));
+	  reg_paras[para_regnum] = i;
+	  para_regnum++;
+	}
+    }
+  if (struct_return)
+    {
+      if (para_regnum < 3)
+	{
+	  store_unsigned_integer (buf, 4, struct_addr);
+	  regcache_cooked_write (regcache, para_regs[para_regnum],
+				 buf);
+	  reg_paras[para_regnum] = nargs;
+	  para_regnum++;
+	}
+    }
+
+  /* Now process the stack parameters from right to left.  */
+  for (write_pass = 0; write_pass < 2; write_pass++)
+    {
+      int args_space = 0;
+      int have_16_byte_aligned_arg = 0;
+
+      for (i = nargs - 1; i >= 0; i--)
+	{
+	  struct type *type = check_typedef(value_enclosing_type (args[i]));
+	  int len = TYPE_LENGTH (type);
+	  int processed = 0;
+
+	  /* Skip parameters already assigned to registers.  */
+	  for (j = 0; j < para_regnum; j++)
+	    if (reg_paras[j] == i)
+	      {
+		processed = 1;
+		break;
+	      }
+	  if (processed)
+	    continue;
+
+	  if (i386_16_byte_align_p (value_enclosing_type (args[i])))
+	    {
+	      args_space = align_up (args_space, 16);
+	      have_16_byte_aligned_arg = 1;
+	    }
+	  if (write_pass)
+	    {
+	      write_memory (sp + args_space,
+			    value_contents_all (args[i]), len);
+	    }
+	  args_space += align_up (len, 4);
+	}
+
+      /* If we have a struct_return then para_regnum cannot be 0,  
since at
+         least this return struct would have been passed in a register.
+	 Additionally, if it is passed via a register, it will always be in
+         last used position of the reg_paras array.  */
+      if (struct_return
+	  && reg_paras[para_regnum-1] != nargs)
+	{
+	  if (write_pass)
+	    {
+	      /* Push value address.  */
+	      store_unsigned_integer (buf, 4, struct_addr);
+	      write_memory (sp + args_space, buf, 4);
+	    }
+	  args_space += 4;
+	}
+
+      if (!write_pass)
+	{
+	  /* Early exit if nothing to do.  */
+	  if (!args_space)
+	    break;
+	  if (have_16_byte_aligned_arg)
+	    args_space = align_up (args_space, 16);
+	  sp -= args_space;
+
+	}
+     }
+
+  /* Store return address.  */
+  sp -= 4;
+  store_unsigned_integer (buf, 4, bp_addr);
+  write_memory (sp, buf, 4);
+
+  /* Finally, update the stack pointer...  */
+  store_unsigned_integer (buf, 4, sp);
+  regcache_cooked_write (regcache, I386_ESP_REGNUM, buf);
+
+  /* ...and fake a frame pointer.  */
+  regcache_cooked_write (regcache, I386_EBP_REGNUM, buf);
+
+  /* See the end of i386_push_dummy_call.  */
+  return sp + 8;
+}
+
+static CORE_ADDR
+i386_push_dummy_generic_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 type *type = check_typedef (value_type (function));
+  if (type
+      && TYPE_CALLING_CONVENTION (type) ==  
DW_CC_GNU_borland_fastcall_i386)
+    return i386_push_dummy_borland_fast_call (gdbarch, function,  
regcache,
+					      bp_addr, nargs, args, sp,
+					      struct_return, struct_addr);
+  else
+    return i386_push_dummy_call (gdbarch, function, regcache,  
bp_addr, nargs,
+				 args, sp, struct_return, struct_addr);
+}
+
  /* These registers are used for returning integers (and on some
     targets also for returning `struct' and `union' values when their
     size and alignment match an integer type).  */
@@ -2799,7 +2946,7 @@ i386_gdbarch_init (struct gdbarch_info info,  
struct gdbarch_list *arches)
    set_gdbarch_get_longjmp_target (gdbarch, i386_get_longjmp_target);

    /* Call dummy code.  */
-  set_gdbarch_push_dummy_call (gdbarch, i386_push_dummy_call);
+  set_gdbarch_push_dummy_call (gdbarch, i386_push_dummy_generic_call);

    set_gdbarch_convert_register_p (gdbarch, i386_convert_register_p);
    set_gdbarch_register_to_value (gdbarch,  i386_register_to_value);
diff --git a/include/elf/dwarf2.h b/include/elf/dwarf2.h
index f8d010b..915d40d 100644
--- a/include/elf/dwarf2.h
+++ b/include/elf/dwarf2.h
@@ -662,7 +662,8 @@ enum dwarf_calling_convention
      DW_CC_normal = 0x1,
      DW_CC_program = 0x2,
      DW_CC_nocall = 0x3,
-    DW_CC_GNU_renesas_sh = 0x40
+    DW_CC_GNU_renesas_sh = 0x40,
+    DW_CC_GNU_borland_fastcall_i386 = 0x41
    };

  #define DW_CC_lo_user 0x40

Re: Borland fastcall support (i386)

[Tom Tromey]

>>>>> "Jonas" == Jonas Maebe <jonas.maebe@elis.ugent.be> writes:

Jonas> b) one of the modified headers (include/elf/dwarf2.h) appears to be
Jonas> imported from gcc, so I guess I should submit that change to gcc first
Jonas> or instead?

I recently noticed that the src and gcc versions of this file have
diverged.  Oops.  It would be good to have just a single copy; most of
the work to be done here is in the gcc repository.

Jonas> c) the patch is against Archer rather than against CVS head (although
Jonas> I don't know how different they are as far as the modified files are
Jonas> concerned)

The 'master' branch of Archer's git repository is updated from CVS
head semi-regularly.  If you are using some other branch, then yeah,
there may be other changes in the way.

Alternatively, you could also pull from the gdb git repository and use
gdb/master, which I think is updated every half hour.

Tom

Re: Borland fastcall support (i386)

[Jonas Maebe]

On 03 Apr 2009, at 16:53, Tom Tromey wrote:

>>>>>> "Jonas" == Jonas Maebe <jonas.maebe@elis.ugent.be> writes:
>
> Jonas> b) one of the modified headers (include/elf/dwarf2.h) appears  
> to be
> Jonas> imported from gcc, so I guess I should submit that change to  
> gcc first
> Jonas> or instead?
>
> I recently noticed that the src and gcc versions of this file have
> diverged.  Oops.  It would be good to have just a single copy; most of
> the work to be done here is in the gcc repository.

I've submitted the patch to gcc-patches.

> The 'master' branch of Archer's git repository is updated from CVS
> head semi-regularly.  If you are using some other branch, then yeah,
> there may be other changes in the way.

The patch was against the "master" branch. I've made some minor  
adjustments to it in the mean time, and I've also received word from  
my employer's contracts department that there will be no problem with  
getting a disclaimer :)

I've continued working on gdb to also support calling Pascal methods,  
and I've run a bit into a hitch: the calling_convention field is part  
of a union that also contains cplus_stuff (inside main_type in  
gdbtypes.h), and I need both for methods using a non-standard calling  
convention. Is there a problem with moving the calling_convention  
field out of the type_specific union, and if not, is there anything I  
should pay particular attention to? (other than adjusting a couple of  
macros)

Thanks,


Jonas

Re: Borland fastcall support (i386)

[Tom Tromey]

>>>>> "Jonas" == Jonas Maebe <jonas.maebe@elis.ugent.be> writes:

Tom> I recently noticed that the src and gcc versions of this file have
Tom> diverged.  Oops.  It would be good to have just a single copy; most of
Tom> the work to be done here is in the gcc repository.

Jonas> I've submitted the patch to gcc-patches.

Actually, I was hoping for something more like a merge of the two
copies of dwarf2.h.  Don't feel obligated to do this, though; the
problem predates your patch.

Jonas> Is there a problem with moving the calling_convention field out
Jonas> of the type_specific union, and if not, is there anything I
Jonas> should pay particular attention to? (other than adjusting a
Jonas> couple of macros)

ISTR that the type-related structures are (supposedly) space
sensitive.  But, according to pahole, there are some free bits in
struct main_type.  So, I would suggest packing the field in there.
Aside from that I don't know of any reason it might be a problem.

Tom

Re: Borland fastcall support (i386)

[Daniel Jacobowitz]

On Sun, Apr 05, 2009 at 11:28:57PM +0200, Jonas Maebe wrote:
> I've continued working on gdb to also support calling Pascal methods, and 
> I've run a bit into a hitch: the calling_convention field is part of a 
> union that also contains cplus_stuff (inside main_type in gdbtypes.h), and 
> I need both for methods using a non-standard calling convention. Is there 
> a problem with moving the calling_convention field out of the 
> type_specific union, and if not, is there anything I should pay particular 
> attention to? (other than adjusting a couple of macros)

Why is cplus_stuff used in this case?

    /* CPLUS_STUFF is for TYPE_CODE_STRUCT.  It is initialized to point to
       cplus_struct_default, a default static instance of a struct
       cplus_struct_type. */

    struct cplus_struct_type *cplus_stuff;

Methods don't have base classes, methods of their own, or so forth.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Borland fastcall support (i386)

[Jonas Maebe]

On 06 Apr 2009, at 04:54, Daniel Jacobowitz wrote:

> On Sun, Apr 05, 2009 at 11:28:57PM +0200, Jonas Maebe wrote:
>> I've continued working on gdb to also support calling Pascal  
>> methods, and
>> I've run a bit into a hitch: the calling_convention field is part  
>> of a
>> union that also contains cplus_stuff (inside main_type in  
>> gdbtypes.h), and
>> I need both for methods using a non-standard calling convention.
>
> Why is cplus_stuff used in this case?

You're right, I was mixing two things. There is indeed no conflict. So  
all that needs to be done is extend smash_to_method_type() with an  
extra parameter for the calling convention.


Jonas