Closed
Bug 615473
Opened 14 years ago
Closed 13 years ago
Make ctypes.stdcall_abi and ctypes.winapi_abi aliases to the sole ABI on Win64
Categories
(Core :: js-ctypes, defect)
Tracking
()
RESOLVED
FIXED
mozilla8
People
(Reporter: m_kato, Assigned: rain1)
References
()
Details
Attachments
(1 file, 1 obsolete file)
11.82 KB,
patch
|
dwitte
:
review+
m_kato
:
feedback+
|
Details | Diff | Splinter Review |
ctypes.winapi_abi is for Win32 only. Even if Win64, it is used in mailTestUtils.js.
Reporter | ||
Updated•14 years ago
|
Reporter | ||
Updated•13 years ago
|
Blocks: support-win64-thunderbird
Assignee | ||
Comment 1•13 years ago
|
||
So depending on whether I'm running on win32 or win64, I'm supposed to write the ctypes definitions differently? Ouch, that sucks -- could we simply change the meaning of winapi_abi on win64 instead?
Reporter | ||
Comment 2•13 years ago
|
||
(In reply to comment #1) > So depending on whether I'm running on win32 or win64, I'm supposed to write > the ctypes definitions differently? Ouch, that sucks -- could we simply change > the meaning of winapi_abi on win64 instead? no winapi_abi on ctypes for Win64. But we consider to add winapi_abi as alias for compatibility of windows.
Assignee | ||
Comment 3•13 years ago
|
||
Yeah, that's what I meant. I'm still calling a Windows API, so it makes sense that winapi_abi is an alias to the x64 calling convention.
Assignee | ||
Comment 4•13 years ago
|
||
What do you think? I've set off a Firefox x64 build.
Assignee | ||
Comment 5•13 years ago
|
||
The justification for stdcall being an alias to the win64 ABI is that that's how it's interpreted by MSVC: http://msdn.microsoft.com/en-us/library/zxk0tw93. "On Itanium Processor Family (IPF) and x64 processors, __stdcall is accepted and ignored by the compiler; on IPF, by convention, parameters are passed in register."
Attachment #539916 -
Attachment is obsolete: true
Attachment #539949 -
Flags: review?(dwitte)
Attachment #539949 -
Flags: feedback?(m_kato)
Attachment #539916 -
Flags: feedback?(m_kato)
Assignee | ||
Comment 6•13 years ago
|
||
Similarly, since WINAPI is defined as __stdcall per <http://msdn.microsoft.com/en-us/library/aa383751>, WINAPI effectively becomes an alias too. I think this fundamentally makes sense because it allows the exact same code to run across Windows x86 and x64.
Comment 7•13 years ago
|
||
So for a given WINAPI function, will its binary signature be identical between Win32 and Win64? DWORDS, pointer sizes, the works?
Assignee | ||
Comment 8•13 years ago
|
||
The binary signature won't be identical, but so won't the binary version of the signature on the JS side. As long as you're not doing something stupid like storing pointers in longs, things should still match up on both ends. Changing product/component/summary to fit patch.
Component: Testing Infrastructure → js-ctypes
Product: MailNews Core → Core
QA Contact: testing-infrastructure → js-ctypes
Summary: Don't use ctypes.winapi_abi when target is Win64 → Make ctypes.stdcall_abi and ctypes.winapi_abi aliases to the sole ABI on Win64
Reporter | ||
Updated•13 years ago
|
Attachment #539949 -
Flags: feedback?(m_kato) → feedback+
Assignee | ||
Comment 9•13 years ago
|
||
review ping? :)
Comment 10•13 years ago
|
||
Comment on attachment 539949 [details] [diff] [review] patch with tests, v1 Looks good. r=dwitte
Attachment #539949 -
Flags: review?(dwitte) → review+
Assignee | ||
Comment 11•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d856045de3bc
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
Assignee | ||
Comment 12•13 years ago
|
||
Uh, not fixed yet. oops.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [inbound]
Target Milestone: mozilla8 → ---
Comment 13•13 years ago
|
||
Merged: http://hg.mozilla.org/mozilla-central/rev/d856045de3bc
Status: REOPENED → RESOLVED
Closed: 13 years ago → 13 years ago
OS: Windows Vista → All
Hardware: x86_64 → All
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla8
Assignee | ||
Comment 14•13 years ago
|
||
(this bug is definitely Windows-only)
OS: All → Windows 7
Hardware: All → x86_64
You need to log in
before you can comment on or make changes to this bug.
Description
•