Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Support different ABI on Windows #34

Closed
Mithgol opened this Issue Jan 13, 2012 · 11 comments

Comments

Projects
None yet
2 participants
Contributor

Mithgol commented Jan 13, 2012

ffi.cc does currently use FFI_DEFAULT_ABI (and FFI_DEFAULT_ABI only) when it calls ffi_prep_cif(…).

That ABI, however, sometimes cannot be used for calling Windows system functions. I guess the following (very simple) code is an example of that:

var user32 = new require('node-ffi').Library('user32', {
   'MessageBoxW': [ 'int32', [ 'int32', 'string', 'string', 'int32' ]]
});

user32.MessageBoxW(0, 'Hello world!', 'Title of the window', 0);

When run, that example creates some dialog window, but the text (such as «Hello world!») is not readable:

screenshot

Here node-ffi could follow Mozilla's js-ctypes route and support a set of ABI constants for an additional optional parameter of require('node-ffi').Library(…). Probably in the same optional object (one object per function name) where {async: 'true'} may already be given.

The complete list of available ABI is defined in https://github.com/atgreen/libffi/blob/master/src/x86/ffitarget.h and similar platform-dependent files, but support for all of them would be complex and unnecessary. I beileve that node-ffi (like js-ctypes) does actually need only some two additional ABI necessary to call Windows system functions.

Contributor

Mithgol commented Jan 13, 2012

Ignore this issue! I totally forgot that MessageBoxW needs UTF-16 instead of UTF-8.

@Mithgol Mithgol closed this Jan 13, 2012

Member

TooTallNate commented Jan 13, 2012

So you didn't need this after all? Seems like a good API to add to me, but I'm not gonna bother if there's no need. What kinds of functions would require a different ABI then?

Also, for curiosity's sake, do you have a fixed version of your example above, that converts to UTF16 bytes first?

Contributor

Mithgol commented Jan 13, 2012

Somehow MessageBoxW() from user23.dll works without any special ABI. I cannot explain it. I just look in astonishment, and it just works, though MSDN quite clearly says it's declared as WINAPI, which most likely implies some special ABI. I haven't tried anything else yet, so I'm equally not sure whether any new ABI are as needed as I've thought them to be. I believe there must be some docs, some manuals on the exact difference between those ABI. English Wikipedia is not exact and is not even close to a good starting point.

I have a fixed version of the above example, and with Russian comments and strings; I posted it on Habrahabr.Ru today. Here's an exact copy and a screenshot:

// функция преобразования строки JavaScript (UTF-8) в UTF-16
function TEXT(text){
   return new Buffer(text, 'ucs2').toString('binary');
}

var FFI = require('node-ffi');

// подключаемся к user32.dll
var user32 = new FFI.Library('user32', {
   'MessageBoxW': [
      'int32', [ 'int32', 'string', 'string', 'int32' ]
   ]
});

// диалоговое окно
var OK_or_Cancel = user32.MessageBoxW(
   0, TEXT('Привет, Хабрахабр!'), TEXT('Заголовок окна'), 1
);

(screenshot deleted because of the picture hosting bug, see below)

The TEXT() converter is analogous to the macro of the same name, described at MSDN.

If using Node.js on Windows API ever becomes popular, then Node's devs should un-deprecate 'binary' encoding of Buffer.

Member

TooTallNate commented Jan 13, 2012

Thanks for the snippet! I don't think un-deprecating binary is gonna happen though. I recommend a "safer" TEXT() function like this one that doesn't depend on deprecated APIs:

function TEXT (text) {
  // malloc() the buffer for the String
  var p = new ffi.Pointer(Buffer.byteLength(text, 'ucs2') + 1);
  var b = p.toBuffer();
  b.write(text, 'ucs2');
  b[b.length - 1] = 0;
  return p;
}

Note that this will now return a Pointer instance instead, so you should change your function signatures to 'pointer' instead of 'string'. Let me know if that works for you!

Contributor

Mithgol commented Jan 13, 2012

It works, thanks.

Member

TooTallNate commented Jan 13, 2012

Ok cool. Well I guess we'll punt on the ABI thing for now, if you find you need it with some other Windows functions then please re-open this ticket, it should be a relatively easy feature to add (though I'd need to recompile the binaries afterwards, GAH!!!)

Contributor

Mithgol commented Jan 14, 2012

Further in Wikipedia, I've finally found a page where the difference between cdecl (FFI_DEFAULT_ABI) and stdcall (FFI_STDCALL) is explained.

It seems that the only difference is dealing with stack. With cdecl, the called function ends with a simple ret and the caller cleans the stack, so the call looks like the following example:

; x = function_name(a, b, c)
push c             ; arg 3
push b             ; arg 2
push a             ; arg 1
call function_name ; jump to function_name's code
add esp, 12        ; pop function args (a, b, c) off the stack
mov x, eax         ; fetch function return value

With stdcall, the called function cleans the stack by itself (it ends with ret 12 for the above example, instead of simple ret) and thus there's no need to clean it (add esp, 12) every time after the call.

So, when stdcall function (such as MessageBoxW) is called with FFI_DEFAULT_ABI, the stack is most likely forced to pop twice the amount of data pushed into it. Looks somewhat dangerous (unless there is some compensator in libffi or in OS that I am not aware about).

Reopening the issue after all. I haven't yet stumbled across anything that actually breaks, but knowing how wrong the stack is manipulated makes me uncomfortable.

@Mithgol Mithgol reopened this Jan 14, 2012

Member

TooTallNate commented Jan 15, 2012

Ok, so we need to add an abi option to the Library function (same options object that receives async). That will pass through to ForeignFunction, which I suppose we could add on as a 5th argument. I usually don't like having functions with arguments lengths that long, so we could also change ForeignFunction's 4th argument to an options object, if we maintain backwards compatibility.

@Mithgol I'll be busy the next couple weeks probably, but if you feel like whipping up a patch then I'll accept that, otherwise it'll be a little while.

Contributor

Mithgol commented Jan 16, 2012

A couple of weeks is fine, I don't mind waiting.

Member

TooTallNate commented May 10, 2012

@Mithgol Specifying an explicit ABI to use of ForeignFunctions and Callback will be supported in v1.0.

v1.0 work-in-progress branch: https://github.com/rbranson/node-ffi/compare/ref
API changes wiki: https://github.com/rbranson/node-ffi/wiki/API-changes-from-v0.x-to-v1.x

Cheers!

Contributor

Mithgol commented May 10, 2012

The above screenshot from 4 month ago was hosted on TinyPic and is now somehow replaced with a picture of an emo girl — must be a bug on that hosting.

I'd recreate the screenshot, but after #42 I'd have to install Python and MSVC Express to do that, which is not convenient.

However, having that emo girl there is weird, and thus I am deleting the picture from my former comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment