Fix "thiscall" method calling convention for inferior calls (PR15559)

Message ID 20140326132302.09b6fc35@octopus
State Committed
Headers

Commit Message

Julian Brown March 26, 2014, 1:23 p.m. UTC
  Hi,

This is an adaptation of the patch in PR15559 that handles calling
inferior methods (on win32/mingw32) using the "thiscall" calling
convention -- i.e. with the "this" pointer in the ecx register. Instead
of attempting to detect the GCC version used to compile a particular
function/method to determine the calling convention to use (which didn't
seem to work well for me), I've introduced a new GNU-specific Dwarf code
to represent the calling convention in question.

GCC does not emit the tag at present: I will submit the GCC part of the
fix (to emit the Dwarf tag as appropriate) shortly.

Testing (cross to mingw32 from Linux) gives testsuite result
improvements as follows:

FAIL -> PASS: default/gdb.sum:gdb.cp/bs15503.exp: print (const char *) (s=s.substr(0,4))
FAIL -> PASS: default/gdb.sum:gdb.cp/bs15503.exp: print (const char *) s
FAIL -> PASS: default/gdb.sum:gdb.cp/bs15503.exp: print (const char *) s.substr(0,4)
FAIL -> PASS: default/gdb.sum:gdb.cp/bs15503.exp: print s.length()
FAIL -> PASS: default/gdb.sum:gdb.cp/bs15503.exp: print s[0]
FAIL -> PASS: default/gdb.sum:gdb.cp/bs15503.exp: print s[s.length()-1]
FAIL -> PASS: default/gdb.sum:gdb.cp/classes.exp: call class_param.Aptr_a (&g_A)
FAIL -> PASS: default/gdb.sum:gdb.cp/classes.exp: call class_param.Aptr_a (&g_B)
FAIL -> PASS: default/gdb.sum:gdb.cp/classes.exp: call class_param.Aptr_x (&g_A)
FAIL -> PASS: default/gdb.sum:gdb.cp/classes.exp: call class_param.Aptr_x (&g_B)
FAIL -> PASS: default/gdb.sum:gdb.cp/classes.exp: call class_param.Aref_a (g_A)
FAIL -> PASS: default/gdb.sum:gdb.cp/classes.exp: call class_param.Aref_a (g_B)
FAIL -> PASS: default/gdb.sum:gdb.cp/classes.exp: call class_param.Aref_x (g_A)
FAIL -> PASS: default/gdb.sum:gdb.cp/classes.exp: call class_param.Aref_x (g_B)
FAIL -> PASS: default/gdb.sum:gdb.cp/classes.exp: call class_param.Aval_a (g_A)
FAIL -> PASS: default/gdb.sum:gdb.cp/classes.exp: call class_param.Aval_a (g_B)
FAIL -> PASS: default/gdb.sum:gdb.cp/classes.exp: call class_param.Aval_x (g_A)
FAIL -> PASS: default/gdb.sum:gdb.cp/classes.exp: call class_param.Aval_x (g_B)
FAIL -> PASS: default/gdb.sum:gdb.cp/classes.exp: calling method for small class
FAIL -> PASS: default/gdb.sum:gdb.cp/gdb2384.exp: gdb2384
FAIL -> PASS: default/gdb.sum:gdb.cp/gdb2384.exp: gdb2384 (second)
FAIL -> PASS: default/gdb.sum:gdb.cp/gdb2384.exp: print d1.meth ()
FAIL -> PASS: default/gdb.sum:gdb.cp/gdb2384.exp: print d2.meth()
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print bar(a)
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print bar(b)
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print bar(c)
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print bar(d)
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print call overloaded func 10 args
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print call overloaded func 11 args
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print call overloaded func 2 args
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print call overloaded func 3 args
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print call overloaded func 4 args
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print call overloaded func 5 args
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print call overloaded func 6 args
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print call overloaded func 7 args
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print call overloaded func 8 args
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print call overloaded func 9 args
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print call overloaded func char arg
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print call overloaded func char\* arg
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print call overloaded func double arg
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print call overloaded func float arg
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print call overloaded func int arg
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print call overloaded func int\* arg
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print call overloaded func long arg
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print call overloaded func short arg
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print call overloaded func signed char arg
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print call overloaded func unsigned char arg
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print call overloaded func unsigned int arg
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print call overloaded func unsigned long arg
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print call overloaded func unsigned short arg
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print call overloaded func void arg
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: print foo_instance1.overloadfnarg(23, intintfunc)
FAIL -> PASS: default/gdb.sum:gdb.cp/overload.exp: re-selected 'main' frame after inferior call
FAIL -> PASS: default/gdb.sum:gdb.cp/smartp.exp: p sp4->a
FAIL -> PASS: default/gdb.sum:gdb.cp/smartp.exp: p sp4->b
FAIL -> PASS: default/gdb.sum:gdb.cp/templates.exp: continue to line 770
FAIL -> PASS: default/gdb.sum:gdb.cp/templates.exp: print fint
FAIL -> PASS: default/gdb.sum:gdb.cp/templates.exp: print fvpchar
FAIL -> PASS: default/gdb.sum:gdb.cp/templates.exp: print t5i.value()
FAIL -> PASS: default/gdb.sum:gdb.cp/templates.exp: ptype bazint
FAIL -> PASS: default/gdb.sum:gdb.cp/templates.exp: ptype bazint2
FAIL -> PASS: default/gdb.sum:gdb.cp/templates.exp: ptype bint
FAIL -> PASS: default/gdb.sum:gdb.cp/templates.exp: ptype bint2
FAIL -> PASS: default/gdb.sum:gdb.cp/templates.exp: ptype fchar
FAIL -> PASS: default/gdb.sum:gdb.cp/templates.exp: ptype fint
FAIL -> PASS: default/gdb.sum:gdb.cp/templates.exp: ptype quxint
FAIL -> PASS: default/gdb.sum:gdb.cp/templates.exp: ptype siip
FAIL -> PASS: default/gdb.sum:gdb.cp/userdef.exp: print !one
FAIL -> PASS: default/gdb.sum:gdb.cp/userdef.exp: print ++one
FAIL -> PASS: default/gdb.sum:gdb.cp/userdef.exp: print +one
FAIL -> PASS: default/gdb.sum:gdb.cp/userdef.exp: print --one
FAIL -> PASS: default/gdb.sum:gdb.cp/userdef.exp: print -one
FAIL -> PASS: default/gdb.sum:gdb.cp/userdef.exp: print one % two
FAIL -> PASS: default/gdb.sum:gdb.cp/userdef.exp: print one & two
FAIL -> PASS: default/gdb.sum:gdb.cp/userdef.exp: print one && two
FAIL -> PASS: default/gdb.sum:gdb.cp/userdef.exp: print one * two
FAIL -> PASS: default/gdb.sum:gdb.cp/userdef.exp: print one + two
FAIL -> PASS: default/gdb.sum:gdb.cp/userdef.exp: print one += 7
FAIL -> PASS: default/gdb.sum:gdb.cp/userdef.exp: print one - two
FAIL -> PASS: default/gdb.sum:gdb.cp/userdef.exp: print one ^ two
FAIL -> PASS: default/gdb.sum:gdb.cp/userdef.exp: print one | two
FAIL -> PASS: default/gdb.sum:gdb.cp/userdef.exp: print one++
FAIL -> PASS: default/gdb.sum:gdb.cp/userdef.exp: print one--
FAIL -> PASS: default/gdb.sum:gdb.cp/userdef.exp: print two = one
FAIL -> PASS: default/gdb.sum:gdb.cp/userdef.exp: print ~one
FAIL -> PASS: default/gdb.sum:gdb.cp/virtfunc.exp: next to pAa->f call
FAIL -> PASS: default/gdb.sum:gdb.cp/virtfunc.exp: next to pDe->vg call
FAIL -> PASS: default/gdb.sum:gdb.cp/virtfunc.exp: print pADe->vg()
FAIL -> PASS: default/gdb.sum:gdb.cp/virtfunc.exp: print pBe->vvb()
FAIL -> PASS: default/gdb.sum:gdb.cp/virtfunc.exp: print pDd->vg()
FAIL -> PASS: default/gdb.sum:gdb.cp/virtfunc.exp: print pDe->vg()
FAIL -> PASS: default/gdb.sum:gdb.cp/virtfunc.exp: print pDe->vvb()
FAIL -> PASS: default/gdb.sum:gdb.cp/virtfunc.exp: print pEe->fvb()
FAIL -> PASS: default/gdb.sum:gdb.cp/virtfunc.exp: print pEe->vd()
FAIL -> PASS: default/gdb.sum:gdb.cp/virtfunc.exp: print pEe->vvb()
FAIL -> PASS: default/gdb.sum:gdb.cp/virtfunc.exp: print pVB->vvb()
FAIL -> PASS: default/gdb.sum:gdb.cp/virtfunc.exp: step through thunk into E::vg
Removed FAIL: default/gdb.sum:gdb.cp/templates.exp: ptype fvpchar
Removed FAIL: default/gdb.sum:gdb.cp/virtfunc.exp: print pEe->D::vg()

OK to apply (pending approval of the GCC part of the patch), or any
comments?

Thanks,

Julian

ChangeLog

    include/
    * dwarf2.h (enum dwarf_calling_convention): Add code for
    DW_CC_GNU_thiscall_i386.

    gdb/
    * dwarf2read.c (dwarf2_add_member_fn): Add calling convention
    argument to smash_to_method_type call.
    (quirk_gcc_member_function_pointer): Likewise.
    (read_tag_ptr_to_member_type): Likewise.
    (smash_to_method_type): Add CALLING_CONVENTION argument. Initialise
    func-specific type for method types, and set the calling convention
    as appropriate.
    (init_type): Call INIT_FUNC_SPECIFIC for method types as well as
    function types.
    * gdbtypes.h (struct main_type): Adjust comment for FUNC_STUFF.
    (struct func_type): Adjust comment.
    * stabsread.c (read_type): Add dummy calling-convention argument to
    smash_to_method_type call.
    * i386-tdep.c (utils.h, infcall.h, dwarf2.h): Include files.
    (i386_push_dummy_call): Handle "thiscall" calling convention.
  

Comments

Julian Brown March 26, 2014, 1:26 p.m. UTC | #1
On Wed, 26 Mar 2014 13:23:02 +0000
Julian Brown <julian@codesourcery.com> wrote:

> GCC does not emit the tag at present: I will submit the GCC part of
> the fix (to emit the Dwarf tag as appropriate) shortly.

For completeness, here is the post:

http://gcc.gnu.org/ml/gcc-patches/2014-03/msg01426.html

Julian
  
Tom Tromey May 19, 2014, 2:50 p.m. UTC | #2
>>>>> "Julian" == Julian Brown <julian@codesourcery.com> writes:

Julian> This is an adaptation of the patch in PR15559 that handles calling
Julian> inferior methods (on win32/mingw32) using the "thiscall" calling
Julian> convention -- i.e. with the "this" pointer in the ecx register. Instead
Julian> of attempting to detect the GCC version used to compile a particular
Julian> function/method to determine the calling convention to use (which didn't
Julian> seem to work well for me), I've introduced a new GNU-specific Dwarf code
Julian> to represent the calling convention in question.

Thanks.  I read through the patch and it all looks good to me.

Julian>     include/
Julian>     * dwarf2.h (enum dwarf_calling_convention): Add code for
Julian>     DW_CC_GNU_thiscall_i386.

This has to be approved on the gcc side, but then it is fine to import.
Or you can wait for one of my more or less random mass imports.

Julian>     gdb/
Julian>     * dwarf2read.c (dwarf2_add_member_fn): Add calling convention
Julian>     argument to smash_to_method_type call.
Julian>     (quirk_gcc_member_function_pointer): Likewise.
Julian>     (read_tag_ptr_to_member_type): Likewise.
Julian>     (smash_to_method_type): Add CALLING_CONVENTION argument. Initialise
Julian>     func-specific type for method types, and set the calling convention
Julian>     as appropriate.
Julian>     (init_type): Call INIT_FUNC_SPECIFIC for method types as well as
Julian>     function types.
Julian>     * gdbtypes.h (struct main_type): Adjust comment for FUNC_STUFF.
Julian>     (struct func_type): Adjust comment.
Julian>     * stabsread.c (read_type): Add dummy calling-convention argument to
Julian>     smash_to_method_type call.
Julian>     * i386-tdep.c (utils.h, infcall.h, dwarf2.h): Include files.
Julian>     (i386_push_dummy_call): Handle "thiscall" calling convention.

This is ok contingent on the gcc patch.

Tom
  

Patch

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 705dc2d..8b784ac 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -12578,7 +12578,8 @@  dwarf2_add_member_fn (struct field_info *fip, struct die_info *die,
 			    TYPE_TARGET_TYPE (this_type),
 			    TYPE_FIELDS (this_type),
 			    TYPE_NFIELDS (this_type),
-			    TYPE_VARARGS (this_type));
+			    TYPE_VARARGS (this_type),
+			    TYPE_CALLING_CONVENTION (this_type));
 
       /* Handle static member functions.
          Dwarf2 has no clean way to discern C++ static and non-static
@@ -12782,7 +12783,8 @@  quirk_gcc_member_function_pointer (struct type *type, struct objfile *objfile)
   new_type = alloc_type (objfile);
   smash_to_method_type (new_type, domain_type, TYPE_TARGET_TYPE (pfn_type),
 			TYPE_FIELDS (pfn_type), TYPE_NFIELDS (pfn_type),
-			TYPE_VARARGS (pfn_type));
+			TYPE_VARARGS (pfn_type),
+			TYPE_CALLING_CONVENTION (pfn_type));
   smash_to_methodptr_type (type, new_type);
 }
 
@@ -13943,7 +13945,8 @@  read_tag_ptr_to_member_type (struct die_info *die, struct dwarf2_cu *cu)
 
       smash_to_method_type (new_type, domain, TYPE_TARGET_TYPE (to_type),
 			    TYPE_FIELDS (to_type), TYPE_NFIELDS (to_type),
-			    TYPE_VARARGS (to_type));
+			    TYPE_VARARGS (to_type),
+			    TYPE_CALLING_CONVENTION (to_type));
       type = lookup_methodptr_type (new_type);
     }
   else
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 98cb873..bee1b09 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1174,7 +1174,7 @@  smash_to_methodptr_type (struct type *type, struct type *to_type)
 void
 smash_to_method_type (struct type *type, struct type *domain,
 		      struct type *to_type, struct field *args,
-		      int nargs, int varargs)
+		      int nargs, int varargs, unsigned int calling_convention)
 {
   smash_type (type);
   TYPE_TARGET_TYPE (type) = to_type;
@@ -1185,6 +1185,8 @@  smash_to_method_type (struct type *type, struct type *domain,
     TYPE_VARARGS (type) = 1;
   TYPE_LENGTH (type) = 1;	/* In practice, this is never needed.  */
   TYPE_CODE (type) = TYPE_CODE_METHOD;
+  INIT_FUNC_SPECIFIC (type);
+  TYPE_CALLING_CONVENTION (type) = calling_convention;
 }
 
 /* Return a typename for a struct/union/enum type without "struct ",
@@ -2082,6 +2084,7 @@  init_type (enum type_code code, int length, int flags,
         TYPE_SPECIFIC_FIELD (type) = TYPE_SPECIFIC_FLOATFORMAT;
         break;
       case TYPE_CODE_FUNC:
+      case TYPE_CODE_METHOD:
 	INIT_FUNC_SPECIFIC (type);
         break;
     }
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index c6943ef..a9fd5a2 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -685,7 +685,7 @@  struct main_type
 
     const struct floatformat **floatformat;
 
-    /* * For TYPE_CODE_FUNC types,  */
+    /* * For TYPE_CODE_FUNC and TYPE_CODE_METHOD types,  */
 
     struct func_type *func_stuff;
   } type_specific;
@@ -982,7 +982,7 @@  struct gnat_aux_type
     struct type* descriptive_type;
   };
 
-/* * For TYPE_CODE_FUNC types.  */
+/* * For TYPE_CODE_FUNC and TYPE_CODE_METHOD types.  */
 
 struct func_type
   {
@@ -1598,7 +1598,8 @@  extern struct type *lookup_methodptr_type (struct type *);
 
 extern void smash_to_method_type (struct type *type, struct type *domain,
 				  struct type *to_type, struct field *args,
-				  int nargs, int varargs);
+				  int nargs, int varargs,
+				  unsigned int calling_convention);
 
 extern void smash_to_memberptr_type (struct type *, struct type *,
 				     struct type *);
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index be40b20..1025e07 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -45,6 +45,9 @@ 
 #include "remote.h"
 #include "exceptions.h"
 #include "gdb_assert.h"
+#include "utils.h"
+#include "infcall.h"
+#include "dwarf2.h"
 #include <string.h>
 
 #include "i386-tdep.h"
@@ -2529,6 +2532,21 @@  i386_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   int i;
   int write_pass;
   int args_space = 0;
+  struct type *func_type = value_type (function);
+  int i386_windows_thiscall = 0;
+
+  if (func_type)
+    {
+      func_type = check_typedef (func_type);
+      
+      if (TYPE_CODE (func_type) == TYPE_CODE_PTR)
+        func_type = check_typedef (TYPE_TARGET_TYPE (func_type));
+      
+      if ((TYPE_CODE (func_type) == TYPE_CODE_METHOD
+	   || TYPE_CODE (func_type) == TYPE_CODE_FUNC)
+	  && TYPE_CALLING_CONVENTION (func_type) == DW_CC_GNU_thiscall_i386)
+	i386_windows_thiscall = 1;
+    }
 
   /* Determine the total space required for arguments and struct
      return address in a first pass (allowing for 16-byte-aligned
@@ -2551,7 +2569,7 @@  i386_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 	    args_space += 4;
 	}
 
-      for (i = 0; i < nargs; i++)
+      for (i = i386_windows_thiscall; i < nargs; i++)
 	{
 	  int len = TYPE_LENGTH (value_enclosing_type (args[i]));
 
@@ -2603,6 +2621,13 @@  i386_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   /* ...and fake a frame pointer.  */
   regcache_cooked_write (regcache, I386_EBP_REGNUM, buf);
 
+  if (i386_windows_thiscall)
+    {
+      /* ARGS[0] refers to the last argument which is the this pointer.  */
+      regcache_cooked_write (regcache, I386_ECX_REGNUM,
+			     value_contents_all (args[0]));
+    }
+
   /* 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
diff --git a/gdb/stabsread.c b/gdb/stabsread.c
index b40cf78..ff4ab10 100644
--- a/gdb/stabsread.c
+++ b/gdb/stabsread.c
@@ -1959,7 +1959,7 @@  again:
 	    return error_type (pp, objfile);
 	  type = dbx_alloc_type (typenums, objfile);
 	  smash_to_method_type (type, domain, return_type, args,
-				nargs, varargs);
+				nargs, varargs, 0);
 	}
       break;
 
diff --git a/include/dwarf2.h b/include/dwarf2.h
index 120e2c1..eb9ee5e 100644
--- a/include/dwarf2.h
+++ b/include/dwarf2.h
@@ -182,6 +182,7 @@  enum dwarf_calling_convention
 
     DW_CC_GNU_renesas_sh = 0x40,
     DW_CC_GNU_borland_fastcall_i386 = 0x41,
+    DW_CC_GNU_thiscall_i386 = 0x42,
 
     /* This DW_CC_ value is not currently generated by any toolchain.  It is
        used internally to GDB to indicate OpenCL C functions that have been