[RFA/commit 2/3] amd64-windows: memory args passed by pointer during function calls.

[RFA/commit 2/3] amd64-windows: memory args passed by pointer during function calls.

[Joel Brobecker]

From: brobecke <brobecke@f8352e7e-cb20-0410-8ce7-b5d9e71c585c>

Another difference between Linux and Windows on amd64: When an argument
is passed through the stack (aka MEMORY), the argument address must also
be passed in the associated integer register (if still available).

http://msdn.microsoft.com/en-us/library/zthk2dkh%28VS.80%29.aspx:

    __m128 types, arrays and strings are never passed by immediate value
    but rather a pointer will be passed to memory allocated by the caller.
    Structs/unions of size 8, 16, 32, or 64 bits and __m64 will
    be passed as if they were integers of the same size. Structs/unions
    other than these sizes will be passed as a pointer to memory
    allocated by the caller.

I should also mention that this affect return values.  This is actually
the situation that I started looking at, because I had a testcase where
a function taking 3 integers was called, and I thought that was the
simplest of all the function-call failures I was looking at:

    struct large
    {
      int x;
      int y;
      int z;
    };

    struct large
    create (int x, int y, int z)
    {
      struct large result = {x, y, z};

      return result;
    }

When I call create with 3 parameters, I get a SIGSEGV:

    (gdb) print create (74, 8, 4)

    Program received signal SIGSEGV, Segmentation fault.
    0x0000000000401695 in create (x=8, y=4, z=2293376) at bar.c:18
    18        return result;

(notice how the parameters get shifted to the left, since the function
thinks that the address of the return value is 74).

This patch enhances the amd64_push_arguments routine, allowing it to
follow either calling convention.  Which calling convention is used
depends on the value of a new field in struct gdbarch_tdep
(memory_args_by_pointer).  The default value is zero (Linux convention),
while amd64-windows-tdep sets this field to 1.

gdb/ChangeLog:

    * i386-tdep.h (gdbarch_tdep): Add field memory_args_by_pointer.
    * amd64-tdep.c (amd64_push_arguments): Add handling of architectures
    where tdep->memory_args_by_pointer is non-zero.
    * amd64-windows-tdep.c (amd64_windows_init_abi): Set
    tdep->memory_args_by_pointer to 1.

I can certainly submit a new testcase, but it looks like struct.exp
should cover much more than this case...

-- 
Joel

---
 gdb/amd64-tdep.c         |   37 +++++++++++++++++++++++++++++++++----
 gdb/amd64-windows-tdep.c |    1 +
 gdb/i386-tdep.h          |    9 +++++++++
 3 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index f69f3f6..e9e0809 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -570,7 +570,8 @@ static CORE_ADDR
 amd64_push_arguments (struct regcache *regcache, int nargs,
 		      struct value **args, CORE_ADDR sp, int struct_return)
 {
-  struct gdbarch_tdep *tdep = gdbarch_tdep (get_regcache_arch (regcache));
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   int *integer_regs = tdep->call_dummy_integer_regs;
   int num_integer_regs = tdep->call_dummy_num_integer_regs;
 
@@ -583,6 +584,11 @@ amd64_push_arguments (struct regcache *regcache, int nargs,
     AMD64_XMM0_REGNUM + 6, AMD64_XMM0_REGNUM + 7,
   };
   struct value **stack_args = alloca (nargs * sizeof (struct value *));
+  /* An array that mirrors the stack_args array.  For all arguments
+     that are passed by MEMORY, if that argument's address also needs
+     to be stored in a register, the ARG_ADDR_REGNO array will contain
+     that register number (or a negative value otherwise).  */
+  int *arg_addr_regno = alloca (nargs * sizeof (int));
   int num_stack_args = 0;
   int num_elements = 0;
   int element = 0;
@@ -626,7 +632,19 @@ amd64_push_arguments (struct regcache *regcache, int nargs,
 	{
 	  /* The argument will be passed on the stack.  */
 	  num_elements += ((len + 7) / 8);
-	  stack_args[num_stack_args++] = args[i];
+	  stack_args[num_stack_args] = args[i];
+          /* If this is an AMD64_MEMORY argument whose address must also
+             be passed in one of the integer registers, reserve that
+             register and associate this value to that register so that
+             we can store the argument address as soon as we know it.  */
+          if (class[0] == AMD64_MEMORY
+              && tdep->memory_args_by_pointer
+              && integer_reg < tdep->call_dummy_num_integer_regs)
+            arg_addr_regno[num_stack_args] =
+              tdep->call_dummy_integer_regs[integer_reg++];
+          else
+            arg_addr_regno[num_stack_args] = -1;
+          num_stack_args++;
 	}
       else
 	{
@@ -682,8 +700,19 @@ amd64_push_arguments (struct regcache *regcache, int nargs,
       struct type *type = value_type (stack_args[i]);
       const gdb_byte *valbuf = value_contents (stack_args[i]);
       int len = TYPE_LENGTH (type);
-
-      write_memory (sp + element * 8, valbuf, len);
+      CORE_ADDR arg_addr = sp + element * 8;
+
+      write_memory (arg_addr, valbuf, len);
+      if (arg_addr_regno[i] >= 0)
+        {
+          /* We also need to store the address of that argument in
+             the given register.  */
+          gdb_byte buf[8];
+          enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+
+          store_unsigned_integer (buf, 8, byte_order, arg_addr);
+          regcache_cooked_write (regcache, arg_addr_regno[i], buf);
+        }
       element += ((len + 7) / 8);
     }
 
diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c
index b5a0035..0ed9368 100644
--- a/gdb/amd64-windows-tdep.c
+++ b/gdb/amd64-windows-tdep.c
@@ -83,6 +83,7 @@ amd64_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
     ARRAY_SIZE (amd64_windows_dummy_call_integer_regs);
   tdep->call_dummy_integer_regs = amd64_windows_dummy_call_integer_regs;
   tdep->classify = amd64_windows_classify;
+  tdep->memory_args_by_pointer = 1;
 
   set_solib_ops (gdbarch, &solib_target_so_ops);
 }
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index d5b24fa..f79a15d 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -86,6 +86,15 @@ struct gdbarch_tdep
      the result in CLASS.  Used on amd64 only.  */
   void (*classify) (struct type *type, enum amd64_reg_class class[2]);
 
+  /* Non-zero if the first few MEMORY arguments should be passed by
+     pointer.
+
+     More precisely, MEMORY arguments are passed through the stack.
+     But certain architectures require that their address be passed
+     by register as well, if there are still some integer registers
+     available for argument passing.  */
+  int memory_args_by_pointer;
+
   /* Floating-point registers.  */
   struct regset *fpregset;
   size_t sizeof_fpregset;
-- 
1.6.3.3

[RFA/commit 1/3] amd64: Integer parameters in function calls on Windows.

[Joel Brobecker]

From: brobecke <brobecke@f8352e7e-cb20-0410-8ce7-b5d9e71c585c>

First problem found regarding function calls on amd64-windows:
Integer parameters are not passed correctly.  After investigation,
I realized that the registers used for integer parameter passing
are quite different from the registers used on Linux. On Windows,
the registers are (used in this order): rcx, rdx, r8 and r9.

    http://msdn.microsoft.com/en-us/magazine/cc300794.aspx

As a quick reference, the registers used on Linux are: rsi, rdi, and
then rdx, rcx, r8, r9.

The solution I chose was to store that information in the gdbarch_tdep
structure.  This information is then used by the various routines that
deal with parameter passing and value returns, instead of using a hard-
coded list of registers.

The heart of the change in the addition of 3 fields in gdbarch_tdep
in i386-tdep.h.  The rest is just extracting out the OS-dependent
portions and putting them into the OS-dependent implementations of
the new gdbarch_tdep routines.

gdb/ChangeLog:

        * i386-tdep.h (enum amd64_reg_class): New, moved here from
        amd64-tdep.c.
        (struct gdbarch_tdep): Add fields call_dummy_num_integer_regs,
        call_dummy_integer_regs, and classify.
        * amd64-tdep.h (amd64_classify): Add declaration.
        * amd64-tdep.c (amd64_dummy_call_integer_regs): New static constant.
        (amd64_reg_class): Delete, moved to i386-tdep.h.
        (amd64_classify): Make non-static.  Move declaration to amd64-tdep.h.
        Replace call to amd64_classify by call to tdep->classify.
        (amd64_push_arguments): Get the list of registers to use for
        passing integer parameters from the gdbarch tdep structure,
        rather than using a hardcoded one.  Replace calls to amd64_classify
        by calls to tdep->classify.
        (amd64_push_dummy_call): Get the register number used for
        the "hidden" argument from tdep->call_dummy_integer_regs.
        (amd64_init_abi): Initialize tdep->call_dummy_num_integer_regs
        and tdep->call_dummy_integer_regs.  Set tdep->classify.
        * amd64-windows-tdep.c: Add include of gdbtypes.h.
        (amd64_windows_dummy_call_integer_regs): New static global.
        (amd64_windows_classify): New function.
        (amd64_windows_init_abi): Initialize tdep->call_dummy_num_integer_regs
        tdep->call_dummy_integer_regs and tdep->classify.

gdb/testsuite/ChangeLog:

        * gdb.ada/call_pn: New testcase.
---
 gdb/amd64-tdep.c                      |   65 +++++++++++++++++----------------
 gdb/amd64-tdep.h                      |    3 ++
 gdb/amd64-windows-tdep.c              |   55 ++++++++++++++++++++++++++++
 gdb/i386-tdep.h                       |   24 ++++++++++++
 gdb/testsuite/gdb.ada/call_pn.exp     |   53 +++++++++++++++++++++++++++
 gdb/testsuite/gdb.ada/call_pn/foo.adb |   23 ++++++++++++
 gdb/testsuite/gdb.ada/call_pn/pck.adb |   25 +++++++++++++
 gdb/testsuite/gdb.ada/call_pn/pck.ads |   23 ++++++++++++
 8 files changed, 240 insertions(+), 31 deletions(-)
 create mode 100644 gdb/testsuite/gdb.ada/call_pn.exp
 create mode 100644 gdb/testsuite/gdb.ada/call_pn/foo.adb
 create mode 100644 gdb/testsuite/gdb.ada/call_pn/pck.adb
 create mode 100644 gdb/testsuite/gdb.ada/call_pn/pck.ads

diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 9f5d330..f69f3f6 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -72,6 +72,17 @@ static const char *amd64_register_names[] =
 /* Total number of registers.  */
 #define AMD64_NUM_REGS	ARRAY_SIZE (amd64_register_names)
 
+/* The registers used to pass integer arguments during a function call.  */
+static int amd64_dummy_call_integer_regs[] =
+{
+  AMD64_RDI_REGNUM,		/* %rdi */
+  AMD64_RSI_REGNUM,		/* %rsi */
+  AMD64_RDX_REGNUM,		/* %rdx */
+  AMD64_RCX_REGNUM,		/* %rcx */
+  8,				/* %r8 */
+  9				/* %r9 */
+};
+
 /* Return the name of register REGNUM.  */
 
 const char *
@@ -240,20 +251,6 @@ amd64_arch_reg_to_regnum (int reg)
 
 \f
 
-/* Register classes as defined in the psABI.  */
-
-enum amd64_reg_class
-{
-  AMD64_INTEGER,
-  AMD64_SSE,
-  AMD64_SSEUP,
-  AMD64_X87,
-  AMD64_X87UP,
-  AMD64_COMPLEX_X87,
-  AMD64_NO_CLASS,
-  AMD64_MEMORY
-};
-
 /* Return the union class of CLASS1 and CLASS2.  See the psABI for
    details.  */
 
@@ -290,8 +287,6 @@ amd64_merge_classes (enum amd64_reg_class class1, enum amd64_reg_class class2)
   return AMD64_SSE;
 }
 
-static void amd64_classify (struct type *type, enum amd64_reg_class class[2]);
-
 /* Return non-zero if TYPE is a non-POD structure or union type.  */
 
 static int
@@ -413,7 +408,7 @@ amd64_classify_aggregate (struct type *type, enum amd64_reg_class class[2])
 
 /* Classify TYPE, and store the result in CLASS.  */
 
-static void
+void
 amd64_classify (struct type *type, enum amd64_reg_class class[2])
 {
   enum type_code code = TYPE_CODE (type);
@@ -464,6 +459,7 @@ amd64_return_value (struct gdbarch *gdbarch, struct type *func_type,
 		    struct type *type, struct regcache *regcache,
 		    gdb_byte *readbuf, const gdb_byte *writebuf)
 {
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   enum amd64_reg_class class[2];
   int len = TYPE_LENGTH (type);
   static int integer_regnum[] = { AMD64_RAX_REGNUM, AMD64_RDX_REGNUM };
@@ -473,9 +469,10 @@ amd64_return_value (struct gdbarch *gdbarch, struct type *func_type,
   int i;
 
   gdb_assert (!(readbuf && writebuf));
+  gdb_assert (tdep->classify);
 
   /* 1. Classify the return type with the classification algorithm.  */
-  amd64_classify (type, class);
+  tdep->classify (type, class);
 
   /* 2. If the type has class MEMORY, then the caller provides space
      for the return value and passes the address of this storage in
@@ -573,15 +570,10 @@ static CORE_ADDR
 amd64_push_arguments (struct regcache *regcache, int nargs,
 		      struct value **args, CORE_ADDR sp, int struct_return)
 {
-  static int integer_regnum[] =
-  {
-    AMD64_RDI_REGNUM,		/* %rdi */
-    AMD64_RSI_REGNUM,		/* %rsi */
-    AMD64_RDX_REGNUM,		/* %rdx */
-    AMD64_RCX_REGNUM,		/* %rcx */
-    8,				/* %r8 */
-    9				/* %r9 */
-  };
+  struct gdbarch_tdep *tdep = gdbarch_tdep (get_regcache_arch (regcache));
+  int *integer_regs = tdep->call_dummy_integer_regs;
+  int num_integer_regs = tdep->call_dummy_num_integer_regs;
+
   static int sse_regnum[] =
   {
     /* %xmm0 ... %xmm7 */
@@ -598,6 +590,8 @@ amd64_push_arguments (struct regcache *regcache, int nargs,
   int sse_reg = 0;
   int i;
 
+  gdb_assert (tdep->classify);
+
   /* Reserve a register for the "hidden" argument.  */
   if (struct_return)
     integer_reg++;
@@ -612,7 +606,7 @@ amd64_push_arguments (struct regcache *regcache, int nargs,
       int j;
 
       /* Classify argument.  */
-      amd64_classify (type, class);
+      tdep->classify (type, class);
 
       /* Calculate the number of integer and SSE registers needed for
          this argument.  */
@@ -626,7 +620,7 @@ amd64_push_arguments (struct regcache *regcache, int nargs,
 
       /* Check whether enough registers are available, and if the
          argument should be passed in registers at all.  */
-      if (integer_reg + needed_integer_regs > ARRAY_SIZE (integer_regnum)
+      if (integer_reg + needed_integer_regs > num_integer_regs
 	  || sse_reg + needed_sse_regs > ARRAY_SIZE (sse_regnum)
 	  || (needed_integer_regs == 0 && needed_sse_regs == 0))
 	{
@@ -650,7 +644,7 @@ amd64_push_arguments (struct regcache *regcache, int nargs,
 	      switch (class[j])
 		{
 		case AMD64_INTEGER:
-		  regnum = integer_regnum[integer_reg++];
+		  regnum = integer_regs[integer_reg++];
 		  break;
 
 		case AMD64_SSE:
@@ -716,8 +710,13 @@ amd64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   /* Pass "hidden" argument".  */
   if (struct_return)
     {
+      struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+      /* The "hidden" argument is passed throught the first argument
+         register.  */
+      const int arg_regnum = tdep->call_dummy_integer_regs[0];
+
       store_unsigned_integer (buf, 8, byte_order, struct_addr);
-      regcache_cooked_write (regcache, AMD64_RDI_REGNUM, buf);
+      regcache_cooked_write (regcache, arg_regnum, buf);
     }
 
   /* Store return address.  */
@@ -2164,6 +2163,10 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_push_dummy_call (gdbarch, amd64_push_dummy_call);
   set_gdbarch_frame_align (gdbarch, amd64_frame_align);
   set_gdbarch_frame_red_zone_size (gdbarch, 128);
+  tdep->call_dummy_num_integer_regs =
+    ARRAY_SIZE (amd64_dummy_call_integer_regs);
+  tdep->call_dummy_integer_regs = amd64_dummy_call_integer_regs;
+  tdep->classify = amd64_classify;
 
   set_gdbarch_convert_register_p (gdbarch, i387_convert_register_p);
   set_gdbarch_register_to_value (gdbarch, i387_register_to_value);
diff --git a/gdb/amd64-tdep.h b/gdb/amd64-tdep.h
index 3bff5d1..363479c 100644
--- a/gdb/amd64-tdep.h
+++ b/gdb/amd64-tdep.h
@@ -98,6 +98,9 @@ extern void amd64_supply_fxsave (struct regcache *regcache, int regnum,
 
 extern void amd64_collect_fxsave (const struct regcache *regcache, int regnum,
 				  void *fxsave);
+
+void amd64_classify (struct type *type, enum amd64_reg_class class[2]);
+
 \f
 
 /* Variables exported from amd64nbsd-tdep.c.  */
diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c
index bdad9bd..b5a0035 100644
--- a/gdb/amd64-windows-tdep.c
+++ b/gdb/amd64-windows-tdep.c
@@ -20,15 +20,70 @@
 #include "amd64-tdep.h"
 #include "solib.h"
 #include "solib-target.h"
+#include "gdbtypes.h"
+
+/* The registers used to pass integer arguments during a function call.  */
+static int amd64_windows_dummy_call_integer_regs[] =
+{
+  AMD64_RCX_REGNUM,          /* %rcx */
+  AMD64_RDX_REGNUM,          /* %rdx */
+  8,                         /* %r8 */
+  9                          /* %r9 */
+};
+
+/* Implement the "classify" method in the gdbarch_tdep structure
+   for amd64-windows.  */
+
+static void
+amd64_windows_classify (struct type *type, enum amd64_reg_class class[2])
+{
+  switch (TYPE_CODE (type))
+    {
+      case TYPE_CODE_ARRAY:
+	/* Arrays are always passed by memory.	*/
+	class[0] = class[1] = AMD64_MEMORY;
+	break;
+
+      case TYPE_CODE_STRUCT:
+      case TYPE_CODE_UNION:
+        /* Struct/Union types whose size is 1, 2, 4, or 8 bytes
+	   are passed as if they were integers of the same size.
+	   Types of different sizes are passed by memory.  */
+	if (TYPE_LENGTH (type) == 1
+	    || TYPE_LENGTH (type) == 2
+	    || TYPE_LENGTH (type) == 4
+	    || TYPE_LENGTH (type) == 8)
+	  {
+	    class[0] = AMD64_INTEGER;
+	    class[1] = AMD64_NO_CLASS;
+	  }
+	else
+	  class[0] = class[1] = AMD64_MEMORY;
+	break;
+
+      default:
+	/* For all the other types, the conventions are the same as
+	   with the System V ABI.  */
+	amd64_classify (type, class);
+    }
+}
 
 static void
 amd64_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
   amd64_init_abi (info, gdbarch);
 
   /* On Windows, "long"s are only 32bit.  */
   set_gdbarch_long_bit (gdbarch, 32);
 
+  /* Function calls.  */
+  tdep->call_dummy_num_integer_regs =
+    ARRAY_SIZE (amd64_windows_dummy_call_integer_regs);
+  tdep->call_dummy_integer_regs = amd64_windows_dummy_call_integer_regs;
+  tdep->classify = amd64_windows_classify;
+
   set_solib_ops (gdbarch, &solib_target_so_ops);
 }
 
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index 6013bdf..d5b24fa 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -53,6 +53,20 @@ enum struct_return
   reg_struct_return		/* Return "short" structures in registers.  */
 };
 
+/* Register classes as defined in the AMD x86-64 psABI.  */
+
+enum amd64_reg_class
+{
+  AMD64_INTEGER,
+  AMD64_SSE,
+  AMD64_SSEUP,
+  AMD64_X87,
+  AMD64_X87UP,
+  AMD64_COMPLEX_X87,
+  AMD64_NO_CLASS,
+  AMD64_MEMORY
+};
+
 /* i386 architecture specific information.  */
 struct gdbarch_tdep
 {
@@ -62,6 +76,16 @@ struct gdbarch_tdep
   int gregset_num_regs;
   size_t sizeof_gregset;
 
+  /* The general-purpose registers used to pass integers when making
+     function calls.  This only applies to amd64, as all parameters
+     are passed through the stack on x86.  */
+  int call_dummy_num_integer_regs;
+  int *call_dummy_integer_regs;
+
+  /* Classify TYPE according to calling conventions, and store
+     the result in CLASS.  Used on amd64 only.  */
+  void (*classify) (struct type *type, enum amd64_reg_class class[2]);
+
   /* Floating-point registers.  */
   struct regset *fpregset;
   size_t sizeof_fpregset;
diff --git a/gdb/testsuite/gdb.ada/call_pn.exp b/gdb/testsuite/gdb.ada/call_pn.exp
new file mode 100644
index 0000000..5ade3fa
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/call_pn.exp
@@ -0,0 +1,53 @@
+# Copyright 2010 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+load_lib "ada.exp"
+
+set testdir "call_pn"
+set testfile "${testdir}/foo"
+set srcfile ${srcdir}/${subdir}/${testfile}.adb
+set binfile ${objdir}/${subdir}/${testfile}
+
+file mkdir ${objdir}/${subdir}/${testdir}
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug ]] != "" } {
+  return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+set bp_location [gdb_get_line_number "STOP" ${testdir}/foo.adb]
+if ![runto "foo.adb:$bp_location" ] then {
+  perror "Couldn't run ${testfile}"
+  return
+}
+
+# Make sure that last_node_id is set to zero...
+gdb_test "print last_node_id" "= 0" "print last_node_id before calling pn"
+
+# Now, call procedure Pn, which should set Last_Node_Id to the value
+# of the parameter used in the function call.  Verify that we can print
+# the returned value correctly, while we're at it.
+gdb_test "print pn (4321)" "= 4321" "print pn (4321)"
+
+# Make sure that last_node_id now has the correct value...
+gdb_test "print last_node_id" "= 4321" "print last_node_id after calling pn"
+
diff --git a/gdb/testsuite/gdb.ada/call_pn/foo.adb b/gdb/testsuite/gdb.ada/call_pn/foo.adb
new file mode 100644
index 0000000..a71969b
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/call_pn/foo.adb
@@ -0,0 +1,23 @@
+--  Copyright 2010 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+with Pck; use Pck;
+
+procedure Foo is
+   New_Node : Node_Id;
+begin
+   New_Node := Pn (1234);  -- STOP
+end Foo;
+
diff --git a/gdb/testsuite/gdb.ada/call_pn/pck.adb b/gdb/testsuite/gdb.ada/call_pn/pck.adb
new file mode 100644
index 0000000..cc2dfb6
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/call_pn/pck.adb
@@ -0,0 +1,25 @@
+--  Copyright 2010 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package body Pck is
+   Last_Node_Id : Node_Id := Node_Id'First;
+
+   function Pn (N : Node_Id) return Node_Id is
+   begin
+      Last_Node_Id := N;
+      return N;
+   end Pn;
+end Pck;
+
diff --git a/gdb/testsuite/gdb.ada/call_pn/pck.ads b/gdb/testsuite/gdb.ada/call_pn/pck.ads
new file mode 100644
index 0000000..11e21c3
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/call_pn/pck.ads
@@ -0,0 +1,23 @@
+--  Copyright 2010 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package Pck is
+   Node_Low_Bound : constant := 0;
+   Node_High_Bound : constant := 099_999_999;
+   type Node_Id is range Node_Low_Bound .. Node_High_Bound;
+
+   function Pn (N : Node_Id) return Node_Id;
+end Pck;
+
-- 
1.6.3.3

[RFA/commit 3/3] amd64: 32 bytes allocated on stack by caller for integer parameter registers.

[Joel Brobecker]

From: brobecke <brobecke@f8352e7e-cb20-0410-8ce7-b5d9e71c585c>

Yet another difference between Linux and Windows in the calling convention.
When calling a function, the caller is responsible for allocating
a 32byte memory region in the caller's frame; the callee can use
this region to spill the 4 registers used for INTEGER parameter passing.

It showed up when trying to call a function that takes a string as a
parameter, but could probably show up anywhere the INTEGER registers
are being used.  This depends on the actual implementation of the
function being called. In all likeliness, the larger the function,
the more chances the registers used for parameter passing are going
to be needed, and thus need to be spilled on stack.

This patch enhances amd64_push_dummy_call allowing it to also allocated
this spaces on the stack if the ABI mandates it.  Whether it is required
or not is determined by the value of a new tdep field:
integer_param_regs_saved_in_caller_frame.  The default value is zero,
and amd64-windows-tdep.c sets it to 1.

gdb/ChangeLog:

    * i386-tdep.h (struct gdbarch_tdep): Add new field
    integer_param_regs_saved_in_caller_frame.
    * amd64-windows-tdep.c (amd64_windows_init_abi): Set
    tdep->integer_param_regs_saved_in_caller_frame to 1.
    * amd64-tdep.c (amd64_push_dummy_call): Allocate some memory on
    stack if tdep->integer_param_regs_saved_in_caller_frame is set.

I can submit the exact testcase that allowed us to spot this problem.
But I think that gdb.ada/arrayparam.exp already covers this issue.

-- 
Joel

---
 gdb/amd64-tdep.c         |    6 ++++++
 gdb/amd64-windows-tdep.c |    1 +
 gdb/i386-tdep.h          |   18 ++++++++++++++----
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index e9e0809..2b15141 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -731,6 +731,7 @@ amd64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 		       int struct_return, CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   gdb_byte buf[8];
 
   /* Pass arguments.  */
@@ -748,6 +749,11 @@ amd64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
       regcache_cooked_write (regcache, arg_regnum, buf);
     }
 
+  /* Reserve some memory on the stack for the integer-parameter registers,
+     if required by the ABI.  */
+  if (tdep->integer_param_regs_saved_in_caller_frame)
+    sp -= tdep->call_dummy_num_integer_regs * 8;
+
   /* Store return address.  */
   sp -= 8;
   store_unsigned_integer (buf, 8, byte_order, bp_addr);
diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c
index 0ed9368..05c4c1e 100644
--- a/gdb/amd64-windows-tdep.c
+++ b/gdb/amd64-windows-tdep.c
@@ -84,6 +84,7 @@ amd64_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   tdep->call_dummy_integer_regs = amd64_windows_dummy_call_integer_regs;
   tdep->classify = amd64_windows_classify;
   tdep->memory_args_by_pointer = 1;
+  tdep->integer_param_regs_saved_in_caller_frame = 1;
 
   set_solib_ops (gdbarch, &solib_target_so_ops);
 }
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index f79a15d..5915eb9 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -82,12 +82,12 @@ struct gdbarch_tdep
   int call_dummy_num_integer_regs;
   int *call_dummy_integer_regs;
 
-  /* Classify TYPE according to calling conventions, and store
-     the result in CLASS.  Used on amd64 only.  */
+  /* Used on amd64 only.  Classify TYPE according to calling conventions,
+     and store the result in CLASS.  */
   void (*classify) (struct type *type, enum amd64_reg_class class[2]);
 
-  /* Non-zero if the first few MEMORY arguments should be passed by
-     pointer.
+  /* Used on amd64 only.  Non-zero if the first few MEMORY arguments
+     should be passed by pointer.
 
      More precisely, MEMORY arguments are passed through the stack.
      But certain architectures require that their address be passed
@@ -95,6 +95,16 @@ struct gdbarch_tdep
      available for argument passing.  */
   int memory_args_by_pointer;
 
+  /* Used on amd64 only.
+
+     If non-zero, then the callers of a function are expected to reserve
+     some space in the stack just before the area where the PC is saved
+     so that the callee may save the integer-parameter registers there.
+     The amount of space is dependent on the list of registers used for
+     integer parameter passing (see component call_dummy_num_integer_regs
+     above).  */
+  int integer_param_regs_saved_in_caller_frame;
+
   /* Floating-point registers.  */
   struct regset *fpregset;
   size_t sizeof_fpregset;
-- 
1.6.3.3

[amd64-windows] Fix function calls on amd64-windows

[Joel Brobecker]

Hello,

The following patch series enhances the amd64 function-call and
return-value code to handle the Microsoft ABI for amd64-windows.
As it turned out, the principles where relatively similar to
the ABI used on Linux systems, but the details where quite significantly
different (list of integer registers, parameters passed by memory,
etc).

These patches were tested on x86_64-linux using the official testsuite,
as well as on x86_64-windows, but using AdaCore's testsuite.

I'd like to commit in a few days if there are no objection.

Re: [amd64-windows] Fix function calls on amd64-windows

[Christopher Faylor]

On Mon, Jan 25, 2010 at 09:42:09AM +0400, Joel Brobecker wrote:
>Hello,
>
>The following patch series enhances the amd64 function-call and
>return-value code to handle the Microsoft ABI for amd64-windows.
>As it turned out, the principles where relatively similar to
>the ABI used on Linux systems, but the details where quite significantly
>different (list of integer registers, parameters passed by memory,
>etc).
>
>These patches were tested on x86_64-linux using the official testsuite,
>as well as on x86_64-windows, but using AdaCore's testsuite.
>
>I'd like to commit in a few days if there are no objection.

I don't believe that I am the maintainer of the x86_64 files but these
changes look ok to me.

cgf

Re: [amd64-windows] Fix function calls on amd64-windows

[Joel Brobecker]

> I don't believe that I am the maintainer of the x86_64 files but these
> changes look ok to me.

Thanks for the review - they are always appreciated, regardless of
is spending the time to look at the patch...

I have now checked in all the patches. Jan made a suggestion privately,
and I decided that it was simpler for me to do a follow up patch,
rather than redoing the patch. It's really minor and almost stylistic,
but a worthwhile suggestion nonetheless, so I will work on it asap.

-- 
Joel

Re: [amd64-windows] Fix function calls on amd64-windows

[Mark Kettenis]

> From: Joel Brobecker <brobecker@adacore.com>
> Date: Mon, 25 Jan 2010 09:42:09 +0400
> 
> Hello,
> 
> The following patch series enhances the amd64 function-call and
> return-value code to handle the Microsoft ABI for amd64-windows.
> As it turned out, the principles where relatively similar to
> the ABI used on Linux systems, but the details where quite significantly
> different (list of integer registers, parameters passed by memory,
> etc).
> 
> These patches were tested on x86_64-linux using the official testsuite,
> as well as on x86_64-windows, but using AdaCore's testsuite.
> 
> I'd like to commit in a few days if there are no objection.

I'd like to have a chance to review these diffs before you do.  I just
returned home from a trip to .au, so it may take a few days before I
recover from the jetlag and the pile of email that was waiting for me.

Re: [amd64-windows] Fix function calls on amd64-windows

[Joel Brobecker]

> I'd like to have a chance to review these diffs before you do.  I just
> returned home from a trip to .au, so it may take a few days before I
> recover from the jetlag and the pile of email that was waiting for me.

Argh, sorry Mark - the patches went in Friday. I still appreciate
the review, and promise to either fix or revert quickly based on
your comments.  I'd rather not revert unless needed, as it's a bit
of a painful process with CVS...

-- 
Joel