search

janet-lang / janet

A dynamic language and bytecode vm
https://janet-lang.org
MIT License
3.51k stars 227 forks source link

FFI Module is not supported on several platforms #1165

Open LeviSchuck opened 1 year ago

LeviSchuck commented 1 year ago

Hello,

The change in https://github.com/janet-lang/janet/commit/398833ebe333efa751c52d2fa0f0a940d1d9878b is no longer building on the following platforms, based upon an alpine docker image, such as 3.18.

The current master branch is a -dev build for now, I understand. This issue is to inform you of the affected platforms for later review with FFI improvements and support.

The build process responds with the following:

Starting suite 0...
Finished suite 0 in 0.037 seconds - 190 of 190 tests passed.
Starting suite 1...
Finished suite 1 in 0.112 seconds - 167 of 167 tests passed.
.... snipped .... 
Finished suite 11 in 0.421 seconds - 23 of 23 tests passed.
Starting suite 12...
error: calling convention not supported
  in ffi/call [src/core/ffi.c] on line 1355
  in memcpy [test/suite0012.janet] on line 38, column 3
  in _thunk [test/suite0012.janet] (tailcall) on line 42, column 3
make: *** [Makefile:232: test] Error 1

The following platforms still compile and work as expected:

I am lead to believe that the FFI module is designed for 64 bit x86 environments only. Therefore I recommend a revert of this commit until other platforms are considered and enabled in an allow-list fashion.

By reverting commit 398833ebe333efa751c52d2fa0f0a940d1d9878b, an automated janet-docker build process I have was able to push a new image, see https://github.com/LeviSchuck/janet-docker/commit/4908e03cfebe0e52ff2b9b5b46def6d1238ce020 if interested.

Best wishes, Levi

bakpakin commented 1 year ago

So the issue here is that the tests were never updated to work on these platforms - there is some functionality in the FFI module that is useful and works on these architectures even without the calling convention support. I don't want to disable completely for that reason, but instead detect this in the test and skip testing for unsupported platforms.

bakpakin commented 1 year ago

Also, #1162 might help address issues like this.

Techcable commented 1 year ago

Maybe use libffi or dyncall to support these other architectures?

bakpakin commented 1 year ago

One of the design goals is to not link to anything besides libc to make embedding easier, so vetoing libffi and dyncall in the core

iacore commented 1 year ago

I think it can link libffi statically. Like how it's possible to link mimalloc statically.