[RFC,PR,gdb/15559] Use thiscall calling convention for class members

Message ID 20200404193534.14775-1-ssbssa@yahoo.de
State New
Headers show
Series
  • [RFC,PR,gdb/15559] Use thiscall calling convention for class members
Related show

Commit Message

Keith Seitz via Gdb-patches April 4, 2020, 7:35 p.m.
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-04  asmwarrior  <asmwarrior@gmail.com>
	    Hannes Domani  <ssbssa@yahoo.de>

	PR gdb/15559
	* i386-tdep.c (i386_push_dummy_call): Use thiscall calling
	convention.
---
Note: This patch is based on the work of asmwarrior (don't know his real name),
taken from PR 15559. I also don't know if he has a copyright assignment.

I just figured out a way to make it work only in i386_push_dummy_call.
---
 gdb/i386-tdep.c | 40 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

-- 
2.26.0

Comments

Tom Tromey April 6, 2020, 8:17 p.m. | #1
>>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:


Hannes> 2020-04-04  asmwarrior  <asmwarrior@gmail.com>
Hannes> 	    Hannes Domani  <ssbssa@yahoo.de>

Hannes> 	PR gdb/15559
Hannes> 	* i386-tdep.c (i386_push_dummy_call): Use thiscall calling
Hannes> 	convention.
Hannes> ---
Hannes> Note: This patch is based on the work of asmwarrior (don't know his real name),
Hannes> taken from PR 15559. I also don't know if he has a copyright assignment.

Hannes> I just figured out a way to make it work only in i386_push_dummy_call.

If you'd rather not be blocked on copyright issues, maybe another way to
do this would be to set the calling convention on member functions when
reading them, in dwarf2/read.c.  We could invent a new DW_CC_* value for
thiscall.  See read_subroutine_type.

Tom
Keith Seitz via Gdb-patches April 6, 2020, 9:07 p.m. | #2
Am Montag, 6. April 2020, 22:17:10 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:

> >>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:

>

> Hannes> 2020-04-04  asmwarrior  <asmwarrior@gmail.com>

> Hannes>         Hannes Domani  <ssbssa@yahoo.de>

>

> Hannes>     PR gdb/15559

> Hannes>     * i386-tdep.c (i386_push_dummy_call): Use thiscall calling

> Hannes>     convention.

>

> Hannes> ---

>

> Hannes> Note: This patch is based on the work of asmwarrior (don't know his real name),

> Hannes> taken from PR 15559. I also don't know if he has a copyright assignment.

>

> Hannes> I just figured out a way to make it work only in i386_push_dummy_call.

>

> If you'd rather not be blocked on copyright issues, maybe another way to

> do this would be to set the calling convention on member functions when

> reading them, in dwarf2/read.c.  We could invent a new DW_CC_* value for

> thiscall.  See read_subroutine_type.


This is the stuff from me:
+  /* For non-static member functions of 32bit Windows programs, the thiscall
+     calling convention is used, so the 'this' pointer is passed in ECX.  */
+  if (gdbarch_osabi (gdbarch) == GDB_OSABI_WINDOWS
+      && gdbarch_ptr_bit (gdbarch) == 32)

+      /* read_subroutine_type sets for non-static member functions the
+         artificial flag of the first parameter ('this' pointer).  */

+          && TYPE_NFIELDS (func_type) > 0
+          && TYPE_FIELD_ARTIFICIAL (func_type, 0)
+          && TYPE_CODE (TYPE_FIELD_TYPE (func_type, 0)) == TYPE_CODE_PTR)
+        {
+          struct compunit_symtab *symtab =
+        find_pc_compunit_symtab (find_function_addr (function, NULL));

So for that we don't need to find another way.

The basics with func_type and i386_windows_thiscall are from asmwarrior,
and these are the things I probably would have done very similar.

But I just found his real name, Yuanhui Zhang, and he already has some commits:
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=search;s=Yuanhui+Zhang;st=author


Hannes

Patch

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 19876c3553..47068da621 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -48,6 +48,8 @@ 
 #include "i387-tdep.h"
 #include "gdbsupport/x86-xstate.h"
 #include "x86-tdep.h"
+#include "infcall.h"
+#include "producer.h"
 
 #include "record.h"
 #include "record-full.h"
@@ -2680,6 +2682,38 @@  i386_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   int i;
   int write_pass;
   int args_space = 0;
+  int i386_windows_thiscall = 0;
+
+  /* For non-static member functions of 32bit Windows programs, the thiscall
+     calling convention is used, so the 'this' pointer is passed in ECX.  */
+  if (gdbarch_osabi (gdbarch) == GDB_OSABI_WINDOWS
+      && gdbarch_ptr_bit (gdbarch) == 32)
+    {
+      struct type *func_type = value_type (function);
+      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));
+
+	  /* read_subroutine_type sets for non-static member functions the
+	     artificial flag of the first parameter ('this' pointer).  */
+	  if (TYPE_CODE (func_type) == TYPE_CODE_METHOD
+	      && TYPE_NFIELDS (func_type) > 0
+	      && TYPE_FIELD_ARTIFICIAL (func_type, 0)
+	      && TYPE_CODE (TYPE_FIELD_TYPE (func_type, 0)) == TYPE_CODE_PTR)
+	    {
+	      struct compunit_symtab *symtab =
+		find_pc_compunit_symtab (find_function_addr (function, NULL));
+	      /* If function is build from gcc 4.7 and later, we should use
+		 thiscall for non static member function.  */
+	      if (symtab != nullptr
+		  && producer_is_gcc_ge_4 (symtab->producer) > 6)
+		i386_windows_thiscall = 1;
+	    }
+	}
+    }
 
   /* BND registers can be in arbitrary values at the moment of the
      inferior call.  This can cause boundary violations that are not
@@ -2709,7 +2743,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]));
 
@@ -2761,6 +2795,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 (i386_windows_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