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

Don't run compiled binaries while cross-compiling #383

Merged
merged 1 commit into from May 5, 2016
Merged

Don't run compiled binaries while cross-compiling #383

merged 1 commit into from May 5, 2016

Conversation

whitequark
Copy link
Contributor

@whitequark whitequark commented May 2, 2016

Trying to run binaries while cross-compiling is an extremely annoying habit. To a degree it can be mitigated by manually shimming it, but for opam-cross-windows I need two shims, for opam-cross-ios I need four, and for opam-cross-android... uh... the amount is theoretically bounded, but practically the limit is too high to be manageable even in principle. Not to mention adding shims being dull and prone to produce errors that are extremely hard to debug, and in some cases just very hard to obtain; the immediate motivation for this was the fact that I have no damn clue how to run a regular binary on an iOS simulator (is it possible at all?) nor do I have any desire to figure that out.

So, I offer an alternative solution that does not need to run any binaries. It works with any sane C compiler, specifically any compiler that places a character array contiguously, verbatim, and in ASCII somewhere in the object file. OCaml doesn't work on non-ASCII systems and the design of contemporary linkers (that are dumb code combiners) combined with the fact that OCaml does not use linker LTO plugins mandate that these assumptions be true.

Specifically, what my solution does is it takes a compile-time constant, converts it to a string guarded by a prefix and a suffix, and assigns it to a global externally visible symbol. Then, it reads the object file and extracts the decimal between the guards. This is guaranteed by C99 to work for any compile-time constant, and FFI defines, sizeof and alignof count.

As a bonus, CC is not used anymore, but rather the C compiler is invoked through ocamlfind ocamlc, which respects the OCAMLFIND_TOOLCHAIN environment variable.

@whitequark
Copy link
Contributor Author

Note: I've not yet ported the primitive detail extraction code as it does more work in C. I'd like to get feedback on existing changes though, and especially on how to deduplicate the C compiler interface code between configure/ and libffi-abigen/. (I've no idea how to do it in your build system.)

@whitequark
Copy link
Contributor Author

Updated to encompass the C primitive queries as well. All tests pass on Linux.

@whitequark
Copy link
Contributor Author

Updated again to add the $(HOSTOCAMLFIND) variable for almost completely seamless cross-compilation support. asneeded.config and libffi.config need to be provided explicitly, but I think that's OK.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 87.909% when pulling 1b608b7 on whitequark:dont-run-binaries into b40107d on ocamllabs:master.

@yallop
Copy link
Owner

yallop commented May 3, 2016

Thanks! I'd like to improve support for cross-compilation, so I appreciate this. I'll aim to take a proper look in the next few days.

@whitequark
Copy link
Contributor Author

I've now verified that I can cross-compile ctypes to Windows, Android and 32/64-bit iOS/iOS simulator using an opam file like this:

opam-version: "1.2"
maintainer: "whitequark@whitequark.org"
patches: ["patches/cross-compilation.patch" "patches/no-shlib.patch"]
substs: ["libffi.config"]
build: [
  [make "OCAMLFIND=ocamlfind -toolchain ios" "HOSTOCAMLFIND=ocamlfind"]
]
install: [
  [make "OCAMLFIND=ocamlfind -toolchain ios" "HOSTOCAMLFIND=ocamlfind"
        "install"]
]
remove: [
  ["ocamlfind" "-toolchain" "ios" "remove" "ctypes"]
]
depends: [
  "ocaml-ios"
  "libffi-sys-ios"
  "ocamlfind" {build}
]

@yallop
Copy link
Owner

yallop commented May 4, 2016

Could you please make the configure step a bit quieter? I'd like to avoid displaying this sort of thing:

/tmp/ctypes_libffi_confige0bf36.c:23:7: error: ‘FFI_EABI’ undeclared here (not in a function)
   D9((FFI_EABI)),
       ^

which might alarm people compiling the library.

try Sys.getenv name
with Not_found -> default

let nsplit sep str =
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that nsplit could be implemented like this:

Str.(split (regexp_string sep)) str

(The behaviour is slightly different when the separator appears at the beginning or the end of the string, but that probably doesn't matter.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you'll agree with me that Str is badly designed and should be deprecated, if so, why use it if I already wrote code without it?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Writing the code is a sunk cost, but maintaining it isn't. I'd prefer to maintain a one-liner.

And yes, Str is badly designed. Its only real virtue is that it's in the standard library.

@whitequark
Copy link
Contributor Author

Could you please make the configure step a bit quieter?

I initially did, but this caused issues when it failed on CI and I had no idea why. How about writing that to a temporary file in the Makefile and then displaying it if the build fails?

@yallop
Copy link
Owner

yallop commented May 4, 2016

Yes, that sounds better.

Specifically, do not try to run any freshly compiled binaries.
Instead, let compiler run the expansions and calculations whose
result we need, put it into .rodata surrounded by markers, and
then extract that.
@whitequark
Copy link
Contributor Author

All fixed.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 87.909% when pulling cb8291d on whitequark:dont-run-binaries into b40107d on ocamllabs:master.

@yallop
Copy link
Owner

yallop commented May 5, 2016

Thank you!

@yallop yallop merged commit a1f288a into yallop:master May 5, 2016
@whitequark whitequark deleted the dont-run-binaries branch June 7, 2016 10:53
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.

None yet

3 participants