[PATCH v3][PR gdb/15559] Use thiscall calling convention for class members

[PATCH v3][PR gdb/15559] Use thiscall calling convention for class members

[Hannes Domani]

Non-static member functions for Windows 32bit programs need the thiscall
calling convention, so the 'this' pointer needs to be passed in ECX.

gdb/ChangeLog:

2020-04-29  Hannes Domani  <ssbssa@yahoo.de>

	PR gdb/15559
	* i386-tdep.c (i386_push_dummy_call): Call
	i386_thiscall_push_dummy_call.
	(i386_thiscall_push_dummy_call): New function.
	* i386-tdep.h (i386_thiscall_push_dummy_call): Declare.
	* i386-windows-tdep.c (i386_windows_push_dummy_call): New function.
	(i386_windows_init_abi): Call set_gdbarch_push_dummy_call.
---
v2:
- Now only code by me is left.

v3:
- Refactored the OSABI_WINDOWS stuff into a i386-windows-tdep.c
  function, but I'm not 100% sure that I did it how Simon meant it.
---
 gdb/i386-tdep.c         | 35 ++++++++++++++++++++++++++++-------
 gdb/i386-tdep.h         | 13 +++++++++++++
 gdb/i386-windows-tdep.c | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 7 deletions(-)

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 84edb3649e..fc63635317 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -2668,12 +2668,15 @@ i386_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp, CORE_ADDR funaddr,
   return sp - 16;
 }
 
-static CORE_ADDR
-i386_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
-		      struct regcache *regcache, CORE_ADDR bp_addr, int nargs,
-		      struct value **args, CORE_ADDR sp,
-		      function_call_return_method return_method,
-		      CORE_ADDR struct_addr)
+/* The "push_dummy_call" gdbarch method, optionally with the thiscall
+   calling convention.  */
+
+CORE_ADDR
+i386_thiscall_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
+			       struct regcache *regcache, CORE_ADDR bp_addr,
+			       int nargs, struct value **args, CORE_ADDR sp,
+			       function_call_return_method return_method,
+			       CORE_ADDR struct_addr, bool thiscall)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   gdb_byte buf[4];
@@ -2709,7 +2712,7 @@ i386_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 	    args_space += 4;
 	}
 
-      for (i = 0; i < nargs; i++)
+      for (i = thiscall ? 1 : 0; i < nargs; i++)
 	{
 	  int len = TYPE_LENGTH (value_enclosing_type (args[i]));
 
@@ -2761,6 +2764,10 @@ i386_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   /* ...and fake a frame pointer.  */
   regcache->cooked_write (I386_EBP_REGNUM, buf);
 
+  /* The 'this' pointer needs to be in ECX.  */
+  if (thiscall)
+    regcache->cooked_write (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
@@ -2773,6 +2780,20 @@ i386_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   return sp + 8;
 }
 
+/* Implement the "push_dummy_call" gdbarch method.  */
+
+static CORE_ADDR
+i386_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
+		      struct regcache *regcache, CORE_ADDR bp_addr, int nargs,
+		      struct value **args, CORE_ADDR sp,
+		      function_call_return_method return_method,
+		      CORE_ADDR struct_addr)
+{
+  return i386_thiscall_push_dummy_call (gdbarch, function, regcache, bp_addr,
+					nargs, args, sp, return_method,
+					struct_addr, false);
+}
+
 /* 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).  */
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index fa29e316a1..79b3b1f942 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -399,6 +399,19 @@ extern CORE_ADDR i386_pe_skip_trampoline_code (struct frame_info *frame,
 extern CORE_ADDR i386_skip_main_prologue (struct gdbarch *gdbarch,
 					  CORE_ADDR pc);
 
+/* The "push_dummy_call" gdbarch method, optionally with the thiscall
+   calling convention.  */
+extern CORE_ADDR i386_thiscall_push_dummy_call (struct gdbarch *gdbarch,
+						struct value *function,
+						struct regcache *regcache,
+						CORE_ADDR bp_addr,
+						int nargs, struct value **args,
+						CORE_ADDR sp,
+						function_call_return_method
+						return_method,
+						CORE_ADDR struct_addr,
+						bool thiscall);
+
 /* Return whether the THIS_FRAME corresponds to a sigtramp routine.  */
 extern int i386_sigtramp_p (struct frame_info *this_frame);
 
diff --git a/gdb/i386-windows-tdep.c b/gdb/i386-windows-tdep.c
index 3a07c862f2..4824a9e552 100644
--- a/gdb/i386-windows-tdep.c
+++ b/gdb/i386-windows-tdep.c
@@ -200,6 +200,36 @@ i386_windows_auto_wide_charset (void)
   return "UTF-16";
 }
 
+/* Implement the "push_dummy_call" gdbarch method.  */
+
+static CORE_ADDR
+i386_windows_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
+			      struct regcache *regcache, CORE_ADDR bp_addr,
+			      int nargs, struct value **args, CORE_ADDR sp,
+			      function_call_return_method return_method,
+			      CORE_ADDR struct_addr)
+{
+  /* For non-static member functions of 32bit Windows programs, the thiscall
+     calling convention is used, so the 'this' pointer is passed in ECX.  */
+  bool thiscall = false;
+
+  struct type *type = check_typedef (value_type (function));
+  if (TYPE_CODE (type) == TYPE_CODE_PTR)
+    type = check_typedef (TYPE_TARGET_TYPE (type));
+
+  /* read_subroutine_type sets for non-static member functions the
+     artificial flag of the first parameter ('this' pointer).  */
+  if (TYPE_CODE (type) == TYPE_CODE_METHOD
+      && TYPE_NFIELDS (type) > 0
+      && TYPE_FIELD_ARTIFICIAL (type, 0)
+      && TYPE_CODE (TYPE_FIELD_TYPE (type, 0)) == TYPE_CODE_PTR)
+    thiscall = 1;
+
+  return i386_thiscall_push_dummy_call (gdbarch, function, regcache, bp_addr,
+					nargs, args, sp, return_method,
+					struct_addr, thiscall);
+}
+
 /* Common parts for gdbarch initialization for Windows and Cygwin on i386.  */
 
 static void
@@ -234,6 +264,8 @@ i386_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
   i386_windows_init_abi_common (info, gdbarch);
   windows_init_abi (info, gdbarch);
+
+  set_gdbarch_push_dummy_call (gdbarch, i386_windows_push_dummy_call);
 }
 
 /* gdbarch initialization for Cygwin on i386.  */
-- 
2.26.2

Re: [PATCH v3][PR gdb/15559] Use thiscall calling convention for class members

[Simon Marchi]

> @@ -234,6 +264,8 @@ i386_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  {
>    i386_windows_init_abi_common (info, gdbarch);
>    windows_init_abi (info, gdbarch);
> +
> +  set_gdbarch_push_dummy_call (gdbarch, i386_windows_push_dummy_call);
>  }

Just to be sure, this call convention does not apply to Cygwin programs?

Simon

Re: [PATCH v3][PR gdb/15559] Use thiscall calling convention for class members

[Hannes Domani]

 Am Mittwoch, 29. April 2020, 18:45:35 MESZ hat Simon Marchi <simark@simark.ca> Folgendes geschrieben:

> > @@ -234,6 +264,8 @@ i386_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> >  {
> >    i386_windows_init_abi_common (info, gdbarch);
> >    windows_init_abi (info, gdbarch);
> > +
> > +  set_gdbarch_push_dummy_call (gdbarch, i386_windows_push_dummy_call);
> >  }
>
> Just to be sure, this call convention does not apply to Cygwin programs?

I don't really know, but you're right, I should find out.
I will install & test with a cygwin compiler tomorrow.


Hannes

Re: [PATCH v3][PR gdb/15559] Use thiscall calling convention for class members

[Hannes Domani]

 Am Mittwoch, 29. April 2020, 18:49:32 MESZ hat Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> Folgendes geschrieben:

> Am Mittwoch, 29. April 2020, 18:45:35 MESZ hat Simon Marchi <simark@simark.ca> Folgendes geschrieben:
>
> > > @@ -234,6 +264,8 @@ i386_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> > >  {
> > >    i386_windows_init_abi_common (info, gdbarch);
> > >    windows_init_abi (info, gdbarch);
> > > +
> > > +  set_gdbarch_push_dummy_call (gdbarch, i386_windows_push_dummy_call);
> > >  }
> >
> > Just to be sure, this call convention does not apply to Cygwin programs?
>
>
> I don't really know, but you're right, I should find out.
> I will install & test with a cygwin compiler tomorrow.

Installing cygwin went faster than I expected.

And yes, cygwin does NOT use thiscall calling convention.
That just makes me more glad that now there exists a distinction between
cygwin & non-cygwin programs in gdb.


Hannes

Re: [PATCH v3][PR gdb/15559] Use thiscall calling convention for class members

[Simon Marchi]

On 2020-04-29 1:19 p.m., Hannes Domani via Gdb-patches wrote:
>  Am Mittwoch, 29. April 2020, 18:49:32 MESZ hat Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> Folgendes geschrieben:
> 
>> Am Mittwoch, 29. April 2020, 18:45:35 MESZ hat Simon Marchi <simark@simark.ca> Folgendes geschrieben:
>>
>>>> @@ -234,6 +264,8 @@ i386_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>>>>   {
>>>>     i386_windows_init_abi_common (info, gdbarch);
>>>>     windows_init_abi (info, gdbarch);
>>>> +
>>>> +  set_gdbarch_push_dummy_call (gdbarch, i386_windows_push_dummy_call);
>>>>   }
>>>
>>> Just to be sure, this call convention does not apply to Cygwin programs?
>>
>>
>> I don't really know, but you're right, I should find out.
>> I will install & test with a cygwin compiler tomorrow.
> 
> Installing cygwin went faster than I expected.
> 
> And yes, cygwin does NOT use thiscall calling convention.
> That just makes me more glad that now there exists a distinction between
> cygwin & non-cygwin programs in gdb.

Ok, the patch LGTM then.

Thanks,

Simon

Re: [PATCH v3][PR gdb/15559] Use thiscall calling convention for class members

[Hannes Domani]

 Am Mittwoch, 29. April 2020, 20:27:02 MESZ hat Simon Marchi <simark@simark.ca> Folgendes geschrieben:

> On 2020-04-29 1:19 p.m., Hannes Domani via Gdb-patches wrote:
> >  Am Mittwoch, 29. April 2020, 18:49:32 MESZ hat Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> Folgendes geschrieben:
> >
> >> Am Mittwoch, 29. April 2020, 18:45:35 MESZ hat Simon Marchi <simark@simark.ca> Folgendes geschrieben:
> >>
> >>>> @@ -234,6 +264,8 @@ i386_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> >>>>   {
> >>>>     i386_windows_init_abi_common (info, gdbarch);
> >>>>     windows_init_abi (info, gdbarch);
> >>>> +
> >>>> +  set_gdbarch_push_dummy_call (gdbarch, i386_windows_push_dummy_call);
> >>>>   }
> >>>
> >>> Just to be sure, this call convention does not apply to Cygwin programs?
> >>
> >>
> >> I don't really know, but you're right, I should find out.
> >> I will install & test with a cygwin compiler tomorrow.
> >
> > Installing cygwin went faster than I expected.
> >
> > And yes, cygwin does NOT use thiscall calling convention.
> > That just makes me more glad that now there exists a distinction between
> > cygwin & non-cygwin programs in gdb.
>
> Ok, the patch LGTM then.

Pushed, thanks.


Hannes