<div dir="ltr"><div>It looks like at one point that was needed for Win64</div><div><a href="https://bugzilla.mozilla.org/show_bug.cgi?id=668436#c8">https://bugzilla.mozilla.org/show_bug.cgi?id=668436#c8</a></div><div><br></div><div>From that comment<br></div><div>"This needs to be ctypes.default_abi so it will also work on Win64... I verified this change returns the correct value on my Win7 x64 system with both 32 and 64 bit Firefox.<br><a href="http://mxr.mozilla.org/mozilla-central/source/js/src/ctypes/CTypes.cpp#4547">http://mxr.mozilla.org/mozilla-central/source/js/src/ctypes/CTypes.cpp#4547</a>"</div><div><br></div><div>Robert</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Apr 25, 2017 at 2:58 PM, Justin Dolske <span dir="ltr"><<a href="mailto:dolske@mozilla.com" target="_blank">dolske@mozilla.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>Eek. This seems like it's likely to bite us again in the future if it's dependent on people remembering this. Is there any way we can do something to make ctypes fail harder in such cases?</div><div><br></div><div>I sorta guess the answer is no (or else the argument wouldn't be needed in the first place)... Can we do something clever in, say, debug builds, by checking against common win32 DLL names, canary values on the stack, etc?</div><div><br></div><div>OOC, how is this handled when code is natively compiled/linked?<span class="HOEnZb"><font color="#888888"><br></font></span></div><span class="HOEnZb"><font color="#888888"><div><br></div><div>Justin<br></div></font></span></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Apr 25, 2017 at 2:49 PM, Chris Peterson <span dir="ltr"><<a href="mailto:cpeterson@mozilla.com" target="_blank">cpeterson@mozilla.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Is there value in consolidate most of these ctypes API calls in a common "winapi.js" module? That might make code reuse and review easier. It looks like there is a lot of brittle boilerplate code, such as the five different .js files that call kernel32.declare("GetVersionEx<wbr>W"), each with their own bespoke definition of struct OSVERSIONINFOEXW.<div class="m_-3815251591192415379HOEnZb"><div class="m_-3815251591192415379h5"><br>
<br>
<br>
On 2017-04-25 2:29 PM, Aaron Klotz wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi everybody,<br>
<br>
Last week I discovered that we have (soon to be "had") several places in our code where we were declaring Windows APIs using an incorrect ABI with js-ctypes. [1]<br>
<br>
On 32-bit Windows builds, this is not a good thing.<br>
<br>
Most (but not all) Windows APIs are declared using the WINAPI macro (which maps to ctypes.winapi_abi). Using ctypes.default_abi on one of these functions could corrupt the stack of the calling thread. [2]<br>
<br>
In general, if you look up your desired Windows API function on MSDN and its declaration contains WINAPI, you should be using ctypes.winapi_abi. On the other hand, if WINAPI is not present, that does not necessarily mean that you may safely use ctypes.default_abi; check the declarations in the Windows SDK headers to be sure.<br>
<br>
Finally, if you are landing code that contains new js-ctypes declarations, please obtain an r+ from somebody who knows and understands the various ABIs. And please do not cargo-cult the ABI parameter from existing declarations. Let's try to prevent future occurrences. :-)<br>
<br>
Thanks,<br>
<br>
Aaron<br>
<br>
<br>
[1] Search for whiteboard containing [ctypes.abi]if you want to see the bugs.<br>
<br>
[2] On Win32, most API functions pop their own parameters off the stack when they return. ctypes.default_abi assumes that the /caller/ must pop the parameters off the stack once the callee returns. If you specify ctypes.default_abi on one of the affected WINAPI functions, then both the caller and the callee pop, thus messing up the calling thread's stack pointer. This distinction goes away on Win64, but ctypes.winapi_abi always does the right thing, so please use it.<br>
<br>
Why haven't we been seeing crashes right and left because of this? I'm not sure. I don't have the time to investigate further (maybe somebody else knows?), but I suspect that either js-ctypes or libffi might be setting up extra stuff in the calling stack frame such that these double-pops have not been moving the stack pointer enough to cross any important boundaries. Whatever it is that made us so lucky, we shouldn't be relying on it! ;-)<br>
______________________________<wbr>_________________<br>
firefox-dev mailing list<br>
<a href="mailto:firefox-dev@mozilla.org" target="_blank">firefox-dev@mozilla.org</a><br>
<a href="https://mail.mozilla.org/listinfo/firefox-dev" rel="noreferrer" target="_blank">https://mail.mozilla.org/listi<wbr>nfo/firefox-dev</a><br>
</blockquote>
<br>
______________________________<wbr>_________________<br>
firefox-dev mailing list<br>
<a href="mailto:firefox-dev@mozilla.org" target="_blank">firefox-dev@mozilla.org</a><br>
<a href="https://mail.mozilla.org/listinfo/firefox-dev" rel="noreferrer" target="_blank">https://mail.mozilla.org/listi<wbr>nfo/firefox-dev</a><br>
</div></div></blockquote></div><br></div>
</div></div><br>______________________________<wbr>_________________<br>
firefox-dev mailing list<br>
<a href="mailto:firefox-dev@mozilla.org">firefox-dev@mozilla.org</a><br>
<a href="https://mail.mozilla.org/listinfo/firefox-dev" rel="noreferrer" target="_blank">https://mail.mozilla.org/<wbr>listinfo/firefox-dev</a><br>
<br></blockquote></div><br></div>