Summary: | Method call and calling convention | ||
---|---|---|---|
Product: | gdb | Reporter: | Kifa <kifathegreat> |
Component: | c++ | Assignee: | Not yet assigned to anyone <unassigned> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | asmwarrior, i.nixman, julianbrown99, ssbssa, ssbssa, tromey, xunxun1982 |
Priority: | P2 | ||
Version: | 7.6 | ||
Target Milestone: | 10.1 | ||
Host: | Target: | ||
Build: | Last reconfirmed: | ||
Attachments: |
handling thiscall calling convention for Windows GCC 7.4.0+ on x86
generate correct calling convention according to GCC version detection use thiscall calling convention for class members use thiscall calling convention for class members (try 2) handling thiscall calling convention for Windows(V3) new patch against gdb git |
Description
Kifa
2013-06-01 19:48:18 UTC
It is very simple to reproduce this bug: Test code: (build under GCC 4.7.x or GCC 4.8.x) #include <string> #include <vector> int fff() { return 3; } int main() { std::vector<std::string> v; v.push_back("a"); v.push_back("b"); std::string abc = v[0]; abc.c_str(); abc.size(); int i = v.size(); i++; i = fff(); return 0; } When debugging, you set a bp in the last line: p v.size() will have such report: GDB has restored the context to what it was before the call. To change this behavior use "set unwindonsignal off". Evaluation of the expression containing the function (std::vector<std::string, std::allocator<std::string> >::size() const) will be abandoned. Program received signal SIGSEGV, Segmentation fault. p fff() works OK. This is because std::string::size() is a __thiscall call, but fff() is a __cdecl calling convention. There is a related discussion about calling stdcall style function under inferior call, see: https://sourceware.org/ml/gdb/2012-10/msg00061.html, though stdcall and __thiscall are different. From what Jon Y said [1]: QUOTE START: The previous call convention is __cdecl[1], arguments pushed to stack right to left, caller unwinds the stack. This convention is used by Linux SysV ABI and normal C code on Linux and Windows. __thiscall[2] is a C++ variant of __stdcall[3] where stack is unwound by callee, however the "this" C++ context is put on the ECX register rather than as a normal first call argument. GCC unfortunately does not mangle __cdecl vs __thiscall call convention into the object symbol tables, this meant you can still link your old code, but it would fall apart during runtime. QUOTE END My question is: how can GDB distinguish between normal functions and class member functions? Is it possible to distinguish them from debug symbol tables? [1] http://sourceforge.net/mailarchive/message.php?msg_id=30019808 Corinna Vinschen - [RFC] Supporting alternative ABIs - https://sourceware.org/ml/gdb/2003-08/msg00252.html, by reading this discussion, I see that some arch already has two calling conversion support. I see that in the file: gdb\sh-tdep.c, the user can specify the calling convention from the command line, see the source: ------------------------------------------- void _initialize_sh_tdep (void) { gdbarch_register (bfd_arch_sh, sh_gdbarch_init, NULL); add_prefix_cmd ("sh", no_class, set_sh_command, "SH specific commands.", &setshcmdlist, "set sh ", 0, &setlist); add_prefix_cmd ("sh", no_class, show_sh_command, "SH specific commands.", &showshcmdlist, "show sh ", 0, &showlist); add_setshow_enum_cmd ("calling-convention", class_vars, sh_cc_enum, &sh_active_calling_convention, _("Set calling convention used when calling target " "functions from GDB."), _("Show calling convention used when calling target " "functions from GDB."), _("gcc - Use GCC calling convention (default).\n" "renesas - Enforce Renesas calling convention."), NULL, NULL, &setshcmdlist, &showshcmdlist); } ------------------------------------------- The variable sh_active_calling_convention is 0 (default) for sh_cc_gcc calling convention, and 1 for sh_cc_renesas, there is a function: static int sh_is_renesas_calling_convention (struct type *func_type) to determine the function calling convention by some checks: TYPE_CALLING_CONVENTION (func_type) == DW_CC_GNU_renesas_sh.... Finally, the different calling conversion will change the behavior of sh_push_dummy_call_*, which is the way GDB make a dummy call. Under MinGW GCC(maybe not limit in Windows system), it looks like the debug information is not enough. E.g. the sample code build from MinGW GCC 4.6.x or 4.8.1 ------------------------------------------- int f_c_call(int a) { return a; } int __attribute__((thiscall)) f_this_call (int b) { return b; } class Test { public: Test(int value) : _value(value) {} int m_f_this_call() const __attribute__((thiscall)) { return _value; } int m_f_c_call() const __attribute__((cdecl)) { return _value; } private: int _value; }; int main() { Test test1(123); Test test2(456); int value1 = test1.m_f_c_call(); int value2 = test2.m_f_this_call(); value1 = f_c_call(1); value2 = f_this_call(2); int value3 = value1+value2; return 0; } ------------------------------------------- Run this inferior under GDB, then type: maintenance print type f_this_call result: type node 0x1b37c58 name '<NULL>' (0x0) tagname '<NULL>' (0x0) code 0x7 (TYPE_CODE_FUNC) length 1 objfile 0x1b29990 target_type 0x1b333c8 type node 0x1b333c8 name 'int' (0x1b34b35) tagname '<NULL>' (0x0) code 0x8 (TYPE_CODE_INT) length 4 objfile 0x1b29990 target_type 0x0 pointer_type 0x0 reference_type 0x0 type_chain 0x1b333c8 instance_flags 0x0 flags nfields 0 0x0 vptr_basetype 0x0 vptr_fieldno -1 pointer_type 0x0 reference_type 0x0 type_chain 0x1b37c58 instance_flags 0x0 flags TYPE_FLAG_PROTOTYPED nfields 1 0x1b37cb0 [0] bitpos 0 bitsize 0 type 0x1b333c8 name '<NULL>' (0x0) type node 0x1b333c8 name 'int' (0x1b34b35) tagname '<NULL>' (0x0) code 0x8 (TYPE_CODE_INT) length 4 objfile 0x1b29990 target_type 0x0 pointer_type 0x0 reference_type 0x0 type_chain 0x1b333c8 instance_flags 0x0 flags nfields 0 0x0 vptr_basetype 0x0 vptr_fieldno -1 vptr_basetype 0x0 vptr_fieldno -1 calling_convention 1 Type maintenance print type f_c_call, maintenance print type f_c_call, maintenance print type Test::m_f_c_call, maintenance print type Test::m_f_this_call, all gives the same result of "calling_convention 1". So, I think a work around is do the same thing like in "sh-tdep.c", that is: we see the inferior is build from GCC 4.7 or later, we need to check the calling_convention by some code snippet like: TYPE_CODE (func_type) == TYPE_CODE_FUNC and this is a member function then we should modify the i386_push_dummy_call like function in i386-tdep.c, oh, do we need a i386-windows-tdeps.c? I see there are files named amd64-windows-tdeps.c, i386-cygwin-tdeps.c, but I don't see i386-windows-tdeps.c, the reason may be that i386-tdep.c is a good place to put the change? I'm sorry I can't go further to implement this, but I hope some guys can help to improve this, thanks. Yuanhui Zhang OK, the tiny patch can fix this issue, note this is only for GCC under Windows, the GCC version should be 4.7.x or later, comments are welcome. See below: gdb/i386-tdep.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c index b159b49..db160ba 100644 --- a/gdb/i386-tdep.c +++ b/gdb/i386-tdep.c @@ -2408,6 +2408,20 @@ 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) + { + if( (TYPE_CODE (func_type) == TYPE_CODE_METHOD) && (nargs > 0)) + { + /* a.f(5,6); + args[0] = this pointer; + args[1] = 5; + args[2] = 6; */ + i386_windows_thiscall = 1; + } + } /* Determine the total space required for arguments and struct return address in a first pass (allowing for 16-byte-aligned @@ -2430,7 +2444,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])); @@ -2482,6 +2496,12 @@ 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] refer 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 Created attachment 7225 [details]
handling thiscall calling convention for Windows GCC 7.4.0+ on x86
Use thiscall calling convention for x86 mingw gcc 4.7.0 and above in C++ class method, borrow some code from sh-tdep.c to handling function typedef and pointers for thiscall calling convention, also there is a hack (global variable) to tell i386_push_dummy_call() function that whether it is a static member function call, it is better to pass this variable in function arguments, but this will make a lot of other changes.
Created attachment 7227 [details]
generate correct calling convention according to GCC version detection
This is the improved version of the previous patch v2, in this version, the gcc version is detected so that correctly calling convention can be generated. The bad thing is that I still use global variables to translate information, I need to find a better and clean way.
This patches will be acepted in GDB? Hi, nixman, I think it won't, if you look at my patch, I use some global variables to translate the information from one module to another module. Currently, both the Linux and Windows 32 bit i386 inferior call code was in a uniformed method (all in i386-tdep.c). If we need to distinguish them, we may need to create some new file like: i386-windows-tdeps.c, which handle the special Windows 32 bit inferior calls. (This is something similar to what amd64-windows-tdeps.c does) Created attachment 7438 [details]
use thiscall calling convention for class members
This uses the debug information to detect if thiscall calling convention should be used.
It's kinda ugly since I used the DWARF codes directly.
And I couldn't figure out if there is a way to get the first function-argument (instead of the 'this' symbol), since it's possible to use thiscall for non-member functions as well.
Hi, Domani Hannes, thanks for the patch. I just tested it, but it failed on testing the sample code in comment 1 when I type "p v.size()". The BT is below: [debug]> bt 30 [debug]#0 0x005a0ed9 in dwarf2_per_cu_objfile (per_cu=0xbaadf00d) at f:\build_gdb\binutils-gdb\gdb\dwarf2read.c:20999 [debug]#1 0x005b21a0 in dwarf2_find_location_expression (baton=0x302e7e8, locexpr_length=0x2a4f234, pc=0) at f:\build_gdb\binutils-gdb\gdb\dwarf2loc.c:214 [debug]#2 0x005b25b8 in loclist_find_frame_base_location (framefunc=0x302e7b8, pc=0, start=0x2a4f238, length=0x2a4f234) at f:\build_gdb\binutils-gdb\gdb\dwarf2loc.c:410 [debug]#3 0x00408d0f in i386_push_dummy_call (gdbarch=0x2f76480, function=0x4daf590, regcache=0x4df70c0, bp_addr=2293391, nargs=1, args=0x2a4f464, sp=2293376, struct_return=0, struct_addr=0) at f:\build_gdb\binutils-gdb\gdb\i386-tdep.c:2561 [debug]#4 0x0053cdc6 in gdbarch_push_dummy_call (gdbarch=0x2f76480, function=0x4daf590, regcache=0x4df70c0, bp_addr=2293391, nargs=1, args=0x2a4f464, sp=2293376, struct_return=0, struct_addr=0) at f:\build_gdb\binutils-gdb\gdb\gdbarch.c:2117 [debug]#5 0x00510299 in call_function_by_hand (function=0x4daf590, nargs=1, args=0x2a4f464) at f:\build_gdb\binutils-gdb\gdb\infcall.c:752 [debug]#6 0x004d9658 in evaluate_subexp_standard (expect_type=0x0, exp=0x4debb20, pos=0x2a4f84c, noside=EVAL_NORMAL) at f:\build_gdb\binutils-gdb\gdb\eval.c:1764 [debug]#7 0x005d7040 in evaluate_subexp_c (expect_type=0x0, exp=0x4debb20, pos=0x2a4f84c, noside=EVAL_NORMAL) at f:\build_gdb\binutils-gdb\gdb\c-lang.c:720 [debug]#8 0x004d5a36 in evaluate_subexp (expect_type=0x0, exp=0x4debb20, pos=0x2a4f84c, noside=EVAL_NORMAL) at f:\build_gdb\binutils-gdb\gdb\eval.c:70 [debug]#9 0x004d5bb5 in evaluate_expression (exp=0x4debb20) at f:\build_gdb\binutils-gdb\gdb\eval.c:145 [debug]#10 0x004eec79 in print_command_1 (exp=0x2a4f9f2 "v.size()", voidprint=1) at f:\build_gdb\binutils-gdb\gdb\printcmd.c:977 [debug]#11 0x004eed84 in print_command (exp=0x2a4f9f2 "v.size()", from_tty=1) at f:\build_gdb\binutils-gdb\gdb\printcmd.c:1009 [debug]#12 0x0044d264 in do_cfunc (c=0x2f4f850, args=0x2a4f9f2 "v.size()", from_tty=1) at f:\build_gdb\binutils-gdb\gdb\cli\cli-decode.c:107 [debug]#13 0x0044fb54 in cmd_func (cmd=0x2f4f850, args=0x2a4f9f2 "v.size()", from_tty=1) at f:\build_gdb\binutils-gdb\gdb\cli\cli-decode.c:1886 [debug]#14 0x00611954 in execute_command (p=0x2a4f9f9 ")", from_tty=1) at f:\build_gdb\binutils-gdb\gdb\top.c:458 [debug]#15 0x00457658 in safe_execute_command (command_uiout=0x2f72b08, command=0x2a4f9f0 "p v.size()", from_tty=1) at f:\build_gdb\binutils-gdb\gdb\cli\cli-interp.c:124 [debug]#16 0x004575d3 in cli_interpreter_exec (data=0x0, command_str=0x4c40758 "p v.size()") at f:\build_gdb\binutils-gdb\gdb\cli\cli-interp.c:107 [debug]#17 0x0052e452 in interp_exec (interp=0x2f72ba8, command_str=0x4c40758 "p v.size()") at f:\build_gdb\binutils-gdb\gdb\interps.c:356 [debug]#18 0x0045d878 in mi_cmd_interpreter_exec (command=0x78663d <procs+2845> "-interpreter-exec", argv=0x2a4fad4, argc=2) at f:\build_gdb\binutils-gdb\gdb\mi\mi-interp.c:246 [debug]#19 0x004623c3 in captured_mi_execute_command (uiout=0x2f79a38, context=0x3014fa0) at f:\build_gdb\binutils-gdb\gdb\mi\mi-main.c:1985 [debug]#20 0x00462698 in mi_execute_command (cmd=0x4c40810 "p v.size()", from_tty=1) at f:\build_gdb\binutils-gdb\gdb\mi\mi-main.c:2074 [debug]#21 0x0045d92a in mi_execute_command_wrapper (cmd=0x4c40810 "p v.size()") at f:\build_gdb\binutils-gdb\gdb\mi\mi-interp.c:291 [debug]#22 0x0045d93d in mi_execute_command_input_handler (cmd=0x4c40810 "p v.size()") at f:\build_gdb\binutils-gdb\gdb\mi\mi-interp.c:299 [debug]#23 0x00536bcd in gdb_readline2 (client_data=0x0) at f:\build_gdb\binutils-gdb\gdb\event-top.c:713 [debug]#24 0x005364ad in stdin_event_handler (error=0, client_data=0x0) at f:\build_gdb\binutils-gdb\gdb\event-top.c:375 [debug]#25 0x005356fa in handle_file_event (data=...) at f:\build_gdb\binutils-gdb\gdb\event-loop.c:768 [debug]#26 0x00534e48 in process_event () at f:\build_gdb\binutils-gdb\gdb\event-loop.c:342 [debug]#27 0x00534f0d in gdb_do_one_event () at f:\build_gdb\binutils-gdb\gdb\event-loop.c:406 [debug]#28 0x00534f5e in start_event_loop () at f:\build_gdb\binutils-gdb\gdb\event-loop.c:431 [debug]#29 0x0045d998 in mi_command_loop (data=0x2f79578) at f:\build_gdb\binutils-gdb\gdb\mi\mi-interp.c:316 [debug](More stack frames follow...) In the code: struct objfile *objfile = per_cu->objfile; I see that per_cu can't be accessed, see the log: [debug]> whatis per_cu [debug]type = struct dwarf2_per_cu_data * [debug]>>>>>>cb_gdb: [debug]> output per_cu [debug](struct dwarf2_per_cu_data *) 0xbaadf00d>>>>>>cb_gdb: [debug]> output *per_cu [debug]Cannot access memory at address 0xbaadf00d [debug]>>>>>>cb_gdb: Yuanhui Zhang Created attachment 7443 [details]
use thiscall calling convention for class members (try 2)
This is a (mostly) similar approach, but now I use tracepoint_var_ref(), which translates the debug information into an agent expression.
Hi, Domani Hannes, thank you for the hard work, with your version 2 patch, now "p v.size()" works OK in comment 1, but "p abc.c_str()" still let GDB crash. I don't understand much about your patch, my knowledge on GDB is much less than yours. PS: my patch attachment 7227 [details] works OK on those tests in comment 1. Hello asmwarrior Yes, looks like my strategy doesn't work in every situation. find_pc_function() can fail (maybe because of some missing debug information?), then it just stops looking. A minimal_symbol would still be available, but it's not really useful for this situation. I looked at your code again as well. find_pc_symtab(find_function_addr(function,NULL)) would work as well, so we don't need that global i386_windows_build_gt_gcc46. But I found no way to locally find out if it's a static function. There is function->type->main_type->flag_static, but it's always 0 (and as far as I can tell not used). (In reply to Domani Hannes from comment #15) > Hello asmwarrior > > Yes, looks like my strategy doesn't work in every situation. > find_pc_function() can fail (maybe because of some missing debug > information?), then it just stops looking. > A minimal_symbol would still be available, but it's not really useful for > this situation. > > I looked at your code again as well. > find_pc_symtab(find_function_addr(function,NULL)) would work as well, so we > don't need that global i386_windows_build_gt_gcc46. Hi, Domani Hannes, thanks, this can simplify my original patch, so I don't need to touch the infcall.c now, I have tested the new way you suggested, and it works fine. I will upload the new patch (version 3) of my patch soon. > > But I found no way to locally find out if it's a static function. > There is function->type->main_type->flag_static, but it's always 0 (and as > far as I can tell not used). So, we can't use this flag. Currently, in my patch, whether a function is a static member function is retrieved from gdb/eval.c, but I don't see it use the function->type->main_type->flag_static either. In infcall.c, it use a function to get whether a function is static: (void) find_overload_match (&argvec[1], nargs, tstr, METHOD, /* method */ &arg2, /* the object */ NULL, &valp, NULL, &static_memfuncp, 0); But the function find_overload_match is quite complex, I don't dig enough to its body, it looks like in int find_overload_match (struct value **args, int nargs, const char *name, enum oload_search_type method, struct value **objp, struct symbol *fsym, struct value **valp, struct symbol **symp, int *staticp, const int no_adl) { There are two places to update the *staticp. Yuanhui Zhang (asmwarrior) Created attachment 7448 [details] handling thiscall calling convention for Windows(V3) This new patch(v3) was suggested by Hannes Domani in comment 15. I posted a patch here: https://sourceware.org/ml/gdb-patches/2014-03/msg00610.html Any good news about this issue? @julian, what's the status about your patch? Thanks. Neither the GCC nor GDB parts gained any traction, unfortunately (and I don't think I have time at the moment to start pursuing them again, sorry!). Created attachment 11182 [details]
new patch against gdb git
This is my updated patch against gdb git.
Thank you for the patch. gdb patches should be sent to the gdb-patches mailing list. See the contribution checklist for more information: https://sourceware.org/gdb/wiki/ContributionChecklist The master branch has been updated by Hannes Domani <ssbssa@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=627c7fb8ea16388f349b6b26e20bf017d71e51fe commit 627c7fb8ea16388f349b6b26e20bf017d71e51fe Author: Hannes Domani <ssbssa@yahoo.de> Date: Mon Apr 27 15:58:09 2020 +0200 Use thiscall calling convention for class members 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-30 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. Fixed. Hi, Hannes Domani, thanks for the commit! Great work! |