Closed Bug 564966 Opened 14 years ago Closed 14 years ago

ctypes stdcall tests borked on windows

Categories

(Core :: js-ctypes, defect)

x86
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- beta4+

People

(Reporter: dwitte, Assigned: dwitte)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(2 files)

Attached patch fix testsSplinter Review
The #ifdef tests in test_jsctypes.js.in aren't getting tickled properly, which means the stdcall tests aren't even getting run -- fixing that revealed a bunch of other papercuts that needed fixing.

Most notably, one of the stdcall closure tests is crashing -- jumping to a garbage address on return from one of the libffi functions. Probably stack corruption/overpushing/overpopping. I commented it out for now.
Attachment #444546 - Flags: review?(mozilla+ben)
Comment on attachment 444546 [details] [diff] [review]
fix tests

> #if defined(_WIN32) && !defined(_WIN64)
> PRInt32
>-test_closure_cdecl(PRInt8 i, PRInt32 (NS_STDCALL *f)(PRInt8))
>+test_closure_stdcall(PRInt8 i, PRInt32 (NS_STDCALL *f)(PRInt8))

That'll teach you to copy/paste! ;p

>+#ifdef WIN32
>+#ifndef HAVE_64BIT_OS
>+  // Disable this test for now -- it crashes.
>+  //run_single_closure_tests(library, ctypes.stdcall_abi, "stdcall");
> #endif
> #endif

When would WIN32 be defined simultaneously with HAVE_64BIT_OS?  Just curious.
Attachment #444546 - Flags: review?(mozilla+ben) → review+
(In reply to comment #1)
> When would WIN32 be defined simultaneously with HAVE_64BIT_OS?  Just curious.

For Windows x64. I *think* our configure does it that way. If it doesn't, I'm sure Makoto will pounce on it.
WIN32 and _WIN32 are defined on Windows x86 and x64 for Mozilla code.  About _WIN32, please read http://msdn.microsoft.com/en-us/library/aa384267%28VS.85%29.aspx.  This is by compiler.  WIN32 macro is used some 3rd party libraries such as zlib and Mozilla for windows x86 and x64.
Whiteboard: fixed-in-tracemonkey
Will file new bug for closure stdcall test fixage. (Requires libffi patches & hence new libffi pull.)
Backed out due to Win Xd fail. http://hg.mozilla.org/tracemonkey/rev/039bd5d263f9
Whiteboard: fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/1d641507f763
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Reopening since this got backed out.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
This should block, since having stdcall work on Win32 is kinda important.

The patch already here is sufficient, we just need a libffi patch as well. I've pushed it upstream; once it's merged I'll pull it into trunk. Shouldn't take long...
blocking2.0: --- → ?
blocking2.0: ? → beta4+
Attached patch add FFI_LAST_ABISplinter Review
This is the libffi fix required here. I've pushed it upstream, but it hasn't been applied yet. Depending on when that happens, we might need to pull it in locally first.
Comment on attachment 462955 [details] [diff] [review]
add FFI_LAST_ABI

Looking for rs= here. (Though if it doesn't get upstreamed soon, we'll carry it locally.)
Attachment #462955 - Flags: review?(benjamin)
Attachment #462955 - Flags: review?(benjamin) → review+
http://hg.mozilla.org/tracemonkey/rev/9912fe3c7935

And pulled in a new libffi:
http://hg.mozilla.org/tracemonkey/rev/e2265b714a02
Whiteboard: fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/9912fe3c7935
Status: REOPENED → RESOLVED
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: