On some plats (x86/linux/gcc), thiscall is just cdecl + 'this' as the first parameter. On x86/win32, 'this' is passed in ecx -- a poor man's fastcall. To begin contemplating C++ support in ctypes, even for virtual methods or raw fnptrs, we need thiscall support in libffi. The implementation doesn't have to be exhaustive; it can be for the big three plats, and other plats just throw FFI_BAD_ABI or whatever. (And two of those plats will probably just have FFI_THISCALL === FFI_SYSV or equivalent, which is a one or two liner.) Writing assembly is a ton of fun; I'd say "good first bug", but what I really mean is "good first bug for someone who enjoys assembly". :)
FWIW, we found that XPCOM and MS-COM interfaces use stdcall declarations, so this is not necessary for those. It's only really necessary for C++ classes which don't explicitly pick a calling convention and therefore get thiscall.
P3, nice to have. Can wait til we get serious about C++ support.
Created attachment 444547 [details] [diff] [review] add FFI_THISCALL Hacks up FFI_THISCALL on Windows, adds the bits to ctypes, and tests it. This is basically what we want. Could use some tweaking. Ideally we'd have a nice class declaration syntax in JS so you don't need to manually pass 'this', e.g. let class_t = ctypes.ClassType("MyClass"); let method_t = ctypes.MethodType(class_t, "add", ...); let c = new class_t(); // class_t.prototype gets an 'add' function let result = c.add(5); but I think we can still provide the down-and-dirty syntax used here: let add = library.declare("_ZN9TestClass3addEi", ctypes.thiscall_abi, ...); let c = ...; // instantiate a blob of appropriate size add(c.address(), 5);
Assignee: nobody → dwitte
Status: NEW → ASSIGNED
Created attachment 444549 [details] [diff] [review] fix FFI_LAST_ABI Required to fix a bug in libffi where the enum values for FFI_LAST_ABI are whacked.
Yeah, I think that if we want the pretty class syntax we should implement it in JS, not in C++.
Reassigning to nobody. If anyone wants to work on this, feel free!
Assignee: dwitte → nobody
Status: ASSIGNED → NEW
FWIW, upstream libffi seems to have thiscall support already: https://github.com/atgreen/libffi/blob/master/src/x86/ffitarget.h#L71
Fixed by updating to libffi 3.1.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
libffi landed but not the patches to actually expose it to ctypes. Reopening to track that. Jorendorff or anyone else interested in mentoring this to done?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Summary: libffi support for thiscall → Expose libffi thiscall to js-ctypes
Attachment #444547 - Attachment is obsolete: true
Comment on attachment 8600858 [details] [diff] [review] bug552533 add thiscall abi support
Attachment #8600858 - Flags: review?(jorendorff)
Comment on attachment 8600858 [details] [diff] [review] bug552533 Review of attachment 8600858 [details] [diff] [review]: ----------------------------------------------------------------- Sorry I never reviewed this. Looks fine, r=me assuming the test passes on all platforms.
Attachment #8600858 - Flags: review?(jorendorff) → review+
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago → 2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
One example usage is in COM APIs on Windows - we can use ctypes.thiscall_abi instead and avoid having to pass the ptr to self on each call - https://github.com/Noitidart/ostypes/blob/master/ostypes_win.jsm#L686 - *I think* I havent tested it yet.
You need to log in before you can comment on or make changes to this bug.