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.

Sign up for GitHub

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

Fix compilation for target x86_64-unknown-linux-musl. #1180

Merged
merged 8 commits into from
Jan 28, 2020
Merged

Fix compilation for target x86_64-unknown-linux-musl. #1180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8411e56
Make wabt optional.
losfair Jan 28, 2020
5476994
Remove CLI dependency on wabt.
losfair Jan 28, 2020
8845e11
Fix musl compatibility issues.
losfair Jan 28, 2020
fb15aa0
Cargo fmt
losfair Jan 28, 2020
f5d2294
Update changelog
losfair Jan 28, 2020
bf5234b
Fix kernel-loader with musl.
losfair Jan 28, 2020
5119fb3
Add musl build target for CI.
losfair Jan 28, 2020
6fa1996
Install musl-tools
losfair Jan 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .azure/install-rust.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ steps:
set -ex
rustup update --no-self-update $RUST_TOOLCHAIN
rustup default $RUST_TOOLCHAIN
rustup target add x86_64-unknown-linux-musl

rustc -Vv
cargo -V
Expand Down
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@

## **[Unreleased]**

- [#1180](https://github.com/wasmerio/wasmer/pull/1180) Fix compilation for target `x86_64-unknown-linux-musl`.
- [#1170](https://github.com/wasmerio/wasmer/pull/1170) Improve the WasiFs builder API with convenience methods for overriding stdin, stdout, and stderr as well as a new sub-builder for controlling the permissions and properties of preopened directories. Also breaks that implementations of `WasiFile` must be `Send` -- please file an issue if this change causes you any issues.
- [#1161](https://github.com/wasmerio/wasmer/pull/1161) Require imported functions to be `Send`. This is a breaking change that fixes a soundness issue in the API.
- [#1129](https://github.com/wasmerio/wasmer/pull/1129) Standard exception types for singlepass backend.
- [#1140](https://github.com/wasmerio/wasmer/pull/1140) Use [`blake3`](https://github.com/BLAKE3-team/BLAKE3) as default hashing algorithm for caching.
- [#1129](https://github.com/wasmerio/wasmer/pull/1129) Standard exception types for singlepass backend.

## 0.13.1 - 2020-01-16
- Fix bug in wapm related to the `package.wasmer_extra_flags` entry in the manifest
Expand Down
4 changes: 2 additions & 2 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ errno = "0.2"
fern = { version = "0.5", features = ["colored"], optional = true }
log = "0.4"
structopt = "0.3"
wabt = "0.9.1"
wabt = { version = "0.9.1", optional = true }
wasmer-clif-backend = { path = "lib/clif-backend", optional = true }
wasmer-singlepass-backend = { path = "lib/singlepass-backend", optional = true }
wasmer-middleware-common = { path = "lib/middleware-common" }
Expand Down Expand Up @@ -79,7 +79,7 @@ serde = { version = "1", features = ["derive"] } # used by the plugin example
typetag = "0.1" # used by the plugin example

[features]
default = ["fast-tests", "wasi", "backend-cranelift"]
default = ["fast-tests", "wasi", "backend-cranelift", "wabt"]
"loader-kernel" = ["wasmer-kernel-loader"]
debug = ["fern", "log/max_level_debug", "log/release_max_level_debug"]
trace = ["fern", "log/max_level_trace", "log/release_max_level_trace"]
Expand Down
6 changes: 6 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,12 @@ check: check-bench
release:
cargo build --release --features backend-singlepass,backend-cranelift,backend-llvm,loader-kernel,experimental-io-devices,log/release_max_level_off

# Release with musl target
release-musl:
# backend-llvm is not included due to dependency on wabt.
# experimental-io-devices is not included due to missing x11-fb.
cargo build --release --target x86_64-unknown-linux-musl --features backend-singlepass,backend-cranelift,loader-kernel,log/release_max_level_off,wasi --no-default-features

# Only one backend (cranelift)
release-clif:
# If you are on macOS, you will need mingw-w64 for cross compiling to Windows
Expand Down
3 changes: 3 additions & 0 deletions azure-pipelines.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,9 @@ jobs:
- bash: make release
displayName: Build (*nix)
condition: and(succeeded(), not(eq(variables['Agent.OS'], 'Windows_NT')))
- bash: sudo apt-get install musl-tools && make release-musl
displayName: Build (Linux, x86_64-unknown-linux-musl)
condition: and(succeeded(), eq(variables['Agent.OS'], 'Linux'))
- bash: make release-llvm
displayName: Build (Windows)
condition: and(succeeded(), eq(variables['Agent.OS'], 'Windows_NT'))
Expand Down
18 changes: 9 additions & 9 deletions lib/emscripten/src/syscalls/unix.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,12 @@ const WASM_TCSETSW: u32 = 0x5403;
// https://github.com/wasmerio/wasmer/pull/532#discussion_r300837800
fn translate_ioctl(wasm_ioctl: u32) -> c_ulong {
match wasm_ioctl {
WASM_FIOCLEX => FIOCLEX,
WASM_TIOCGWINSZ => TIOCGWINSZ,
WASM_TIOCSPGRP => TIOCSPGRP,
WASM_FIONBIO => FIONBIO,
WASM_TCGETS => TCGETS,
WASM_TCSETSW => TCSETSW,
WASM_FIOCLEX => FIOCLEX as _,
WASM_TIOCGWINSZ => TIOCGWINSZ as _,
WASM_TIOCSPGRP => TIOCSPGRP as _,
WASM_FIONBIO => FIONBIO as _,
WASM_TCGETS => TCGETS as _,
WASM_TCSETSW => TCSETSW as _,
_otherwise => {
unimplemented!("The ioctl {} is not yet implemented", wasm_ioctl);
}
Expand Down Expand Up @@ -465,7 +465,7 @@ pub fn ___syscall54(ctx: &mut Ctx, _which: c_int, mut varargs: VarArgs) -> c_int
let argp: u32 = varargs.get(ctx);
let argp_ptr = emscripten_memory_pointer!(ctx.memory(0), argp) as *mut c_void;
let translated_request = translate_ioctl(request);
let ret = unsafe { ioctl(fd, translated_request, argp_ptr) };
let ret = unsafe { ioctl(fd, translated_request as _, argp_ptr) };
debug!(
" => request: {}, translated: {}, return: {}",
request, translated_request, ret
Expand Down Expand Up @@ -526,7 +526,7 @@ pub fn ___syscall102(ctx: &mut Ctx, _which: c_int, mut varargs: VarArgs) -> c_in
if ty_and_flags & SOCK_CLOEXC != 0 {
// set_cloexec
unsafe {
ioctl(fd, translate_ioctl(WASM_FIOCLEX));
ioctl(fd, translate_ioctl(WASM_FIOCLEX) as _);
};
}

Expand Down Expand Up @@ -633,7 +633,7 @@ pub fn ___syscall102(ctx: &mut Ctx, _which: c_int, mut varargs: VarArgs) -> c_in
// why is this here?
// set_cloexec
unsafe {
ioctl(fd, translate_ioctl(WASM_FIOCLEX));
ioctl(fd, translate_ioctl(WASM_FIOCLEX) as _);
};

debug!(
Expand Down
8 changes: 4 additions & 4 deletions lib/kernel-loader/src/service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ impl ServiceContext {
let ret = unsafe {
::libc::ioctl(
fd,
Command::LoadCode as i32 as ::libc::c_ulong,
Command::LoadCode as i32 as _,
&req as *const _ as ::libc::c_ulong,
)
};
Expand All @@ -193,7 +193,7 @@ impl ServiceContext {
let err = unsafe {
::libc::ioctl(
fd,
Command::RunCode as i32 as ::libc::c_ulong,
Command::RunCode as i32 as _,
&mut req as *mut _ as ::libc::c_ulong,
)
};
Expand Down Expand Up @@ -221,7 +221,7 @@ impl ServiceContext {
let err = unsafe {
::libc::ioctl(
fd,
Command::ReadMemory as i32 as ::libc::c_ulong,
Command::ReadMemory as i32 as _,
&req as *const _ as ::libc::c_ulong,
)
};
Expand All @@ -242,7 +242,7 @@ impl ServiceContext {
let err = unsafe {
::libc::ioctl(
fd,
Command::WriteMemory as i32 as ::libc::c_ulong,
Command::WriteMemory as i32 as _,
&req as *const _ as ::libc::c_ulong,
)
};
Expand Down
72 changes: 44 additions & 28 deletions lib/runtime-core/src/fault.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,13 +570,13 @@ pub unsafe fn get_fault_info(siginfo: *const c_void, ucontext: *mut c_void) -> F
#[cfg(all(target_os = "linux", target_arch = "x86_64"))]
/// Get fault info from siginfo and ucontext.
pub unsafe fn get_fault_info(siginfo: *const c_void, ucontext: *mut c_void) -> FaultInfo {
use crate::state::x64::XMM;
use libc::{
_libc_xmmreg, ucontext_t, REG_R10, REG_R11, REG_R12, REG_R13, REG_R14, REG_R15, REG_R8,
REG_R9, REG_RAX, REG_RBP, REG_RBX, REG_RCX, REG_RDI, REG_RDX, REG_RIP, REG_RSI, REG_RSP,
ucontext_t, REG_R10, REG_R11, REG_R12, REG_R13, REG_R14, REG_R15, REG_R8, REG_R9, REG_RAX,
REG_RBP, REG_RBX, REG_RCX, REG_RDI, REG_RDX, REG_RIP, REG_RSI, REG_RSP,
};

fn read_xmm(reg: &_libc_xmmreg) -> u64 {
#[cfg(not(target_env = "musl"))]
fn read_xmm(reg: &libc::_libc_xmmreg) -> u64 {
(reg.element[0] as u64) | ((reg.element[1] as u64) << 32)
}

Expand Down Expand Up @@ -615,30 +615,46 @@ pub unsafe fn get_fault_info(siginfo: *const c_void, ucontext: *mut c_void) -> F
known_registers[X64Register::GPR(GPR::RBP).to_index().0] = Some(gregs[REG_RBP as usize] as _);
known_registers[X64Register::GPR(GPR::RSP).to_index().0] = Some(gregs[REG_RSP as usize] as _);

if !(*ucontext).uc_mcontext.fpregs.is_null() {
let fpregs = &*(*ucontext).uc_mcontext.fpregs;
known_registers[X64Register::XMM(XMM::XMM0).to_index().0] = Some(read_xmm(&fpregs._xmm[0]));
known_registers[X64Register::XMM(XMM::XMM1).to_index().0] = Some(read_xmm(&fpregs._xmm[1]));
known_registers[X64Register::XMM(XMM::XMM2).to_index().0] = Some(read_xmm(&fpregs._xmm[2]));
known_registers[X64Register::XMM(XMM::XMM3).to_index().0] = Some(read_xmm(&fpregs._xmm[3]));
known_registers[X64Register::XMM(XMM::XMM4).to_index().0] = Some(read_xmm(&fpregs._xmm[4]));
known_registers[X64Register::XMM(XMM::XMM5).to_index().0] = Some(read_xmm(&fpregs._xmm[5]));
known_registers[X64Register::XMM(XMM::XMM6).to_index().0] = Some(read_xmm(&fpregs._xmm[6]));
known_registers[X64Register::XMM(XMM::XMM7).to_index().0] = Some(read_xmm(&fpregs._xmm[7]));
known_registers[X64Register::XMM(XMM::XMM8).to_index().0] = Some(read_xmm(&fpregs._xmm[8]));
known_registers[X64Register::XMM(XMM::XMM9).to_index().0] = Some(read_xmm(&fpregs._xmm[9]));
known_registers[X64Register::XMM(XMM::XMM10).to_index().0] =
Some(read_xmm(&fpregs._xmm[10]));
known_registers[X64Register::XMM(XMM::XMM11).to_index().0] =
Some(read_xmm(&fpregs._xmm[11]));
known_registers[X64Register::XMM(XMM::XMM12).to_index().0] =
Some(read_xmm(&fpregs._xmm[12]));
known_registers[X64Register::XMM(XMM::XMM13).to_index().0] =
Some(read_xmm(&fpregs._xmm[13]));
known_registers[X64Register::XMM(XMM::XMM14).to_index().0] =
Some(read_xmm(&fpregs._xmm[14]));
known_registers[X64Register::XMM(XMM::XMM15).to_index().0] =
Some(read_xmm(&fpregs._xmm[15]));
// Skip reading floating point registers when building with musl libc.
// FIXME: Depends on https://github.com/rust-lang/libc/pull/1646
#[cfg(not(target_env = "musl"))]
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to handle the other case too? Like

#[cfg(target_env = "musl")]
unimplemented!("blocked on https://github.com/rust-lang/libc/pull/1646");

Or will it work fine as-is?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is on the always-taken path for fault handling so panicking here will break things that depend on fault handling, like middlewares. Now all XMM registers will be defaulted to zero if not dumped; do you think it is fine?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I'm not familiar enough to know what the impacts of that are: if you think this leaves the system in a sensible state where execution makes sense then I'm happy with it! Otherwise, I don't think compiling and failing silently is necessarily a step up from panicking or not compiling.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only case where doing so is possibly incorrect is when we take a managed snapshot and then unwind out from WebAssembly code (where floating point state will be lost). This feature is not widely used (yet) and I think it is "cold" enough to ignore for now and defer to after a future upstream fix in musl-libc.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, that sounds reasonable to me! We just need to remember to not forget to update this!

{
use crate::state::x64::XMM;
if !(*ucontext).uc_mcontext.fpregs.is_null() {
let fpregs = &*(*ucontext).uc_mcontext.fpregs;
known_registers[X64Register::XMM(XMM::XMM0).to_index().0] =
Some(read_xmm(&fpregs._xmm[0]));
known_registers[X64Register::XMM(XMM::XMM1).to_index().0] =
Some(read_xmm(&fpregs._xmm[1]));
known_registers[X64Register::XMM(XMM::XMM2).to_index().0] =
Some(read_xmm(&fpregs._xmm[2]));
known_registers[X64Register::XMM(XMM::XMM3).to_index().0] =
Some(read_xmm(&fpregs._xmm[3]));
known_registers[X64Register::XMM(XMM::XMM4).to_index().0] =
Some(read_xmm(&fpregs._xmm[4]));
known_registers[X64Register::XMM(XMM::XMM5).to_index().0] =
Some(read_xmm(&fpregs._xmm[5]));
known_registers[X64Register::XMM(XMM::XMM6).to_index().0] =
Some(read_xmm(&fpregs._xmm[6]));
known_registers[X64Register::XMM(XMM::XMM7).to_index().0] =
Some(read_xmm(&fpregs._xmm[7]));
known_registers[X64Register::XMM(XMM::XMM8).to_index().0] =
Some(read_xmm(&fpregs._xmm[8]));
known_registers[X64Register::XMM(XMM::XMM9).to_index().0] =
Some(read_xmm(&fpregs._xmm[9]));
known_registers[X64Register::XMM(XMM::XMM10).to_index().0] =
Some(read_xmm(&fpregs._xmm[10]));
known_registers[X64Register::XMM(XMM::XMM11).to_index().0] =
Some(read_xmm(&fpregs._xmm[11]));
known_registers[X64Register::XMM(XMM::XMM12).to_index().0] =
Some(read_xmm(&fpregs._xmm[12]));
known_registers[X64Register::XMM(XMM::XMM13).to_index().0] =
Some(read_xmm(&fpregs._xmm[13]));
known_registers[X64Register::XMM(XMM::XMM14).to_index().0] =
Some(read_xmm(&fpregs._xmm[14]));
known_registers[X64Register::XMM(XMM::XMM15).to_index().0] =
Some(read_xmm(&fpregs._xmm[15]));
}
}

FaultInfo {
Expand Down
47 changes: 29 additions & 18 deletions src/bin/wasmer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ extern crate log;

use std::collections::HashMap;
use std::env;
use std::error::Error;
use std::fs::{read_to_string, File};
use std::io;
use std::io::Read;
Expand Down Expand Up @@ -100,6 +99,7 @@ struct PrestandardFeatures {

impl PrestandardFeatures {
/// Generate [`wabt::Features`] struct from CLI options
#[cfg(feature = "wabt")]
pub fn into_wabt_features(&self) -> wabt::Features {
let mut features = wabt::Features::new();
if self.simd || self.all {
Expand Down Expand Up @@ -570,6 +570,7 @@ fn execute_wasm(options: &Run) -> Result<(), String> {
let env_vars = get_env_var_args(&options.env_vars[..])?;
let wasm_path = &options.path;

#[allow(unused_mut)]
let mut wasm_binary: Vec<u8> = read_file_contents(wasm_path).map_err(|err| {
format!(
"Can't read the file {}: {}",
Expand Down Expand Up @@ -635,23 +636,33 @@ fn execute_wasm(options: &Run) -> Result<(), String> {
}

if !utils::is_wasm_binary(&wasm_binary) {
let features = options.features.into_wabt_features();
wasm_binary = wabt::wat2wasm_with_features(wasm_binary, features).map_err(|e| {
format!(
"Can't convert from wast to wasm because \"{}\"{}",
e.description(),
match e.kind() {
wabt::ErrorKind::Deserialize(s)
| wabt::ErrorKind::Parse(s)
| wabt::ErrorKind::ResolveNames(s)
| wabt::ErrorKind::Validate(s) => format!(":\n\n{}", s),
wabt::ErrorKind::Nul
| wabt::ErrorKind::WriteText
| wabt::ErrorKind::NonUtf8Result
| wabt::ErrorKind::WriteBinary => "".to_string(),
}
)
})?;
#[cfg(feature = "wabt")]
{
let features = options.features.into_wabt_features();
wasm_binary = wabt::wat2wasm_with_features(wasm_binary, features).map_err(|e| {
format!(
"Can't convert from wast to wasm because \"{}\"{}",
e,
match e.kind() {
wabt::ErrorKind::Deserialize(s)
| wabt::ErrorKind::Parse(s)
| wabt::ErrorKind::ResolveNames(s)
| wabt::ErrorKind::Validate(s) => format!(":\n\n{}", s),
wabt::ErrorKind::Nul
| wabt::ErrorKind::WriteText
| wabt::ErrorKind::NonUtf8Result
| wabt::ErrorKind::WriteBinary => "".to_string(),
}
)
})?;
}

#[cfg(not(feature = "wabt"))]
{
return Err(
"Input is not a wasm binary and the `wabt` feature is not enabled".to_string(),
);
}
}

let compiler: Box<dyn Compiler> = get_compiler_by_backend(options.backend, options)
Expand Down