Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

closure api: always extend integers to full word size #456

Merged
merged 2 commits into from
Oct 17, 2016

Conversation

fdopen
Copy link
Contributor

@fdopen fdopen commented Oct 17, 2016

This will fix #441 and related problems on nearly all ARM and MIPS platforms (x86 doesn't seem to be affected because of different calling conventions).

The problems are return results of closures. If integers smaller than word-size are returned, they must be extended to full word-size. But they are currently always zero-extended, never sign-extended (https://github.com/ocamllabs/ocaml-ctypes/blob/0d0c62f5b2432a49c3d0c965b8fa9d0dd50ea857/src/ctypes-foreign-base/ffi_call_stubs.c#L489 ).

The first commit contains test code to demonstrate the issue: callback_returns_int8_t (fun () -> -128) will sometimes return +128.

If one byte is set to -128 and the higher bits are set to zero, the whole word will be read as 128. It's not a problem for the (exclusive) foreign interface, because it's hard-coded that the higher bits are ignored. But if stub code is generated, 128 is the more likely result (it depends on the compiler and its optimization settings).

@yallop
Copy link
Owner

yallop commented Oct 17, 2016

Excellent -- thanks for tracking this down! Let's hope this is the last promotion-related issue in the libffi code.

@yallop yallop merged commit 95e2e3e into yallop:master Oct 17, 2016
@fdopen fdopen deleted the integer-promotion branch April 14, 2018 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tests are failing for mips64el on Debian
2 participants