From a19cf99b55faef8d8d662d0689a2bb5fe47c6b61 Mon Sep 17 00:00:00 2001 From: "Adam C. Foltzer" Date: Thu, 23 Apr 2020 17:06:07 -0700 Subject: [PATCH 1/7] =?UTF-8?q?=F0=9F=8E=AC=20Add=20explicit=20API=20to=20?= =?UTF-8?q?run=20start=20functions;=20don't=20run=20it=20implicitly?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, the start function would be implicitly run upon instance creation or resetting. Since the environment can be different at instantiation time to when export functions are typically called (missing embedder contexts, etc), this has tripped people up. It can also just be surprising that any code runs at all at those points, which combined with the inherently confusing nature of Wasm start functions just led to too many messes. `Instance::run_start()` is now part of the public API, and will run the start function if it is present in that instance's Wasm module. It does nothing if there is no start function. `Instance::run()` will now return `Err(Error::InstanceNeedsStart)` if the start function is present but hasn't been run since the instance was created or reset. This commit bumps the development version to `0.7.0-dev`, because while the API changes are semver-compatible, the semantics are not. --- CHANGELOG.md | 4 + Cargo.lock | 140 ++++++++-------- benchmarks/lucet-benchmarks/Cargo.toml | 2 +- lucet-module/Cargo.toml | 2 +- lucet-module/src/functions.rs | 6 + lucet-objdump/Cargo.toml | 4 +- lucet-runtime/Cargo.toml | 12 +- lucet-runtime/include/lucet.h | 2 + .../lucet-runtime-internals/Cargo.toml | 6 +- .../lucet-runtime-internals/src/c_api.rs | 2 + .../lucet-runtime-internals/src/error.rs | 3 + .../lucet-runtime-internals/src/instance.rs | 130 ++++++++++----- .../src/instance/state.rs | 22 ++- .../lucet-runtime-internals/src/module.rs | 6 +- .../lucet-runtime-internals/src/module/dl.rs | 15 +- .../src/module/mock.rs | 9 +- .../lucet-runtime-internals/src/region.rs | 10 -- lucet-runtime/lucet-runtime-macros/Cargo.toml | 2 +- lucet-runtime/lucet-runtime-tests/Cargo.toml | 10 +- .../lucet-runtime-tests/src/guest_fault.rs | 37 +---- .../lucet-runtime-tests/src/helpers.rs | 30 ++++ .../lucet-runtime-tests/src/start.rs | 156 +++++++++++++++++- lucet-runtime/src/c_api.rs | 10 ++ lucet-spectest/Cargo.toml | 8 +- lucet-validate/Cargo.toml | 6 +- lucet-wasi-fuzz/Cargo.toml | 2 +- lucet-wasi-sdk/Cargo.toml | 10 +- lucet-wasi/Cargo.toml | 12 +- lucet-wasi/generate/Cargo.toml | 4 +- lucet-wiggle/Cargo.toml | 12 +- lucet-wiggle/generate/Cargo.toml | 4 +- lucet-wiggle/macro/Cargo.toml | 4 +- lucetc/Cargo.toml | 8 +- 33 files changed, 466 insertions(+), 224 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46103f244..a2e983bce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ - Added `install_lucet_signal_handler()` and `remove_lucet_signal_handler()`, along with `Instance::ensure_signal_handler_installed()` and `Instance::ensure_sigstack_installed()` options to control the automatic installation and removal of signal handlers and alternate signal stacks. The default behaviors have not changed. +- Added `Instance::run_start()` to the public API, which runs the Wasm start function if it is present in that instance's Wasm module. It does nothing if there is no start function. + + Creating or resetting an instance no longer implicitly runs the start function. Embedders must ensure that `run_start()` is called before calling any other exported functions. `Instance::run()` will now return `Err(Error::InstanceNeedsStart)` if the start function is present but hasn't been run since the instance was created or reset. + ### 0.6.1 (2020-02-18) - Added metadata to compiled modules that record whether instruction counting instrumentation is present. diff --git a/Cargo.lock b/Cargo.lock index a0d025cf6..a9f8ecdb2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -735,16 +735,16 @@ dependencies = [ [[package]] name = "lucet-benchmarks" -version = "0.6.2-dev" +version = "0.7.0-dev" dependencies = [ "criterion 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.68 (registry+https://github.com/rust-lang/crates.io-index)", - "lucet-module 0.6.2-dev", - "lucet-runtime 0.6.2-dev", - "lucet-runtime-internals 0.6.2-dev", - "lucet-wasi 0.6.2-dev", - "lucet-wasi-sdk 0.6.2-dev", - "lucetc 0.6.2-dev", + "lucet-module 0.7.0-dev", + "lucet-runtime 0.7.0-dev", + "lucet-runtime-internals 0.7.0-dev", + "lucet-wasi 0.7.0-dev", + "lucet-wasi-sdk 0.7.0-dev", + "lucetc 0.7.0-dev", "nix 0.17.0 (registry+https://github.com/rust-lang/crates.io-index)", "num_cpus 1.12.0 (registry+https://github.com/rust-lang/crates.io-index)", "rayon 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -753,7 +753,7 @@ dependencies = [ [[package]] name = "lucet-module" -version = "0.6.2-dev" +version = "0.7.0-dev" dependencies = [ "anyhow 1.0.28 (registry+https://github.com/rust-lang/crates.io-index)", "bincode 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -773,28 +773,28 @@ dependencies = [ [[package]] name = "lucet-objdump" -version = "0.6.2-dev" +version = "0.7.0-dev" dependencies = [ "byteorder 1.3.4 (registry+https://github.com/rust-lang/crates.io-index)", "colored 1.9.3 (registry+https://github.com/rust-lang/crates.io-index)", - "lucet-module 0.6.2-dev", + "lucet-module 0.7.0-dev", "object 0.18.0 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] name = "lucet-runtime" -version = "0.6.2-dev" +version = "0.7.0-dev" dependencies = [ "anyhow 1.0.28 (registry+https://github.com/rust-lang/crates.io-index)", "byteorder 1.3.4 (registry+https://github.com/rust-lang/crates.io-index)", "cc 1.0.50 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.68 (registry+https://github.com/rust-lang/crates.io-index)", - "lucet-module 0.6.2-dev", - "lucet-runtime-internals 0.6.2-dev", - "lucet-runtime-tests 0.6.2-dev", - "lucet-wasi-sdk 0.6.2-dev", - "lucetc 0.6.2-dev", + "lucet-module 0.7.0-dev", + "lucet-runtime-internals 0.7.0-dev", + "lucet-runtime-tests 0.7.0-dev", + "lucet-wasi-sdk 0.7.0-dev", + "lucetc 0.7.0-dev", "nix 0.17.0 (registry+https://github.com/rust-lang/crates.io-index)", "num-derive 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "num-traits 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)", @@ -806,13 +806,13 @@ dependencies = [ name = "lucet-runtime-example" version = "0.1.0" dependencies = [ - "lucet-runtime 0.6.2-dev", - "lucet-wasi 0.6.2-dev", + "lucet-runtime 0.7.0-dev", + "lucet-wasi 0.7.0-dev", ] [[package]] name = "lucet-runtime-internals" -version = "0.6.2-dev" +version = "0.7.0-dev" dependencies = [ "anyhow 1.0.28 (registry+https://github.com/rust-lang/crates.io-index)", "bincode 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -823,8 +823,8 @@ dependencies = [ "lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.68 (registry+https://github.com/rust-lang/crates.io-index)", "libloading 0.5.2 (registry+https://github.com/rust-lang/crates.io-index)", - "lucet-module 0.6.2-dev", - "lucet-runtime-macros 0.6.2-dev", + "lucet-module 0.7.0-dev", + "lucet-runtime-macros 0.7.0-dev", "memoffset 0.5.4 (registry+https://github.com/rust-lang/crates.io-index)", "nix 0.17.0 (registry+https://github.com/rust-lang/crates.io-index)", "num-derive 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -836,7 +836,7 @@ dependencies = [ [[package]] name = "lucet-runtime-macros" -version = "0.6.2-dev" +version = "0.7.0-dev" dependencies = [ "quote 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)", "syn 1.0.17 (registry+https://github.com/rust-lang/crates.io-index)", @@ -844,27 +844,27 @@ dependencies = [ [[package]] name = "lucet-runtime-tests" -version = "0.6.2-dev" +version = "0.7.0-dev" dependencies = [ "anyhow 1.0.28 (registry+https://github.com/rust-lang/crates.io-index)", "cc 1.0.50 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.68 (registry+https://github.com/rust-lang/crates.io-index)", - "lucet-module 0.6.2-dev", - "lucet-runtime-internals 0.6.2-dev", - "lucet-wasi-sdk 0.6.2-dev", - "lucetc 0.6.2-dev", + "lucet-module 0.7.0-dev", + "lucet-runtime-internals 0.7.0-dev", + "lucet-wasi-sdk 0.7.0-dev", + "lucetc 0.7.0-dev", "tempfile 3.1.0 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] name = "lucet-spectest" -version = "0.6.2-dev" +version = "0.7.0-dev" dependencies = [ "clap 2.33.0 (registry+https://github.com/rust-lang/crates.io-index)", - "lucet-module 0.6.2-dev", - "lucet-runtime 0.6.2-dev", - "lucetc 0.6.2-dev", + "lucet-module 0.7.0-dev", + "lucet-runtime 0.7.0-dev", + "lucetc 0.7.0-dev", "serde 1.0.106 (registry+https://github.com/rust-lang/crates.io-index)", "serde_json 1.0.51 (registry+https://github.com/rust-lang/crates.io-index)", "target-lexicon 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -875,12 +875,12 @@ dependencies = [ [[package]] name = "lucet-validate" -version = "0.6.2-dev" +version = "0.7.0-dev" dependencies = [ "clap 2.33.0 (registry+https://github.com/rust-lang/crates.io-index)", "cranelift-entity 0.62.0", - "lucet-wasi 0.6.2-dev", - "lucet-wasi-sdk 0.6.2-dev", + "lucet-wasi 0.7.0-dev", + "lucet-wasi-sdk 0.7.0-dev", "tempfile 3.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "thiserror 1.0.14 (registry+https://github.com/rust-lang/crates.io-index)", "wabt 0.9.2 (registry+https://github.com/rust-lang/crates.io-index)", @@ -890,21 +890,21 @@ dependencies = [ [[package]] name = "lucet-wasi" -version = "0.6.2-dev" +version = "0.7.0-dev" dependencies = [ "anyhow 1.0.28 (registry+https://github.com/rust-lang/crates.io-index)", "cast 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)", "clap 2.33.0 (registry+https://github.com/rust-lang/crates.io-index)", "human-size 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.68 (registry+https://github.com/rust-lang/crates.io-index)", - "lucet-module 0.6.2-dev", - "lucet-runtime 0.6.2-dev", - "lucet-runtime-internals 0.6.2-dev", - "lucet-validate 0.6.2-dev", - "lucet-wasi-generate 0.6.2-dev", - "lucet-wasi-sdk 0.6.2-dev", - "lucet-wiggle 0.6.2-dev", - "lucetc 0.6.2-dev", + "lucet-module 0.7.0-dev", + "lucet-runtime 0.7.0-dev", + "lucet-runtime-internals 0.7.0-dev", + "lucet-validate 0.7.0-dev", + "lucet-wasi-generate 0.7.0-dev", + "lucet-wasi-sdk 0.7.0-dev", + "lucet-wiggle 0.7.0-dev", + "lucetc 0.7.0-dev", "nix 0.17.0 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.6.5 (registry+https://github.com/rust-lang/crates.io-index)", "tempfile 3.1.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -913,16 +913,16 @@ dependencies = [ [[package]] name = "lucet-wasi-fuzz" -version = "0.6.2-dev" +version = "0.7.0-dev" dependencies = [ "anyhow 1.0.28 (registry+https://github.com/rust-lang/crates.io-index)", "clap 2.33.0 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.68 (registry+https://github.com/rust-lang/crates.io-index)", - "lucet-module 0.6.2-dev", - "lucet-runtime 0.6.2-dev", - "lucet-wasi 0.6.2-dev", - "lucet-wasi-sdk 0.6.2-dev", - "lucetc 0.6.2-dev", + "lucet-module 0.7.0-dev", + "lucet-runtime 0.7.0-dev", + "lucet-wasi 0.7.0-dev", + "lucet-wasi-sdk 0.7.0-dev", + "lucetc 0.7.0-dev", "nix 0.17.0 (registry+https://github.com/rust-lang/crates.io-index)", "num_cpus 1.12.0 (registry+https://github.com/rust-lang/crates.io-index)", "progress 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -936,9 +936,9 @@ dependencies = [ [[package]] name = "lucet-wasi-generate" -version = "0.6.2-dev" +version = "0.7.0-dev" dependencies = [ - "lucet-wiggle 0.6.2-dev", + "lucet-wiggle 0.7.0-dev", "proc-macro2 1.0.10 (registry+https://github.com/rust-lang/crates.io-index)", "quote 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)", "syn 1.0.17 (registry+https://github.com/rust-lang/crates.io-index)", @@ -948,13 +948,13 @@ dependencies = [ [[package]] name = "lucet-wasi-sdk" -version = "0.6.2-dev" +version = "0.7.0-dev" dependencies = [ "anyhow 1.0.28 (registry+https://github.com/rust-lang/crates.io-index)", - "lucet-module 0.6.2-dev", - "lucet-validate 0.6.2-dev", - "lucet-wasi 0.6.2-dev", - "lucetc 0.6.2-dev", + "lucet-module 0.7.0-dev", + "lucet-validate 0.7.0-dev", + "lucet-wasi 0.7.0-dev", + "lucetc 0.7.0-dev", "target-lexicon 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)", "tempfile 3.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "thiserror 1.0.14 (registry+https://github.com/rust-lang/crates.io-index)", @@ -962,13 +962,13 @@ dependencies = [ [[package]] name = "lucet-wiggle" -version = "0.6.2-dev" +version = "0.7.0-dev" dependencies = [ - "lucet-runtime 0.6.2-dev", - "lucet-wasi-sdk 0.6.2-dev", - "lucet-wiggle-generate 0.6.2-dev", - "lucet-wiggle-macro 0.6.2-dev", - "lucetc 0.6.2-dev", + "lucet-runtime 0.7.0-dev", + "lucet-wasi-sdk 0.7.0-dev", + "lucet-wiggle-generate 0.7.0-dev", + "lucet-wiggle-macro 0.7.0-dev", + "lucetc 0.7.0-dev", "tempfile 3.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "wiggle 0.15.0", "wiggle-test 0.15.0", @@ -976,10 +976,10 @@ dependencies = [ [[package]] name = "lucet-wiggle-generate" -version = "0.6.2-dev" +version = "0.7.0-dev" dependencies = [ "heck 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)", - "lucet-module 0.6.2-dev", + "lucet-module 0.7.0-dev", "proc-macro2 1.0.10 (registry+https://github.com/rust-lang/crates.io-index)", "quote 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)", "syn 1.0.17 (registry+https://github.com/rust-lang/crates.io-index)", @@ -989,9 +989,9 @@ dependencies = [ [[package]] name = "lucet-wiggle-macro" -version = "0.6.2-dev" +version = "0.7.0-dev" dependencies = [ - "lucet-wiggle-generate 0.6.2-dev", + "lucet-wiggle-generate 0.7.0-dev", "quote 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)", "syn 1.0.17 (registry+https://github.com/rust-lang/crates.io-index)", "wiggle-generate 0.15.0", @@ -1000,7 +1000,7 @@ dependencies = [ [[package]] name = "lucetc" -version = "0.6.2-dev" +version = "0.7.0-dev" dependencies = [ "anyhow 1.0.28 (registry+https://github.com/rust-lang/crates.io-index)", "bimap 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1017,9 +1017,9 @@ dependencies = [ "env_logger 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", "human-size 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", - "lucet-module 0.6.2-dev", - "lucet-validate 0.6.2-dev", - "lucet-wiggle 0.6.2-dev", + "lucet-module 0.7.0-dev", + "lucet-validate 0.7.0-dev", + "lucet-wiggle 0.7.0-dev", "memoffset 0.5.4 (registry+https://github.com/rust-lang/crates.io-index)", "minisign 0.5.15 (registry+https://github.com/rust-lang/crates.io-index)", "object 0.18.0 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/benchmarks/lucet-benchmarks/Cargo.toml b/benchmarks/lucet-benchmarks/Cargo.toml index 18094f8dd..703aea4d9 100644 --- a/benchmarks/lucet-benchmarks/Cargo.toml +++ b/benchmarks/lucet-benchmarks/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lucet-benchmarks" -version = "0.6.2-dev" +version = "0.7.0-dev" description = "Benchmarks for the Lucet runtime" homepage = "https://github.com/fastly/lucet" repository = "https://github.com/fastly/lucet" diff --git a/lucet-module/Cargo.toml b/lucet-module/Cargo.toml index 414324425..5b0f815d0 100644 --- a/lucet-module/Cargo.toml +++ b/lucet-module/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lucet-module" -version = "0.6.2-dev" +version = "0.7.0-dev" description = "A structured interface for Lucet modules" homepage = "https://github.com/fastly/lucet" repository = "https://github.com/fastly/lucet" diff --git a/lucet-module/src/functions.rs b/lucet-module/src/functions.rs index 134450513..99495dbe5 100644 --- a/lucet-module/src/functions.rs +++ b/lucet-module/src/functions.rs @@ -125,9 +125,15 @@ impl OwnedFunctionMetadata { } } +#[derive(Clone, Copy, Debug)] pub struct FunctionHandle { pub ptr: FunctionPointer, pub id: FunctionIndex, + /// This is `true` if and only if this handle was referenced through the start section. + /// + /// The same function may be referenced via other contexts, and will appear with `false` in this + /// field in those cases. + pub is_start_func: bool, } // The layout of this struct is very tightly coupled to lucetc's `write_function_manifest`! diff --git a/lucet-objdump/Cargo.toml b/lucet-objdump/Cargo.toml index 10aac027f..7f4384753 100644 --- a/lucet-objdump/Cargo.toml +++ b/lucet-objdump/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lucet-objdump" -version = "0.6.2-dev" +version = "0.7.0-dev" description = "Analyze object files emitted by the Lucet compiler" homepage = "https://github.com/fastly/lucet" repository = "https://github.com/fastly/lucet" @@ -13,7 +13,7 @@ edition = "2018" object = "0.18" byteorder="1.2.1" colored="1.8.0" -lucet-module = { path = "../lucet-module", version = "=0.6.2-dev" } +lucet-module = { path = "../lucet-module", version = "=0.7.0-dev" } [package.metadata.deb] name = "fst-lucet-objdump" diff --git a/lucet-runtime/Cargo.toml b/lucet-runtime/Cargo.toml index 5562620c0..1de7eed59 100644 --- a/lucet-runtime/Cargo.toml +++ b/lucet-runtime/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lucet-runtime" -version = "0.6.2-dev" +version = "0.7.0-dev" description = "Pure Rust runtime for Lucet WebAssembly toolchain" homepage = "https://github.com/fastly/lucet" repository = "https://github.com/fastly/lucet" @@ -11,17 +11,17 @@ edition = "2018" [dependencies] libc = "0.2.65" -lucet-runtime-internals = { path = "lucet-runtime-internals", version = "=0.6.2-dev" } -lucet-module = { path = "../lucet-module", version = "=0.6.2-dev" } +lucet-runtime-internals = { path = "lucet-runtime-internals", version = "=0.7.0-dev" } +lucet-module = { path = "../lucet-module", version = "=0.7.0-dev" } num-traits = "0.2" num-derive = "0.3.0" [dev-dependencies] byteorder = "1.2" lazy_static = "1.1" -lucetc = { path = "../lucetc", version = "=0.6.2-dev" } -lucet-runtime-tests = { path = "lucet-runtime-tests", version = "=0.6.2-dev" } -lucet-wasi-sdk = { path = "../lucet-wasi-sdk", version = "=0.6.2-dev" } +lucetc = { path = "../lucetc", version = "=0.7.0-dev" } +lucet-runtime-tests = { path = "lucet-runtime-tests", version = "=0.7.0-dev" } +lucet-wasi-sdk = { path = "../lucet-wasi-sdk", version = "=0.7.0-dev" } nix = "0.17" rayon = "1.0" tempfile = "3.0" diff --git a/lucet-runtime/include/lucet.h b/lucet-runtime/include/lucet.h index b360784ce..4d98151b8 100644 --- a/lucet-runtime/include/lucet.h +++ b/lucet-runtime/include/lucet.h @@ -33,6 +33,8 @@ uint32_t lucet_instance_heap_len(const struct lucet_instance *inst); void lucet_instance_release(struct lucet_instance *inst); +enum lucet_error lucet_instance_run_start(struct lucet_instance *inst); + enum lucet_error lucet_instance_reset(struct lucet_instance *inst); enum lucet_error lucet_instance_run(struct lucet_instance * inst, diff --git a/lucet-runtime/lucet-runtime-internals/Cargo.toml b/lucet-runtime/lucet-runtime-internals/Cargo.toml index edd72bdb5..04c9e4d15 100644 --- a/lucet-runtime/lucet-runtime-internals/Cargo.toml +++ b/lucet-runtime/lucet-runtime-internals/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lucet-runtime-internals" -version = "0.6.2-dev" +version = "0.7.0-dev" description = "Pure Rust runtime for Lucet WebAssembly toolchain (internals)" homepage = "https://github.com/fastly/lucet" repository = "https://github.com/fastly/lucet" @@ -10,8 +10,8 @@ authors = ["Lucet team "] edition = "2018" [dependencies] -lucet-module = { path = "../../lucet-module", version = "=0.6.2-dev" } -lucet-runtime-macros = { path = "../lucet-runtime-macros", version = "=0.6.2-dev" } +lucet-module = { path = "../../lucet-module", version = "=0.7.0-dev" } +lucet-runtime-macros = { path = "../lucet-runtime-macros", version = "=0.7.0-dev" } anyhow = "1.0" bitflags = "1.0" diff --git a/lucet-runtime/lucet-runtime-internals/src/c_api.rs b/lucet-runtime/lucet-runtime-internals/src/c_api.rs index 1afa33301..9f9701684 100644 --- a/lucet-runtime/lucet-runtime-internals/src/c_api.rs +++ b/lucet-runtime/lucet-runtime-internals/src/c_api.rs @@ -78,6 +78,7 @@ pub enum lucet_error { Dl, InstanceNotReturned, InstanceNotYielded, + InstanceNeedsStart, StartYielded, Internal, Unsupported, @@ -104,6 +105,7 @@ impl From<&Error> for lucet_error { Error::DlError(_) => lucet_error::Dl, Error::InstanceNotReturned => lucet_error::InstanceNotReturned, Error::InstanceNotYielded => lucet_error::InstanceNotYielded, + Error::InstanceNeedsStart => lucet_error::InstanceNeedsStart, Error::StartYielded => lucet_error::StartYielded, Error::InternalError(_) => lucet_error::Internal, Error::Unsupported(_) => lucet_error::Unsupported, diff --git a/lucet-runtime/lucet-runtime-internals/src/error.rs b/lucet-runtime/lucet-runtime-internals/src/error.rs index 0e9e44afb..c878a1e80 100644 --- a/lucet-runtime/lucet-runtime-internals/src/error.rs +++ b/lucet-runtime/lucet-runtime-internals/src/error.rs @@ -55,6 +55,9 @@ pub enum Error { #[error("Instance not yielded")] InstanceNotYielded, + #[error("`Instance::run_start()` must be run before running other exported functions")] + InstanceNeedsStart, + #[error("Start function yielded")] StartYielded, diff --git a/lucet-runtime/lucet-runtime-internals/src/instance.rs b/lucet-runtime/lucet-runtime-internals/src/instance.rs index 1d0e65da5..151983f30 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance.rs @@ -11,7 +11,7 @@ use crate::alloc::{Alloc, HOST_PAGE_SIZE_EXPECTED}; use crate::context::Context; use crate::embed_ctx::CtxMap; use crate::error::Error; -use crate::module::{self, FunctionHandle, FunctionPointer, Global, GlobalValue, Module, TrapCode}; +use crate::module::{self, FunctionHandle, Global, GlobalValue, Module, TrapCode}; use crate::region::RegionInternal; use crate::val::{UntypedRetVal, Val}; use crate::WASM_PAGE_SIZE; @@ -67,11 +67,6 @@ unsafe impl Send for InstanceHandle {} /// /// This is not meant for public consumption, but rather is used to make implementations of /// `Region`. -/// -/// # Safety -/// -/// This function runs the guest code for the WebAssembly `start` section, and running any guest -/// code is potentially unsafe; see [`Instance::run()`](struct.Instance.html#method.run). pub fn new_instance_handle( instance: *mut Instance, module: Arc, @@ -256,8 +251,8 @@ pub struct Instance { /// Whether to install an alternate signal stack while the instance is running. ensure_sigstack_installed: bool, - /// Pointer to the function used as the entrypoint (for use in backtraces) - entrypoint: Option, + /// Pointer to the function used as the entrypoint. + entrypoint: Option, /// The value passed back to the guest when resuming a yielded instance. pub(crate) resumed_val: Option>, @@ -530,9 +525,43 @@ impl Instance { self.swap_and_return() } + /// Run the module's [start function][start], if one exists. + /// + /// If there is no start function in the module, this does nothing. + /// + /// If the module contains a start function, you must run it before running any other exported + /// functions. If an instance is reset, you must run the start function again. + /// + /// Start functions may assume that Wasm tables and memories are properly initialized, but may + /// not assume that imported functions or globals are available. + /// + /// # Errors + /// + /// In addition to the errors that can be returned from [`Instance::run()`][run], this can also + /// return `Error::StartYielded` if the start function attempts to yield. This should not arise + /// as long as the start function does not attempt to use any imported functions. + /// + /// # Safety + /// + /// The foreign code safety caveat of [`Instance::run()`][run] + /// applies. + /// + /// [run]: struct.Instance.html#method.run + /// [start]: https://webassembly.github.io/spec/core/syntax/modules.html#syntax-start + pub fn run_start(&mut self) -> Result<(), Error> { + if let Some(start) = self.module.get_start_func()? { + let res = self.run_func(start, &[])?; + if res.is_yielded() { + return Err(Error::StartYielded); + } + } + Ok(()) + } + /// Reset the instance's heap and global variables to their initial state. /// - /// The WebAssembly `start` section will also be run, if one exists. + /// The WebAssembly `start` section, if present, will need to be re-run with + /// [`Instance::run_start()`][run_start] before running any other exported functions. /// /// The embedder contexts present at instance creation or added with /// [`Instance::insert_embed_ctx()`](struct.Instance.html#method.insert_embed_ctx) are not @@ -544,10 +573,7 @@ impl Instance { /// It is the embedder's responsibility to initialize new `KillSwitch`es after resetting an /// instance. /// - /// # Safety - /// - /// This function runs the guest code for the WebAssembly `start` section, and running any - /// guest code is potentially unsafe; see [`Instance::run()`](struct.Instance.html#method.run). + /// [run_start]: struct.Instance.html#method.run pub fn reset(&mut self) -> Result<(), Error> { self.alloc.reset_heap(self.module.as_ref())?; let globals = unsafe { self.alloc.globals_mut() }; @@ -564,9 +590,13 @@ impl Instance { }; } - self.state = State::Ready; + if self.module.get_start_func()?.is_some() { + self.state = State::NotStarted; + } else { + self.state = State::Ready; + } + self.kill_state = Arc::new(KillState::new()); - self.run_start()?; Ok(()) } @@ -637,11 +667,6 @@ impl Instance { /// Insert a context value. /// /// If a context value of the same type already existed, it is returned. - /// - /// **Note**: this method is intended for embedder contexts that need to be added _after_ an - /// instance is created and initialized. To add a context for an instance's entire lifetime, - /// including the execution of its `start` section, see - /// [`Region::new_instance_builder()`](trait.Region.html#method.new_instance_builder). pub fn insert_embed_ctx(&mut self, x: T) -> Option { self.embed_ctx.insert(x) } @@ -750,6 +775,10 @@ impl Instance { KillSwitch::new(Arc::downgrade(&self.kill_state)) } + pub fn is_not_started(&self) -> bool { + self.state.is_not_started() + } + pub fn is_ready(&self) -> bool { self.state.is_ready() } @@ -869,9 +898,17 @@ impl Instance { /// Run a function in guest context at the given entrypoint. fn run_func(&mut self, func: FunctionHandle, args: &[Val]) -> Result { - if !(self.state.is_ready() || (self.state.is_faulted() && !self.state.is_fatal())) { + let needs_start = self.state.is_not_started() && !func.is_start_func; + if needs_start { + return Err(Error::InstanceNeedsStart); + } + + let is_ready = self.state.is_ready(); + let is_starting = self.state.is_not_started() && func.is_start_func; + let is_non_fatally_faulted = self.state.is_faulted() && !self.state.is_fatal(); + if !(is_ready || is_starting || is_non_fatally_faulted) { return Err(Error::InvalidArgument( - "instance must be ready or non-fatally faulted", + "instance must be ready, starting, or non-fatally faulted", )); } if func.ptr.as_usize() == 0 { @@ -899,7 +936,7 @@ impl Instance { } } - self.entrypoint = Some(func.ptr); + self.entrypoint = Some(func); let mut args_with_vmctx = vec![Val::from(self.alloc.slot().heap)]; args_with_vmctx.extend_from_slice(args); @@ -954,11 +991,17 @@ impl Instance { /// The core routine for context switching into a guest, and extracting a result. /// - /// This must only be called for an instance in a ready, non-fatally faulted, or yielded - /// state. The public wrappers around this function should make sure the state is appropriate. + /// This must only be called for an instance in a ready, non-fatally faulted, or yielded state, + /// or in the not-started state on the start function. The public wrappers around this function + /// should make sure the state is appropriate. fn swap_and_return(&mut self) -> Result { + let is_start_func = self + .entrypoint + .expect("we always have an entrypoint by now") + .is_start_func; debug_assert!( self.state.is_ready() + || self.state.is_not_started() && is_start_func || (self.state.is_faulted() && !self.state.is_fatal()) || self.state.is_yielded() ); @@ -981,15 +1024,19 @@ impl Instance { if let Err(e) = res { // Something went wrong setting up or tearing down the signal handlers and signal // stack. This is an error, but we don't want it to mask an error that may have arisen - // due to a guest fault or guest termination. So, we set the state back to `Ready` only - // if it is still `Running`, which likely indicates we never even made it into the - // guest. + // due to a guest fault or guest termination. So, we set the state back to `Ready` or + // `NotStarted` only if it is still `Running`, which likely indicates we never even made + // it into the guest. // // As of 2020-03-20, the only early return points in the code above happen before the - // guest would be able to run, so this should always transition from running to ready if - // there's an error. + // guest would be able to run, so this should always transition from running to + // ready or not started if there's an error. if let State::Running = self.state { - self.state = State::Ready; + if is_start_func { + self.state = State::NotStarted; + } else { + self.state = State::Ready; + } } return Err(e); } @@ -1068,9 +1115,14 @@ impl Instance { Err(Error::RuntimeFault(details)) } } - State::Ready | State::Terminated | State::Yielded { .. } | State::Transitioning => Err( - lucet_format_err!("\"impossible\" state found in `swap_and_return()`: {}", st), - ), + State::NotStarted + | State::Ready + | State::Terminated + | State::Yielded { .. } + | State::Transitioning => Err(lucet_format_err!( + "\"impossible\" state found in `swap_and_return()`: {}", + st + )), } } @@ -1096,16 +1148,6 @@ impl Instance { res } - - fn run_start(&mut self) -> Result<(), Error> { - if let Some(start) = self.module.get_start_func()? { - let res = self.run_func(start, &[])?; - if res.is_yielded() { - return Err(Error::StartYielded); - } - } - Ok(()) - } } /// Information about a runtime fault. diff --git a/lucet-runtime/lucet-runtime-internals/src/instance/state.rs b/lucet-runtime/lucet-runtime-internals/src/instance/state.rs index b1811b7ef..bb77d52da 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance/state.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance/state.rs @@ -7,6 +7,11 @@ use std::ffi::{CStr, CString}; /// The representation of a Lucet instance's state machine. pub enum State { + /// The instance is freshly-created, but needs to run the start section before running other entrypoints. + /// + /// Transitions to `Running` when the start section is run, and then to `Ready` if it succeeds. + NotStarted, + /// The instance is ready to run. /// /// Transitions to `Running` when the instance is run, or to `Ready` when it's reset. @@ -21,7 +26,8 @@ pub enum State { /// The instance has faulted, potentially fatally. /// /// Transitions to `Faulted` when filling in additional fault details, to `Running` if - /// re-running a non-fatally faulted instance, or to `Ready` when the instance is reset. + /// re-running a non-fatally faulted instance, or to `Ready` or `NotStarted` when the instance + /// is reset. Faulted { details: FaultDetails, siginfo: libc::siginfo_t, @@ -36,7 +42,7 @@ pub enum State { /// The instance has terminated, and must be reset before running again. /// - /// Transitions to `Ready` if the instance is reset. + /// Transitions to `Ready` or `NotStarted` if the instance is reset. Terminated, /// The instance is in the process of yielding. @@ -54,7 +60,8 @@ pub enum State { /// The instance has yielded. /// - /// Transitions to `Running` if the instance is resumed, or to `Ready` if the instance is reset. + /// Transitions to `Running` if the instance is resumed, or to `Ready` or `NotStarted` if the + /// instance is reset. Yielded { /// A phantom value carrying the type of the expected resumption value. /// @@ -76,6 +83,7 @@ pub enum State { impl std::fmt::Display for State { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { + State::NotStarted => write!(f, "not started"), State::Ready => write!(f, "ready"), State::Running => write!(f, "running"), State::Faulted { @@ -113,6 +121,14 @@ impl std::fmt::Display for State { } impl State { + pub fn is_not_started(&self) -> bool { + if let State::NotStarted { .. } = self { + true + } else { + false + } + } + pub fn is_ready(&self) -> bool { if let State::Ready { .. } = self { true diff --git a/lucet-runtime/lucet-runtime-internals/src/module.rs b/lucet-runtime/lucet-runtime-internals/src/module.rs index d08ad2a8d..a171954b6 100644 --- a/lucet-runtime/lucet-runtime-internals/src/module.rs +++ b/lucet-runtime/lucet-runtime-internals/src/module.rs @@ -80,7 +80,11 @@ pub trait ModuleInternal: Send + Sync { .map(|(fn_id, _)| FunctionIndex::from_u32(fn_id as u32)) .expect("valid function pointer"); - FunctionHandle { ptr, id } + FunctionHandle { + ptr, + id, + is_start_func: false, + } } /// Look up an instruction pointer in the trap manifest. diff --git a/lucet-runtime/lucet-runtime-internals/src/module/dl.rs b/lucet-runtime/lucet-runtime-internals/src/module/dl.rs index 89f4cd332..69e69de5d 100644 --- a/lucet-runtime/lucet-runtime-internals/src/module/dl.rs +++ b/lucet-runtime/lucet-runtime-internals/src/module/dl.rs @@ -249,7 +249,11 @@ impl ModuleInternal for DlModule { .ok_or_else(|| Error::SymbolNotFound(sym.to_string())) .map(|id| { let ptr = self.function_manifest()[id.as_u32() as usize].ptr(); - FunctionHandle { ptr, id } + FunctionHandle { + ptr, + id, + is_start_func: false, + } }) } @@ -274,9 +278,12 @@ impl ModuleInternal for DlModule { if start_func.is_null() { lucet_incorrect_module!("`guest_start` is defined but null"); } - Ok(Some(self.function_handle_from_ptr( - FunctionPointer::from_usize(unsafe { **start_func } as usize), - ))) + let mut func = self + .function_handle_from_ptr(FunctionPointer::from_usize( + unsafe { **start_func } as usize + )); + func.is_start_func = true; + Ok(Some(func)) } else { Ok(None) } diff --git a/lucet-runtime/lucet-runtime-internals/src/module/mock.rs b/lucet-runtime/lucet-runtime-internals/src/module/mock.rs index 91964e354..0f642bdec 100644 --- a/lucet-runtime/lucet-runtime-internals/src/module/mock.rs +++ b/lucet-runtime/lucet-runtime-internals/src/module/mock.rs @@ -308,9 +308,12 @@ impl ModuleInternal for MockModule { } fn get_start_func(&self) -> Result, Error> { - Ok(self - .start_func - .map(|start| self.function_handle_from_ptr(start))) + let func = self.start_func.map(|start| { + let mut func = self.function_handle_from_ptr(start); + func.is_start_func = true; + func + }); + Ok(func) } fn function_manifest(&self) -> &[FunctionSpec] { diff --git a/lucet-runtime/lucet-runtime-internals/src/region.rs b/lucet-runtime/lucet-runtime-internals/src/region.rs index 1f067497b..58a751b06 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region.rs @@ -19,11 +19,6 @@ pub trait Region: RegionInternal { /// Calling `region.new_instance(module)` is shorthand for /// `region.new_instance_builder(module).build()` for use when further customization is /// unnecessary. - /// - /// # Safety - /// - /// This function runs the guest code for the WebAssembly `start` section, and running any guest - /// code is potentially unsafe; see [`Instance::run()`](struct.Instance.html#method.run). fn new_instance(&self, module: Arc) -> Result { self.new_instance_builder(module).build() } @@ -126,11 +121,6 @@ impl<'a> InstanceBuilder<'a> { } /// Build the instance. - /// - /// # Safety - /// - /// This function runs the guest code for the WebAssembly `start` section, and running any guest - /// code is potentially unsafe; see [`Instance::run()`](struct.Instance.html#method.run). pub fn build(self) -> Result { self.region .new_instance_with(self.module, self.embed_ctx, self.heap_memory_size_limit) diff --git a/lucet-runtime/lucet-runtime-macros/Cargo.toml b/lucet-runtime/lucet-runtime-macros/Cargo.toml index 91b8f57a0..e93c72ea8 100644 --- a/lucet-runtime/lucet-runtime-macros/Cargo.toml +++ b/lucet-runtime/lucet-runtime-macros/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lucet-runtime-macros" -version = "0.6.2-dev" +version = "0.7.0-dev" description = "Macros for the Lucet WebAssembly runtime" homepage = "https://github.com/bytecodealliance/lucet" repository = "https://github.com/bytecodealliance/lucet" diff --git a/lucet-runtime/lucet-runtime-tests/Cargo.toml b/lucet-runtime/lucet-runtime-tests/Cargo.toml index 21df47f97..15fbc28cc 100644 --- a/lucet-runtime/lucet-runtime-tests/Cargo.toml +++ b/lucet-runtime/lucet-runtime-tests/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lucet-runtime-tests" -version = "0.6.2-dev" +version = "0.7.0-dev" description = "Pure Rust runtime for Lucet WebAssembly toolchain (tests)" homepage = "https://github.com/fastly/lucet" repository = "https://github.com/fastly/lucet" @@ -19,10 +19,10 @@ anyhow = "1" lazy_static = "1.1" libc = "0.2.65" tempfile = "3.0" -lucet-module = { path = "../../lucet-module", version = "=0.6.2-dev" } -lucet-runtime-internals = { path = "../lucet-runtime-internals", version = "=0.6.2-dev" } -lucet-wasi-sdk = { path = "../../lucet-wasi-sdk", version = "=0.6.2-dev" } -lucetc = { path = "../../lucetc", version = "=0.6.2-dev" } +lucet-module = { path = "../../lucet-module", version = "=0.7.0-dev" } +lucet-runtime-internals = { path = "../lucet-runtime-internals", version = "=0.7.0-dev" } +lucet-wasi-sdk = { path = "../../lucet-wasi-sdk", version = "=0.7.0-dev" } +lucetc = { path = "../../lucetc", version = "=0.7.0-dev" } [build-dependencies] cc = "1.0" diff --git a/lucet-runtime/lucet-runtime-tests/src/guest_fault.rs b/lucet-runtime/lucet-runtime-tests/src/guest_fault.rs index 98de17062..88616a964 100644 --- a/lucet-runtime/lucet-runtime-tests/src/guest_fault.rs +++ b/lucet-runtime/lucet-runtime-tests/src/guest_fault.rs @@ -1,9 +1,7 @@ #[macro_export] macro_rules! guest_fault_common_defs { () => { - use common::{ - mock_traps_module, with_unchanged_signal_handlers, HOSTCALL_TEST_ERROR, RECOVERABLE_PTR, - }; + use common::{mock_traps_module, HOSTCALL_TEST_ERROR, RECOVERABLE_PTR}; pub mod common { use lucet_module::{FunctionPointer, TrapCode, TrapSite}; use lucet_runtime::vmctx::{lucet_vmctx, Vmctx}; @@ -161,36 +159,6 @@ macro_rules! guest_fault_common_defs { )) .build() } - - pub fn with_unchanged_signal_handlers(f: F) { - fn get_handlers() -> Vec { - use libc::*; - use std::mem::MaybeUninit; - const SIGNALS: &'static [c_int] = &[SIGBUS, SIGFPE, SIGILL, SIGSEGV, SIGALRM]; - - SIGNALS - .iter() - .map(|sig| unsafe { - let mut out = MaybeUninit::::uninit(); - sigaction(*sig, std::ptr::null(), out.as_mut_ptr()); - out.assume_init() - }) - .collect() - } - - let before = get_handlers(); - - f(); - - let after = get_handlers(); - - for (before, after) in before.into_iter().zip(after.into_iter()) { - assert_eq!( - before.sa_sigaction, after.sa_sigaction, - "signal handlers match before and after" - ); - } - } } #[test] @@ -218,7 +186,8 @@ macro_rules! guest_fault_tests { use std::sync::{Arc, Mutex}; use $TestRegion as TestRegion; use $crate::helpers::{ - test_ex, test_nonex, FunctionPointer, MockExportBuilder, MockModuleBuilder, + test_ex, test_nonex, with_unchanged_signal_handlers, FunctionPointer, + MockExportBuilder, MockModuleBuilder, }; lazy_static! { diff --git a/lucet-runtime/lucet-runtime-tests/src/helpers.rs b/lucet-runtime/lucet-runtime-tests/src/helpers.rs index a73ee55a6..b135880dd 100644 --- a/lucet-runtime/lucet-runtime-tests/src/helpers.rs +++ b/lucet-runtime/lucet-runtime-tests/src/helpers.rs @@ -37,3 +37,33 @@ where drop(lock); r } + +pub fn with_unchanged_signal_handlers(f: F) { + fn get_handlers() -> Vec { + use libc::*; + use std::mem::MaybeUninit; + const SIGNALS: &'static [c_int] = &[SIGBUS, SIGFPE, SIGILL, SIGSEGV, SIGALRM]; + + SIGNALS + .iter() + .map(|sig| unsafe { + let mut out = MaybeUninit::::uninit(); + sigaction(*sig, std::ptr::null(), out.as_mut_ptr()); + out.assume_init() + }) + .collect() + } + + let before = get_handlers(); + + f(); + + let after = get_handlers(); + + for (before, after) in before.into_iter().zip(after.into_iter()) { + assert_eq!( + before.sa_sigaction, after.sa_sigaction, + "signal handlers match before and after" + ); + } +} diff --git a/lucet-runtime/lucet-runtime-tests/src/start.rs b/lucet-runtime/lucet-runtime-tests/src/start.rs index 45983aef6..91e94c331 100644 --- a/lucet-runtime/lucet-runtime-tests/src/start.rs +++ b/lucet-runtime/lucet-runtime-tests/src/start.rs @@ -1,10 +1,11 @@ #[macro_export] macro_rules! start_tests { ( $TestRegion:path ) => { - use lucet_runtime::{DlModule, Limits, Region}; + use lucet_runtime::{DlModule, Error, Limits, Region}; use std::sync::Arc; use $TestRegion as TestRegion; use $crate::build::test_module_wasm; + use $crate::helpers::{test_ex, with_unchanged_signal_handlers}; #[test] fn ensure_linked() { @@ -20,6 +21,7 @@ macro_rules! start_tests { .new_instance(module) .expect("instance can be created"); + inst.run_start().expect("start section runs"); inst.run("main", &[]).expect("instance runs"); // Now the globals should be: @@ -40,6 +42,7 @@ macro_rules! start_tests { .new_instance(module) .expect("instance can be created"); + inst.run_start().expect("start section runs"); inst.run("main", &[]).expect("instance runs"); // Now the globals should be: @@ -51,6 +54,53 @@ macro_rules! start_tests { assert_eq!(heap[0], 17); } + #[test] + fn start_is_required() { + let module = test_module_wasm("start", "start_and_call.wat") + .expect("module compiled and loaded"); + let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + + match inst.run("main", &[]).unwrap_err() { + Error::InstanceNeedsStart => (), + e => panic!("unexpected error: {}", e), + } + } + + #[test] + fn start_and_reset() { + let module = test_module_wasm("start", "start_and_call.wat") + .expect("module compiled and loaded"); + let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + + inst.run_start().expect("start section runs"); + inst.run("main", &[]).expect("instance runs"); + + // Now the globals should be: + // $flossie = 17 + // and heap should be: + // [0] = 17 + + let heap = inst.heap_u32(); + assert_eq!(heap[0], 17); + + inst.reset().expect("instance resets"); + + let heap = inst.heap_u32(); + assert_eq!(heap[0], 0); + + inst.run_start().expect("start section runs again"); + inst.run("main", &[]).expect("instance runs again"); + + let heap = inst.heap_u32(); + assert_eq!(heap[0], 17); + } + #[test] fn no_start() { let module = @@ -60,6 +110,91 @@ macro_rules! start_tests { .new_instance(module) .expect("instance can be created"); + inst.run_start().expect("start section doesn't run"); + inst.run("main", &[]).expect("instance runs"); + + // Now the globals should be: + // $flossie = 17 + // and heap should be: + // [0] = 17 + + let heap = inst.heap_u32(); + assert_eq!(heap[0], 17); + } + + #[test] + fn manual_signal_handler_ok() { + test_ex(|| { + with_unchanged_signal_handlers(|| { + let module = test_module_wasm("start", "start_and_call.wat") + .expect("module compiled and loaded"); + let region = + TestRegion::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + + inst.ensure_signal_handler_installed(false); + lucet_runtime::install_lucet_signal_handler(); + + inst.run_start().expect("start section runs"); + inst.run("main", &[]).expect("instance runs"); + + // Now the globals should be: + // $flossie = 17 + // and heap should be: + // [0] = 17 + + let heap = inst.heap_u32(); + assert_eq!(heap[0], 17); + + inst.reset().expect("instance resets"); + + let heap = inst.heap_u32(); + assert_eq!(heap[0], 0); + + inst.run_start().expect("start section runs again"); + inst.run("main", &[]).expect("instance runs again"); + + let heap = inst.heap_u32(); + assert_eq!(heap[0], 17); + + lucet_runtime::remove_lucet_signal_handler(); + }) + }); + } + + #[test] + fn manual_sigstack_ok() { + use libc::*; + use std::mem::MaybeUninit; + + let mut our_sigstack_alloc = vec![0; lucet_runtime::DEFAULT_SIGNAL_STACK_SIZE]; + let our_sigstack = stack_t { + ss_sp: our_sigstack_alloc.as_mut_ptr() as *mut _, + ss_flags: 0, + ss_size: lucet_runtime::DEFAULT_SIGNAL_STACK_SIZE, + }; + let mut beforestack = MaybeUninit::::uninit(); + let beforestack = unsafe { + sigaltstack(&our_sigstack, beforestack.as_mut_ptr()); + beforestack.assume_init() + }; + + let module = test_module_wasm("start", "start_and_call.wat") + .expect("module compiled and loaded"); + let limits_no_sigstack = Limits { + signal_stack_size: 0, + ..Limits::default() + }; + let region = TestRegion::create(1, &limits_no_sigstack).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + + inst.ensure_sigstack_installed(false); + + inst.run_start().expect("start section runs"); inst.run("main", &[]).expect("instance runs"); // Now the globals should be: @@ -69,6 +204,25 @@ macro_rules! start_tests { let heap = inst.heap_u32(); assert_eq!(heap[0], 17); + + inst.reset().expect("instance resets"); + + let heap = inst.heap_u32(); + assert_eq!(heap[0], 0); + + inst.run_start().expect("start section runs again"); + inst.run("main", &[]).expect("instance runs again"); + + let heap = inst.heap_u32(); + assert_eq!(heap[0], 17); + + let mut afterstack = MaybeUninit::::uninit(); + let afterstack = unsafe { + sigaltstack(&beforestack, afterstack.as_mut_ptr()); + afterstack.assume_init() + }; + + assert_eq!(afterstack.ss_sp, our_sigstack_alloc.as_mut_ptr() as *mut _); } }; } diff --git a/lucet-runtime/src/c_api.rs b/lucet-runtime/src/c_api.rs index e8efdd144..e4affef76 100644 --- a/lucet-runtime/src/c_api.rs +++ b/lucet-runtime/src/c_api.rs @@ -48,6 +48,7 @@ pub extern "C" fn lucet_error_name(e: c_int) -> *const c_char { Dl => "lucet_error_dl\0".as_ptr() as _, InstanceNotReturned => "lucet_error_instance_not_returned\0".as_ptr() as _, InstanceNotYielded => "lucet_error_instance_not_yielded\0".as_ptr() as _, + InstanceNeedsStart => "lucet_error_instance_needs_start\0".as_ptr() as _, StartYielded => "lucet_error_start_yielded\0".as_ptr() as _, Internal => "lucet_error_internal\0".as_ptr() as _, Unsupported => "lucet_error_unsupported\0".as_ptr() as _, @@ -242,6 +243,15 @@ pub unsafe extern "C" fn lucet_instance_resume( }) } +#[no_mangle] +pub unsafe extern "C" fn lucet_instance_run_start(inst: *mut lucet_instance) -> lucet_error { + with_instance_ptr!(inst, { + inst.run_start() + .map(|_| lucet_error::Ok) + .unwrap_or_else(|e| e.into()) + }) +} + #[no_mangle] pub unsafe extern "C" fn lucet_instance_reset(inst: *mut lucet_instance) -> lucet_error { with_instance_ptr!(inst, { diff --git a/lucet-spectest/Cargo.toml b/lucet-spectest/Cargo.toml index fece6a067..ef33fe3bb 100644 --- a/lucet-spectest/Cargo.toml +++ b/lucet-spectest/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lucet-spectest" -version = "0.6.2-dev" +version = "0.7.0-dev" description = "Test harness to run WebAssembly spec tests (.wast) against the Lucet toolchain" homepage = "https://github.com/fastly/lucet" repository = "https://github.com/fastly/lucet" @@ -17,9 +17,9 @@ name = "spec-test" path = "src/main.rs" [dependencies] -lucetc = { path = "../lucetc", version = "=0.6.2-dev" } -lucet-module = { path = "../lucet-module", version = "=0.6.2-dev" } -lucet-runtime = { path = "../lucet-runtime", version = "=0.6.2-dev" } +lucetc = { path = "../lucetc", version = "=0.7.0-dev" } +lucet-module = { path = "../lucet-module", version = "=0.7.0-dev" } +lucet-runtime = { path = "../lucet-runtime", version = "=0.7.0-dev" } wabt = "0.9.2" serde = "1.0" serde_json = "1.0" diff --git a/lucet-validate/Cargo.toml b/lucet-validate/Cargo.toml index d847555fb..f861f6907 100644 --- a/lucet-validate/Cargo.toml +++ b/lucet-validate/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lucet-validate" -version = "0.6.2-dev" +version = "0.7.0-dev" description = "Parse and validate webassembly files against witx interface" homepage = "https://github.com/fastly/lucet" repository = "https://github.com/fastly/lucet" @@ -24,8 +24,8 @@ thiserror = "1.0.4" wasmparser = "0.51.2" [dev-dependencies] -lucet-wasi = { path = "../lucet-wasi", version = "=0.6.2-dev" } -lucet-wasi-sdk = { path = "../lucet-wasi-sdk", version = "=0.6.2-dev" } +lucet-wasi = { path = "../lucet-wasi", version = "=0.7.0-dev" } +lucet-wasi-sdk = { path = "../lucet-wasi-sdk", version = "=0.7.0-dev" } tempfile = "3.0" wabt = "0.9.2" diff --git a/lucet-wasi-fuzz/Cargo.toml b/lucet-wasi-fuzz/Cargo.toml index 501a2040d..310a8bdab 100644 --- a/lucet-wasi-fuzz/Cargo.toml +++ b/lucet-wasi-fuzz/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lucet-wasi-fuzz" -version = "0.6.2-dev" +version = "0.7.0-dev" description = "Test the Lucet toolchain against native code execution using Csmith" homepage = "https://github.com/fastly/lucet" repository = "https://github.com/fastly/lucet" diff --git a/lucet-wasi-sdk/Cargo.toml b/lucet-wasi-sdk/Cargo.toml index 70f7acce1..7a76dded8 100644 --- a/lucet-wasi-sdk/Cargo.toml +++ b/lucet-wasi-sdk/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lucet-wasi-sdk" -version = "0.6.2-dev" +version = "0.7.0-dev" description = "A Rust interface to the wasi-sdk compiler and linker" homepage = "https://github.com/fastly/lucet" repository = "https://github.com/fastly/lucet" @@ -10,13 +10,13 @@ authors = ["Lucet team "] edition = "2018" [dependencies] -lucetc = { path = "../lucetc", version = "=0.6.2-dev" } -lucet-module = { path = "../lucet-module", version = "=0.6.2-dev" } +lucetc = { path = "../lucetc", version = "=0.7.0-dev" } +lucet-module = { path = "../lucet-module", version = "=0.7.0-dev" } tempfile = "3.0" thiserror = "1.0.4" [dev-dependencies] anyhow = "1" -lucet-validate = { path = "../lucet-validate", version = "=0.6.2-dev" } -lucet-wasi = { path = "../lucet-wasi", version = "=0.6.2-dev" } +lucet-validate = { path = "../lucet-validate", version = "=0.7.0-dev" } +lucet-wasi = { path = "../lucet-wasi", version = "=0.7.0-dev" } target-lexicon = "0.10" diff --git a/lucet-wasi/Cargo.toml b/lucet-wasi/Cargo.toml index 085ec69d2..11a418b2f 100644 --- a/lucet-wasi/Cargo.toml +++ b/lucet-wasi/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lucet-wasi" -version = "0.6.2-dev" +version = "0.7.0-dev" description = "Fastly's runtime for the WebAssembly System Interface (WASI)" homepage = "https://github.com/fastly/lucet" repository = "https://github.com/fastly/lucet" @@ -14,11 +14,11 @@ anyhow = "1" cast = "0.2" clap = "2.23" human-size = "0.4" -lucet-runtime = { path = "../lucet-runtime", version = "=0.6.2-dev" } -lucet-runtime-internals = { path = "../lucet-runtime/lucet-runtime-internals", version = "=0.6.2-dev" } -lucet-module = { path = "../lucet-module", version = "=0.6.2-dev" } -lucet-wasi-generate = { path = "./generate", version = "=0.6.2-dev" } -lucet-wiggle = { path = "../lucet-wiggle", version = "=0.6.2-dev" } +lucet-runtime = { path = "../lucet-runtime", version = "=0.7.0-dev" } +lucet-runtime-internals = { path = "../lucet-runtime/lucet-runtime-internals", version = "=0.7.0-dev" } +lucet-module = { path = "../lucet-module", version = "=0.7.0-dev" } +lucet-wasi-generate = { path = "./generate", version = "=0.7.0-dev" } +lucet-wiggle = { path = "../lucet-wiggle", version = "=0.7.0-dev" } libc = "0.2.65" nix = "0.17" rand = "0.6" diff --git a/lucet-wasi/generate/Cargo.toml b/lucet-wasi/generate/Cargo.toml index 7817888ba..1e82697ed 100644 --- a/lucet-wasi/generate/Cargo.toml +++ b/lucet-wasi/generate/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lucet-wasi-generate" -version = "0.6.2-dev" +version = "0.7.0-dev" homepage = "https://github.com/fastly/lucet" repository = "https://github.com/fastly/lucet" license = "Apache-2.0 WITH LLVM-exception" @@ -12,7 +12,7 @@ edition = "2018" proc-macro = true [dependencies] -lucet-wiggle = { path = "../../lucet-wiggle", version = "0.6.2-dev" } +lucet-wiggle = { path = "../../lucet-wiggle", version = "0.7.0-dev" } wasi-common = { path = "../../wasmtime/crates/wasi-common", version = "0.15.0", features = ["wiggle_metadata"] } wiggle-generate = { path = "../../wasmtime/crates/wiggle/generate", version = "0.15.0" } syn = { version = "1.0", features = ["full"] } diff --git a/lucet-wiggle/Cargo.toml b/lucet-wiggle/Cargo.toml index 06075eea4..ed8781b86 100644 --- a/lucet-wiggle/Cargo.toml +++ b/lucet-wiggle/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lucet-wiggle" -version = "0.6.2-dev" +version = "0.7.0-dev" homepage = "https://github.com/fastly/lucet" repository = "https://github.com/fastly/lucet" license = "Apache-2.0 WITH LLVM-exception" @@ -11,13 +11,13 @@ edition = "2018" [lib] [dependencies] -lucet-wiggle-macro = { path = "./macro", version = "0.6.2-dev" } -lucet-wiggle-generate = { path = "./generate", version = "0.6.2-dev" } -lucet-runtime = { path = "../lucet-runtime", version = "0.6.2-dev" } +lucet-wiggle-macro = { path = "./macro", version = "0.7.0-dev" } +lucet-wiggle-generate = { path = "./generate", version = "0.7.0-dev" } +lucet-runtime = { path = "../lucet-runtime", version = "0.7.0-dev" } wiggle = { path = "../wasmtime/crates/wiggle", version = "0.15.0" } [dev-dependencies] wiggle-test = { path = "../wasmtime/crates/wiggle/test-helpers" } tempfile = "3.1" -lucet-wasi-sdk = { path = "../lucet-wasi-sdk", version = "0.6.2-dev" } -lucetc = { path = "../lucetc", version = "0.6.2-dev" } +lucet-wasi-sdk = { path = "../lucet-wasi-sdk", version = "0.7.0-dev" } +lucetc = { path = "../lucetc", version = "0.7.0-dev" } diff --git a/lucet-wiggle/generate/Cargo.toml b/lucet-wiggle/generate/Cargo.toml index 22bd52e03..82e7dc4e4 100644 --- a/lucet-wiggle/generate/Cargo.toml +++ b/lucet-wiggle/generate/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lucet-wiggle-generate" -version = "0.6.2-dev" +version = "0.7.0-dev" homepage = "https://github.com/fastly/lucet" repository = "https://github.com/fastly/lucet" license = "Apache-2.0 WITH LLVM-exception" @@ -10,7 +10,7 @@ edition = "2018" [dependencies] wiggle-generate = { path = "../../wasmtime/crates/wiggle/generate", version = "0.15.0" } -lucet-module = { path = "../../lucet-module", version = "0.6.2-dev" } +lucet-module = { path = "../../lucet-module", version = "0.7.0-dev" } witx = { path = "../../wasmtime/crates/wasi-common/WASI/tools/witx", version = "0.8.4" } quote = "1.0" proc-macro2 = "1.0" diff --git a/lucet-wiggle/macro/Cargo.toml b/lucet-wiggle/macro/Cargo.toml index 8e11ce0c0..efa1056fa 100644 --- a/lucet-wiggle/macro/Cargo.toml +++ b/lucet-wiggle/macro/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lucet-wiggle-macro" -version = "0.6.2-dev" +version = "0.7.0-dev" homepage = "https://github.com/fastly/lucet" repository = "https://github.com/fastly/lucet" license = "Apache-2.0 WITH LLVM-exception" @@ -12,7 +12,7 @@ edition = "2018" proc-macro = true [dependencies] -lucet-wiggle-generate = { path = "../generate", version = "0.6.2-dev" } +lucet-wiggle-generate = { path = "../generate", version = "0.7.0-dev" } wiggle-generate = { path = "../../wasmtime/crates/wiggle/generate", version = "0.15.0" } witx = { path = "../../wasmtime/crates/wasi-common/WASI/tools/witx", version = "0.8.4" } syn = { version = "1.0", features = ["full"] } diff --git a/lucetc/Cargo.toml b/lucetc/Cargo.toml index 7a032d603..2f56c9b0f 100644 --- a/lucetc/Cargo.toml +++ b/lucetc/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lucetc" -version = "0.6.2-dev" +version = "0.7.0-dev" description = "Fastly's WebAssembly to native code compiler" homepage = "https://github.com/fastly/lucet" repository = "https://github.com/fastly/lucet" @@ -24,9 +24,9 @@ cranelift-module = { path = "../wasmtime/cranelift/module", version = "0.62.0" } cranelift-object = { path = "../wasmtime/cranelift/object", version = "0.62.0" } cranelift-wasm = { path = "../wasmtime/cranelift/wasm", version = "0.62.0" } target-lexicon = "0.10" -lucet-module = { path = "../lucet-module", version = "=0.6.2-dev" } -lucet-validate = { path = "../lucet-validate", version = "=0.6.2-dev" } -lucet-wiggle = { path = "../lucet-wiggle", version = "=0.6.2-dev" } +lucet-module = { path = "../lucet-module", version = "=0.7.0-dev" } +lucet-validate = { path = "../lucet-validate", version = "=0.7.0-dev" } +lucet-wiggle = { path = "../lucet-wiggle", version = "=0.7.0-dev" } wasmparser = "0.51.2" clap="2.32" log = "0.4" From 21a8ff3b3c92b44d9f4c6133f3302e58ee9b4462 Mon Sep 17 00:00:00 2001 From: "Adam C. Foltzer" Date: Thu, 23 Apr 2020 17:30:00 -0700 Subject: [PATCH 2/7] don't allow multiple `run_start()`s per instantiation/reset --- .../lucet-runtime-internals/src/c_api.rs | 2 ++ .../lucet-runtime-internals/src/error.rs | 3 +++ .../lucet-runtime-internals/src/instance.rs | 6 ++++++ lucet-runtime/lucet-runtime-tests/src/start.rs | 16 ++++++++++++++++ lucet-runtime/src/c_api.rs | 1 + 5 files changed, 28 insertions(+) diff --git a/lucet-runtime/lucet-runtime-internals/src/c_api.rs b/lucet-runtime/lucet-runtime-internals/src/c_api.rs index 9f9701684..e192d6bb5 100644 --- a/lucet-runtime/lucet-runtime-internals/src/c_api.rs +++ b/lucet-runtime/lucet-runtime-internals/src/c_api.rs @@ -79,6 +79,7 @@ pub enum lucet_error { InstanceNotReturned, InstanceNotYielded, InstanceNeedsStart, + StartAlreadyRun, StartYielded, Internal, Unsupported, @@ -106,6 +107,7 @@ impl From<&Error> for lucet_error { Error::InstanceNotReturned => lucet_error::InstanceNotReturned, Error::InstanceNotYielded => lucet_error::InstanceNotYielded, Error::InstanceNeedsStart => lucet_error::InstanceNeedsStart, + Error::StartAlreadyRun => lucet_error::StartAlreadyRun, Error::StartYielded => lucet_error::StartYielded, Error::InternalError(_) => lucet_error::Internal, Error::Unsupported(_) => lucet_error::Unsupported, diff --git a/lucet-runtime/lucet-runtime-internals/src/error.rs b/lucet-runtime/lucet-runtime-internals/src/error.rs index c878a1e80..2d10271b1 100644 --- a/lucet-runtime/lucet-runtime-internals/src/error.rs +++ b/lucet-runtime/lucet-runtime-internals/src/error.rs @@ -58,6 +58,9 @@ pub enum Error { #[error("`Instance::run_start()` must be run before running other exported functions")] InstanceNeedsStart, + #[error("`Instance::run_start()` called multiple times after a single instantiation or reset")] + StartAlreadyRun, + #[error("Start function yielded")] StartYielded, diff --git a/lucet-runtime/lucet-runtime-internals/src/instance.rs b/lucet-runtime/lucet-runtime-internals/src/instance.rs index 151983f30..9588db0bd 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance.rs @@ -541,6 +541,9 @@ impl Instance { /// return `Error::StartYielded` if the start function attempts to yield. This should not arise /// as long as the start function does not attempt to use any imported functions. /// + /// This also returns `Error::StartAlreadyRun` if the start function has already run since the + /// instance was created or last reset. + /// /// # Safety /// /// The foreign code safety caveat of [`Instance::run()`][run] @@ -550,6 +553,9 @@ impl Instance { /// [start]: https://webassembly.github.io/spec/core/syntax/modules.html#syntax-start pub fn run_start(&mut self) -> Result<(), Error> { if let Some(start) = self.module.get_start_func()? { + if !self.is_not_started() { + return Err(Error::StartAlreadyRun); + } let res = self.run_func(start, &[])?; if res.is_yielded() { return Err(Error::StartYielded); diff --git a/lucet-runtime/lucet-runtime-tests/src/start.rs b/lucet-runtime/lucet-runtime-tests/src/start.rs index 91e94c331..fe0e8d135 100644 --- a/lucet-runtime/lucet-runtime-tests/src/start.rs +++ b/lucet-runtime/lucet-runtime-tests/src/start.rs @@ -69,6 +69,22 @@ macro_rules! start_tests { } } + #[test] + fn no_start_without_reset() { + let module = test_module_wasm("start", "start_and_call.wat") + .expect("module compiled and loaded"); + let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + + inst.run_start().expect("start section runs"); + match inst.run_start().unwrap_err() { + Error::StartAlreadyRun => (), + e => panic!("unexpected error: {}", e), + } + } + #[test] fn start_and_reset() { let module = test_module_wasm("start", "start_and_call.wat") diff --git a/lucet-runtime/src/c_api.rs b/lucet-runtime/src/c_api.rs index e4affef76..2428636f9 100644 --- a/lucet-runtime/src/c_api.rs +++ b/lucet-runtime/src/c_api.rs @@ -49,6 +49,7 @@ pub extern "C" fn lucet_error_name(e: c_int) -> *const c_char { InstanceNotReturned => "lucet_error_instance_not_returned\0".as_ptr() as _, InstanceNotYielded => "lucet_error_instance_not_yielded\0".as_ptr() as _, InstanceNeedsStart => "lucet_error_instance_needs_start\0".as_ptr() as _, + StartAlreadyRun => "lucet_error_start_already_run\0".as_ptr() as _, StartYielded => "lucet_error_start_yielded\0".as_ptr() as _, Internal => "lucet_error_internal\0".as_ptr() as _, Unsupported => "lucet_error_unsupported\0".as_ptr() as _, From 50f808fd596982dc7f2805bd73176ec846adfe5f Mon Sep 17 00:00:00 2001 From: "Adam C. Foltzer" Date: Thu, 23 Apr 2020 18:05:35 -0700 Subject: [PATCH 3/7] terminate the guest if a start function tries to call an import func Thank you for the suggestion, @sunfishcode! --- CHANGELOG.md | 6 +++- lucet-runtime/include/lucet_types.h | 3 ++ .../lucet-runtime-internals/src/c_api.rs | 5 ++++ .../lucet-runtime-internals/src/instance.rs | 14 ++++----- .../src/instance/state.rs | 23 +++++++++++++-- .../lucet-runtime-internals/src/vmctx.rs | 4 +++ lucet-runtime/lucet-runtime-macros/src/lib.rs | 4 +++ .../guests/start/bindings.json | 6 +++- .../guests/start/call_import_func.wat | 11 +++++++ .../lucet-runtime-tests/src/start.rs | 29 ++++++++++++++++++- lucet-runtime/tests/start.rs | 3 +- 11 files changed, 94 insertions(+), 14 deletions(-) create mode 100644 lucet-runtime/lucet-runtime-tests/guests/start/call_import_func.wat diff --git a/CHANGELOG.md b/CHANGELOG.md index a2e983bce..3b0f1bd9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,10 +2,14 @@ - Added `install_lucet_signal_handler()` and `remove_lucet_signal_handler()`, along with `Instance::ensure_signal_handler_installed()` and `Instance::ensure_sigstack_installed()` options to control the automatic installation and removal of signal handlers and alternate signal stacks. The default behaviors have not changed. -- Added `Instance::run_start()` to the public API, which runs the Wasm start function if it is present in that instance's Wasm module. It does nothing if there is no start function. +- Added `Instance::run_start()` to the public API, which runs the [Wasm start function][start-function] if it is present in that instance's Wasm module. It does nothing if there is no start function. Creating or resetting an instance no longer implicitly runs the start function. Embedders must ensure that `run_start()` is called before calling any other exported functions. `Instance::run()` will now return `Err(Error::InstanceNeedsStart)` if the start function is present but hasn't been run since the instance was created or reset. +- Added a check that prevents [Wasm start functions][start-function] from calling imported functions. An instance that attempts to do so will fail with `Err(Error::RuntimeTerminated(TerminationDetails::StartCalledImportFunc))`. + +[start-function]: https://webassembly.github.io/spec/core/syntax/modules.html#syntax-start + ### 0.6.1 (2020-02-18) - Added metadata to compiled modules that record whether instruction counting instrumentation is present. diff --git a/lucet-runtime/include/lucet_types.h b/lucet-runtime/include/lucet_types.h index a634e448b..f14203f93 100644 --- a/lucet-runtime/include/lucet_types.h +++ b/lucet-runtime/include/lucet_types.h @@ -30,6 +30,8 @@ enum lucet_error { lucet_error_dl, lucet_error_instance_not_returned, lucet_error_instance_not_yielded, + lucet_error_instance_needs_start, + lucet_error_start_already_run, lucet_error_start_yielded, lucet_error_internal, lucet_error_unsupported, @@ -48,6 +50,7 @@ enum lucet_terminated_reason { lucet_terminated_reason_borrow_error, lucet_terminated_reason_provided, lucet_terminated_reason_remote, + lucet_terminated_reason_start_called_import_func, }; enum lucet_trapcode { diff --git a/lucet-runtime/lucet-runtime-internals/src/c_api.rs b/lucet-runtime/lucet-runtime-internals/src/c_api.rs index e192d6bb5..f2ac1cc3e 100644 --- a/lucet-runtime/lucet-runtime-internals/src/c_api.rs +++ b/lucet-runtime/lucet-runtime-internals/src/c_api.rs @@ -315,6 +315,10 @@ pub mod lucet_result { reason: lucet_terminated_reason::Remote, provided: std::ptr::null_mut(), }, + TerminationDetails::StartCalledImportFunc => lucet_terminated { + reason: lucet_terminated_reason::StartCalledImportFunc, + provided: std::ptr::null_mut(), + }, }, }, }, @@ -369,6 +373,7 @@ pub mod lucet_result { BorrowError, Provided, Remote, + StartCalledImportFunc, } #[repr(C)] diff --git a/lucet-runtime/lucet-runtime-internals/src/instance.rs b/lucet-runtime/lucet-runtime-internals/src/instance.rs index 9588db0bd..9e620e90d 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance.rs @@ -1011,7 +1011,7 @@ impl Instance { || (self.state.is_faulted() && !self.state.is_fatal()) || self.state.is_yielded() ); - self.state = State::Running; + self.state = State::Running { is_start_func }; self.kill_state.schedule(unsafe { pthread_self() }); @@ -1037,7 +1037,7 @@ impl Instance { // As of 2020-03-20, the only early return points in the code above happen before the // guest would be able to run, so this should always transition from running to // ready or not started if there's an error. - if let State::Running = self.state { + if let State::Running { is_start_func } = self.state { if is_start_func { self.state = State::NotStarted; } else { @@ -1068,7 +1068,7 @@ impl Instance { } match st { - State::Running => { + State::Running { .. } => { let retval = self.ctx.get_untyped_retval(); self.state = State::Ready; Ok(RunResult::Returned(retval)) @@ -1205,10 +1205,6 @@ impl std::fmt::Display for FaultDetails { } /// Information about a terminated guest. -/// -/// Guests are terminated either explicitly by `Vmctx::terminate()`, or implicitly by signal -/// handlers that return `SignalBehavior::Terminate`. It usually indicates that an unrecoverable -/// error has occurred in a hostcall, rather than in WebAssembly code. pub enum TerminationDetails { /// Returned when a signal handler terminates the instance. Signal, @@ -1226,7 +1222,10 @@ pub enum TerminationDetails { BorrowError(&'static str), /// Calls to `lucet_hostcall_terminate` provide a payload for use by the embedder. Provided(Box), + /// The instance was terminated by its `KillSwitch`. Remote, + /// The instance's start function attempted to call an import function. + StartCalledImportFunc, } impl TerminationDetails { @@ -1277,6 +1276,7 @@ impl std::fmt::Debug for TerminationDetails { TerminationDetails::YieldTypeMismatch => write!(f, "YieldTypeMismatch"), TerminationDetails::Provided(_) => write!(f, "Provided(Any)"), TerminationDetails::Remote => write!(f, "Remote"), + TerminationDetails::StartCalledImportFunc => write!(f, "StartCalledImportFunc"), } } } diff --git a/lucet-runtime/lucet-runtime-internals/src/instance/state.rs b/lucet-runtime/lucet-runtime-internals/src/instance/state.rs index bb77d52da..8ed4b6e36 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance/state.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance/state.rs @@ -21,7 +21,15 @@ pub enum State { /// /// Transitions to `Ready` when the guest function returns normally, or to `Faulted`, /// `Terminating`, or `Yielding` if the instance faults, terminates, or yields. - Running, + Running { + /// Indicates whether the instance is running due to `Instance::run_start()`. + /// + /// Certain elements of the runtime environment are not available during the start + /// function's execution, including imported functions. This flag allows us to detect when a + /// start function attempts to call an imported function, so that the instance can be + /// terminated. + is_start_func: bool, + }, /// The instance has faulted, potentially fatally. /// @@ -85,7 +93,8 @@ impl std::fmt::Display for State { match self { State::NotStarted => write!(f, "not started"), State::Ready => write!(f, "ready"), - State::Running => write!(f, "running"), + State::Running { is_start_func } if *is_start_func => write!(f, "running start func"), + State::Running { .. } => write!(f, "running"), State::Faulted { details, siginfo, .. } => { @@ -138,13 +147,21 @@ impl State { } pub fn is_running(&self) -> bool { - if let State::Running = self { + if let State::Running { .. } = self { true } else { false } } + pub fn is_running_start_func(&self) -> bool { + if let State::Running { is_start_func } = self { + *is_start_func + } else { + false + } + } + pub fn is_faulted(&self) -> bool { if let State::Faulted { .. } = self { true diff --git a/lucet-runtime/lucet-runtime-internals/src/vmctx.rs b/lucet-runtime/lucet-runtime-internals/src/vmctx.rs index b666f3574..a959e38c0 100644 --- a/lucet-runtime/lucet-runtime-internals/src/vmctx.rs +++ b/lucet-runtime/lucet-runtime-internals/src/vmctx.rs @@ -79,6 +79,10 @@ pub trait VmctxInternal { /// The dynamic type checks used by the other yield methods should make this explicit option /// type redundant, however this interface is used to avoid exposing a panic to the C API. fn yield_val_try_val(&mut self, val: A) -> Option; + + fn is_running_start_func(&self) -> bool { + self.instance().state.is_running_start_func() + } } impl VmctxInternal for Vmctx { diff --git a/lucet-runtime/lucet-runtime-macros/src/lib.rs b/lucet-runtime/lucet-runtime-macros/src/lib.rs index 6c42aa11b..d0ad5f91f 100644 --- a/lucet-runtime/lucet-runtime-macros/src/lib.rs +++ b/lucet-runtime/lucet-runtime-macros/src/lib.rs @@ -104,6 +104,10 @@ pub fn lucet_hostcall(_attr: TokenStream, item: TokenStream) -> TokenStream { #hostcall let mut vmctx = #vmctx_mod::Vmctx::from_raw(vmctx_raw); + // import functions are not available during start func execution + if #vmctx_mod::VmctxInternal::is_running_start_func(&vmctx) { + vmctx.terminate_no_unwind(#termination_details::StartCalledImportFunc); + } #vmctx_mod::VmctxInternal::instance_mut(&mut vmctx).uninterruptable(|| { let res = std::panic::catch_unwind(move || { #hostcall_ident(&mut #vmctx_mod::Vmctx::from_raw(vmctx_raw), #(#impl_args),*) diff --git a/lucet-runtime/lucet-runtime-tests/guests/start/bindings.json b/lucet-runtime/lucet-runtime-tests/guests/start/bindings.json index 0967ef424..2d24dc657 100644 --- a/lucet-runtime/lucet-runtime-tests/guests/start/bindings.json +++ b/lucet-runtime/lucet-runtime-tests/guests/start/bindings.json @@ -1 +1,5 @@ -{} +{ + "env": { + "not_allowed_during_start": "not_allowed_during_start" + } +} diff --git a/lucet-runtime/lucet-runtime-tests/guests/start/call_import_func.wat b/lucet-runtime/lucet-runtime-tests/guests/start/call_import_func.wat new file mode 100644 index 000000000..e98bb96e3 --- /dev/null +++ b/lucet-runtime/lucet-runtime-tests/guests/start/call_import_func.wat @@ -0,0 +1,11 @@ +;; sets the `start` section to a function that we also call from +;; normal user code + +(module + (type $v (func)) + (func $not_allowed_during_start (import "env" "not_allowed_during_start")) + (start $bad_start) + (func $bad_start (; 1 ;) (type $v) + (call $not_allowed_during_start) + ) +) diff --git a/lucet-runtime/lucet-runtime-tests/src/start.rs b/lucet-runtime/lucet-runtime-tests/src/start.rs index fe0e8d135..6532ec9d6 100644 --- a/lucet-runtime/lucet-runtime-tests/src/start.rs +++ b/lucet-runtime/lucet-runtime-tests/src/start.rs @@ -1,7 +1,19 @@ +#[macro_export] +macro_rules! start_common_defs { + () => { + use lucet_runtime::lucet_hostcall; + use lucet_runtime::vmctx::Vmctx; + + #[lucet_hostcall] + #[no_mangle] + pub fn not_allowed_during_start(_vmctx: &mut Vmctx) {} + }; +} + #[macro_export] macro_rules! start_tests { ( $TestRegion:path ) => { - use lucet_runtime::{DlModule, Error, Limits, Region}; + use lucet_runtime::{DlModule, Error, Limits, Region, TerminationDetails}; use std::sync::Arc; use $TestRegion as TestRegion; use $crate::build::test_module_wasm; @@ -240,5 +252,20 @@ macro_rules! start_tests { assert_eq!(afterstack.ss_sp, our_sigstack_alloc.as_mut_ptr() as *mut _); } + + #[test] + fn no_import_funcs_in_start() { + let module = test_module_wasm("start", "call_import_func.wat") + .expect("module compiled and loaded"); + let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + + match inst.run_start().unwrap_err() { + Error::RuntimeTerminated(TerminationDetails::StartCalledImportFunc) => (), + e => panic!("unexpected error: {}", e), + } + } }; } diff --git a/lucet-runtime/tests/start.rs b/lucet-runtime/tests/start.rs index 6b1f2a26c..fb1fccf18 100644 --- a/lucet-runtime/tests/start.rs +++ b/lucet-runtime/tests/start.rs @@ -1,3 +1,4 @@ -use lucet_runtime_tests::start_tests; +use lucet_runtime_tests::{start_common_defs, start_tests}; +start_common_defs!(); start_tests!(lucet_runtime::MmapRegion); From b152808110fabedb68555ee3aa3d4a40089bd3d7 Mon Sep 17 00:00:00 2001 From: "Adam C. Foltzer" Date: Thu, 23 Apr 2020 18:16:18 -0700 Subject: [PATCH 4/7] fix up doc about import func termination --- lucet-runtime/lucet-runtime-internals/src/instance.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/instance.rs b/lucet-runtime/lucet-runtime-internals/src/instance.rs index 9e620e90d..d671c7dd5 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance.rs @@ -525,7 +525,7 @@ impl Instance { self.swap_and_return() } - /// Run the module's [start function][start], if one exists. + /// Run the module's [Wasm start function][start], if one exists. /// /// If there is no start function in the module, this does nothing. /// @@ -541,9 +541,13 @@ impl Instance { /// return `Error::StartYielded` if the start function attempts to yield. This should not arise /// as long as the start function does not attempt to use any imported functions. /// - /// This also returns `Error::StartAlreadyRun` if the start function has already run since the + /// This returns `Error::StartAlreadyRun` if the start function has already run since the /// instance was created or last reset. /// + /// Wasm start functions are not allowed to call imported functions. If the start function + /// attempts to do so, the instance will be terminated with + /// `TerminationDetails::StartCalledImportFunc`. + /// /// # Safety /// /// The foreign code safety caveat of [`Instance::run()`][run] From fb0f840f17ed22214f4d652706f714ec1b81f1a1 Mon Sep 17 00:00:00 2001 From: "Adam C. Foltzer" Date: Thu, 23 Apr 2020 18:19:53 -0700 Subject: [PATCH 5/7] mark non-exclusive tests in the `start` suite properly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `test_ex` doesn't do much good if the non-exclusive tests aren't trying to take the read lock 😬 --- .../lucet-runtime-tests/src/start.rs | 352 ++++++++++-------- 1 file changed, 188 insertions(+), 164 deletions(-) diff --git a/lucet-runtime/lucet-runtime-tests/src/start.rs b/lucet-runtime/lucet-runtime-tests/src/start.rs index 6532ec9d6..6ffc19a40 100644 --- a/lucet-runtime/lucet-runtime-tests/src/start.rs +++ b/lucet-runtime/lucet-runtime-tests/src/start.rs @@ -17,7 +17,7 @@ macro_rules! start_tests { use std::sync::Arc; use $TestRegion as TestRegion; use $crate::build::test_module_wasm; - use $crate::helpers::{test_ex, with_unchanged_signal_handlers}; + use $crate::helpers::{test_ex, test_nonex, with_unchanged_signal_handlers}; #[test] fn ensure_linked() { @@ -26,128 +26,146 @@ macro_rules! start_tests { #[test] fn global_init() { - let module = - test_module_wasm("start", "global_init.wat").expect("module compiled and loaded"); - let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); - let mut inst = region - .new_instance(module) - .expect("instance can be created"); - - inst.run_start().expect("start section runs"); - inst.run("main", &[]).expect("instance runs"); - - // Now the globals should be: - // $flossie = 17 - // and heap should be: - // [0] = 17 - - let heap = inst.heap_u32(); - assert_eq!(heap[0], 17); + test_nonex(|| { + let module = test_module_wasm("start", "global_init.wat") + .expect("module compiled and loaded"); + let region = + TestRegion::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + + inst.run_start().expect("start section runs"); + inst.run("main", &[]).expect("instance runs"); + + // Now the globals should be: + // $flossie = 17 + // and heap should be: + // [0] = 17 + + let heap = inst.heap_u32(); + assert_eq!(heap[0], 17); + }); } #[test] fn start_and_call() { - let module = test_module_wasm("start", "start_and_call.wat") - .expect("module compiled and loaded"); - let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); - let mut inst = region - .new_instance(module) - .expect("instance can be created"); - - inst.run_start().expect("start section runs"); - inst.run("main", &[]).expect("instance runs"); - - // Now the globals should be: - // $flossie = 17 - // and heap should be: - // [0] = 17 - - let heap = inst.heap_u32(); - assert_eq!(heap[0], 17); + test_nonex(|| { + let module = test_module_wasm("start", "start_and_call.wat") + .expect("module compiled and loaded"); + let region = + TestRegion::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + + inst.run_start().expect("start section runs"); + inst.run("main", &[]).expect("instance runs"); + + // Now the globals should be: + // $flossie = 17 + // and heap should be: + // [0] = 17 + + let heap = inst.heap_u32(); + assert_eq!(heap[0], 17); + }); } #[test] fn start_is_required() { - let module = test_module_wasm("start", "start_and_call.wat") - .expect("module compiled and loaded"); - let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); - let mut inst = region - .new_instance(module) - .expect("instance can be created"); - - match inst.run("main", &[]).unwrap_err() { - Error::InstanceNeedsStart => (), - e => panic!("unexpected error: {}", e), - } + test_nonex(|| { + let module = test_module_wasm("start", "start_and_call.wat") + .expect("module compiled and loaded"); + let region = + TestRegion::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + + match inst.run("main", &[]).unwrap_err() { + Error::InstanceNeedsStart => (), + e => panic!("unexpected error: {}", e), + } + }); } #[test] fn no_start_without_reset() { - let module = test_module_wasm("start", "start_and_call.wat") - .expect("module compiled and loaded"); - let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); - let mut inst = region - .new_instance(module) - .expect("instance can be created"); - - inst.run_start().expect("start section runs"); - match inst.run_start().unwrap_err() { - Error::StartAlreadyRun => (), - e => panic!("unexpected error: {}", e), - } + test_nonex(|| { + let module = test_module_wasm("start", "start_and_call.wat") + .expect("module compiled and loaded"); + let region = + TestRegion::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + + inst.run_start().expect("start section runs"); + match inst.run_start().unwrap_err() { + Error::StartAlreadyRun => (), + e => panic!("unexpected error: {}", e), + } + }); } #[test] fn start_and_reset() { - let module = test_module_wasm("start", "start_and_call.wat") - .expect("module compiled and loaded"); - let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); - let mut inst = region - .new_instance(module) - .expect("instance can be created"); + test_nonex(|| { + let module = test_module_wasm("start", "start_and_call.wat") + .expect("module compiled and loaded"); + let region = + TestRegion::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); - inst.run_start().expect("start section runs"); - inst.run("main", &[]).expect("instance runs"); + inst.run_start().expect("start section runs"); + inst.run("main", &[]).expect("instance runs"); - // Now the globals should be: - // $flossie = 17 - // and heap should be: - // [0] = 17 + // Now the globals should be: + // $flossie = 17 + // and heap should be: + // [0] = 17 - let heap = inst.heap_u32(); - assert_eq!(heap[0], 17); + let heap = inst.heap_u32(); + assert_eq!(heap[0], 17); - inst.reset().expect("instance resets"); + inst.reset().expect("instance resets"); - let heap = inst.heap_u32(); - assert_eq!(heap[0], 0); + let heap = inst.heap_u32(); + assert_eq!(heap[0], 0); - inst.run_start().expect("start section runs again"); - inst.run("main", &[]).expect("instance runs again"); + inst.run_start().expect("start section runs again"); + inst.run("main", &[]).expect("instance runs again"); - let heap = inst.heap_u32(); - assert_eq!(heap[0], 17); + let heap = inst.heap_u32(); + assert_eq!(heap[0], 17); + }); } #[test] fn no_start() { - let module = - test_module_wasm("start", "no_start.wat").expect("module compiled and loaded"); - let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); - let mut inst = region - .new_instance(module) - .expect("instance can be created"); - - inst.run_start().expect("start section doesn't run"); - inst.run("main", &[]).expect("instance runs"); - - // Now the globals should be: - // $flossie = 17 - // and heap should be: - // [0] = 17 - - let heap = inst.heap_u32(); - assert_eq!(heap[0], 17); + test_nonex(|| { + let module = + test_module_wasm("start", "no_start.wat").expect("module compiled and loaded"); + let region = + TestRegion::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + + inst.run_start().expect("start section doesn't run"); + inst.run("main", &[]).expect("instance runs"); + + // Now the globals should be: + // $flossie = 17 + // and heap should be: + // [0] = 17 + + let heap = inst.heap_u32(); + assert_eq!(heap[0], 17); + }); } #[test] @@ -194,78 +212,84 @@ macro_rules! start_tests { #[test] fn manual_sigstack_ok() { - use libc::*; - use std::mem::MaybeUninit; - - let mut our_sigstack_alloc = vec![0; lucet_runtime::DEFAULT_SIGNAL_STACK_SIZE]; - let our_sigstack = stack_t { - ss_sp: our_sigstack_alloc.as_mut_ptr() as *mut _, - ss_flags: 0, - ss_size: lucet_runtime::DEFAULT_SIGNAL_STACK_SIZE, - }; - let mut beforestack = MaybeUninit::::uninit(); - let beforestack = unsafe { - sigaltstack(&our_sigstack, beforestack.as_mut_ptr()); - beforestack.assume_init() - }; - - let module = test_module_wasm("start", "start_and_call.wat") - .expect("module compiled and loaded"); - let limits_no_sigstack = Limits { - signal_stack_size: 0, - ..Limits::default() - }; - let region = TestRegion::create(1, &limits_no_sigstack).expect("region can be created"); - let mut inst = region - .new_instance(module) - .expect("instance can be created"); - - inst.ensure_sigstack_installed(false); - - inst.run_start().expect("start section runs"); - inst.run("main", &[]).expect("instance runs"); - - // Now the globals should be: - // $flossie = 17 - // and heap should be: - // [0] = 17 - - let heap = inst.heap_u32(); - assert_eq!(heap[0], 17); - - inst.reset().expect("instance resets"); - - let heap = inst.heap_u32(); - assert_eq!(heap[0], 0); - - inst.run_start().expect("start section runs again"); - inst.run("main", &[]).expect("instance runs again"); - - let heap = inst.heap_u32(); - assert_eq!(heap[0], 17); - - let mut afterstack = MaybeUninit::::uninit(); - let afterstack = unsafe { - sigaltstack(&beforestack, afterstack.as_mut_ptr()); - afterstack.assume_init() - }; - - assert_eq!(afterstack.ss_sp, our_sigstack_alloc.as_mut_ptr() as *mut _); + test_nonex(|| { + use libc::*; + use std::mem::MaybeUninit; + + let mut our_sigstack_alloc = vec![0; lucet_runtime::DEFAULT_SIGNAL_STACK_SIZE]; + let our_sigstack = stack_t { + ss_sp: our_sigstack_alloc.as_mut_ptr() as *mut _, + ss_flags: 0, + ss_size: lucet_runtime::DEFAULT_SIGNAL_STACK_SIZE, + }; + let mut beforestack = MaybeUninit::::uninit(); + let beforestack = unsafe { + sigaltstack(&our_sigstack, beforestack.as_mut_ptr()); + beforestack.assume_init() + }; + + let module = test_module_wasm("start", "start_and_call.wat") + .expect("module compiled and loaded"); + let limits_no_sigstack = Limits { + signal_stack_size: 0, + ..Limits::default() + }; + let region = + TestRegion::create(1, &limits_no_sigstack).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + + inst.ensure_sigstack_installed(false); + + inst.run_start().expect("start section runs"); + inst.run("main", &[]).expect("instance runs"); + + // Now the globals should be: + // $flossie = 17 + // and heap should be: + // [0] = 17 + + let heap = inst.heap_u32(); + assert_eq!(heap[0], 17); + + inst.reset().expect("instance resets"); + + let heap = inst.heap_u32(); + assert_eq!(heap[0], 0); + + inst.run_start().expect("start section runs again"); + inst.run("main", &[]).expect("instance runs again"); + + let heap = inst.heap_u32(); + assert_eq!(heap[0], 17); + + let mut afterstack = MaybeUninit::::uninit(); + let afterstack = unsafe { + sigaltstack(&beforestack, afterstack.as_mut_ptr()); + afterstack.assume_init() + }; + + assert_eq!(afterstack.ss_sp, our_sigstack_alloc.as_mut_ptr() as *mut _); + }); } #[test] fn no_import_funcs_in_start() { - let module = test_module_wasm("start", "call_import_func.wat") - .expect("module compiled and loaded"); - let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); - let mut inst = region - .new_instance(module) - .expect("instance can be created"); - - match inst.run_start().unwrap_err() { - Error::RuntimeTerminated(TerminationDetails::StartCalledImportFunc) => (), - e => panic!("unexpected error: {}", e), - } + test_nonex(|| { + let module = test_module_wasm("start", "call_import_func.wat") + .expect("module compiled and loaded"); + let region = + TestRegion::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + + match inst.run_start().unwrap_err() { + Error::RuntimeTerminated(TerminationDetails::StartCalledImportFunc) => (), + e => panic!("unexpected error: {}", e), + } + }); } }; } From 15c91841caf218b7a25c248bc7f62314ed173346 Mon Sep 17 00:00:00 2001 From: "Adam C. Foltzer" Date: Thu, 23 Apr 2020 18:32:29 -0700 Subject: [PATCH 6/7] explicitly run the start section in `lucet-wasi` --- lucet-wasi/src/main.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lucet-wasi/src/main.rs b/lucet-wasi/src/main.rs index e725b9772..34c7f95b3 100644 --- a/lucet-wasi/src/main.rs +++ b/lucet-wasi/src/main.rs @@ -258,6 +258,8 @@ fn run(config: Config<'_>) { }); } + inst.run_start().expect("Wasm start function runs"); + match inst.run(config.entrypoint, &[]) { // normal termination implies 0 exit code Ok(RunResult::Returned(_)) => 0, From 1e84d31d7118ae29acb7963fa03b37733a4a8f8c Mon Sep 17 00:00:00 2001 From: "Adam C. Foltzer" Date: Fri, 24 Apr 2020 08:57:24 -0700 Subject: [PATCH 7/7] Back out the termination on import func calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I was really tired yesterday, and misinterpreted @sunfishcode's comment 🙃 --- CHANGELOG.md | 2 -- lucet-runtime/include/lucet_types.h | 1 - .../lucet-runtime-internals/src/c_api.rs | 5 --- .../lucet-runtime-internals/src/instance.rs | 13 +++----- .../src/instance/state.rs | 23 ++----------- .../lucet-runtime-internals/src/vmctx.rs | 4 --- lucet-runtime/lucet-runtime-macros/src/lib.rs | 4 --- .../guests/start/bindings.json | 6 +--- .../guests/start/call_import_func.wat | 11 ------- .../lucet-runtime-tests/src/start.rs | 32 +------------------ lucet-runtime/tests/start.rs | 3 +- 11 files changed, 11 insertions(+), 93 deletions(-) delete mode 100644 lucet-runtime/lucet-runtime-tests/guests/start/call_import_func.wat diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b0f1bd9d..a8604685d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,6 @@ Creating or resetting an instance no longer implicitly runs the start function. Embedders must ensure that `run_start()` is called before calling any other exported functions. `Instance::run()` will now return `Err(Error::InstanceNeedsStart)` if the start function is present but hasn't been run since the instance was created or reset. -- Added a check that prevents [Wasm start functions][start-function] from calling imported functions. An instance that attempts to do so will fail with `Err(Error::RuntimeTerminated(TerminationDetails::StartCalledImportFunc))`. - [start-function]: https://webassembly.github.io/spec/core/syntax/modules.html#syntax-start ### 0.6.1 (2020-02-18) diff --git a/lucet-runtime/include/lucet_types.h b/lucet-runtime/include/lucet_types.h index f14203f93..fd4619aaf 100644 --- a/lucet-runtime/include/lucet_types.h +++ b/lucet-runtime/include/lucet_types.h @@ -50,7 +50,6 @@ enum lucet_terminated_reason { lucet_terminated_reason_borrow_error, lucet_terminated_reason_provided, lucet_terminated_reason_remote, - lucet_terminated_reason_start_called_import_func, }; enum lucet_trapcode { diff --git a/lucet-runtime/lucet-runtime-internals/src/c_api.rs b/lucet-runtime/lucet-runtime-internals/src/c_api.rs index f2ac1cc3e..e192d6bb5 100644 --- a/lucet-runtime/lucet-runtime-internals/src/c_api.rs +++ b/lucet-runtime/lucet-runtime-internals/src/c_api.rs @@ -315,10 +315,6 @@ pub mod lucet_result { reason: lucet_terminated_reason::Remote, provided: std::ptr::null_mut(), }, - TerminationDetails::StartCalledImportFunc => lucet_terminated { - reason: lucet_terminated_reason::StartCalledImportFunc, - provided: std::ptr::null_mut(), - }, }, }, }, @@ -373,7 +369,6 @@ pub mod lucet_result { BorrowError, Provided, Remote, - StartCalledImportFunc, } #[repr(C)] diff --git a/lucet-runtime/lucet-runtime-internals/src/instance.rs b/lucet-runtime/lucet-runtime-internals/src/instance.rs index d671c7dd5..480513c3c 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance.rs @@ -525,7 +525,7 @@ impl Instance { self.swap_and_return() } - /// Run the module's [Wasm start function][start], if one exists. + /// Run the module's [start function][start], if one exists. /// /// If there is no start function in the module, this does nothing. /// @@ -541,7 +541,7 @@ impl Instance { /// return `Error::StartYielded` if the start function attempts to yield. This should not arise /// as long as the start function does not attempt to use any imported functions. /// - /// This returns `Error::StartAlreadyRun` if the start function has already run since the + /// This also returns `Error::StartAlreadyRun` if the start function has already run since the /// instance was created or last reset. /// /// Wasm start functions are not allowed to call imported functions. If the start function @@ -1015,7 +1015,7 @@ impl Instance { || (self.state.is_faulted() && !self.state.is_fatal()) || self.state.is_yielded() ); - self.state = State::Running { is_start_func }; + self.state = State::Running; self.kill_state.schedule(unsafe { pthread_self() }); @@ -1041,7 +1041,7 @@ impl Instance { // As of 2020-03-20, the only early return points in the code above happen before the // guest would be able to run, so this should always transition from running to // ready or not started if there's an error. - if let State::Running { is_start_func } = self.state { + if let State::Running = self.state { if is_start_func { self.state = State::NotStarted; } else { @@ -1072,7 +1072,7 @@ impl Instance { } match st { - State::Running { .. } => { + State::Running => { let retval = self.ctx.get_untyped_retval(); self.state = State::Ready; Ok(RunResult::Returned(retval)) @@ -1228,8 +1228,6 @@ pub enum TerminationDetails { Provided(Box), /// The instance was terminated by its `KillSwitch`. Remote, - /// The instance's start function attempted to call an import function. - StartCalledImportFunc, } impl TerminationDetails { @@ -1280,7 +1278,6 @@ impl std::fmt::Debug for TerminationDetails { TerminationDetails::YieldTypeMismatch => write!(f, "YieldTypeMismatch"), TerminationDetails::Provided(_) => write!(f, "Provided(Any)"), TerminationDetails::Remote => write!(f, "Remote"), - TerminationDetails::StartCalledImportFunc => write!(f, "StartCalledImportFunc"), } } } diff --git a/lucet-runtime/lucet-runtime-internals/src/instance/state.rs b/lucet-runtime/lucet-runtime-internals/src/instance/state.rs index 8ed4b6e36..bb77d52da 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance/state.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance/state.rs @@ -21,15 +21,7 @@ pub enum State { /// /// Transitions to `Ready` when the guest function returns normally, or to `Faulted`, /// `Terminating`, or `Yielding` if the instance faults, terminates, or yields. - Running { - /// Indicates whether the instance is running due to `Instance::run_start()`. - /// - /// Certain elements of the runtime environment are not available during the start - /// function's execution, including imported functions. This flag allows us to detect when a - /// start function attempts to call an imported function, so that the instance can be - /// terminated. - is_start_func: bool, - }, + Running, /// The instance has faulted, potentially fatally. /// @@ -93,8 +85,7 @@ impl std::fmt::Display for State { match self { State::NotStarted => write!(f, "not started"), State::Ready => write!(f, "ready"), - State::Running { is_start_func } if *is_start_func => write!(f, "running start func"), - State::Running { .. } => write!(f, "running"), + State::Running => write!(f, "running"), State::Faulted { details, siginfo, .. } => { @@ -147,21 +138,13 @@ impl State { } pub fn is_running(&self) -> bool { - if let State::Running { .. } = self { + if let State::Running = self { true } else { false } } - pub fn is_running_start_func(&self) -> bool { - if let State::Running { is_start_func } = self { - *is_start_func - } else { - false - } - } - pub fn is_faulted(&self) -> bool { if let State::Faulted { .. } = self { true diff --git a/lucet-runtime/lucet-runtime-internals/src/vmctx.rs b/lucet-runtime/lucet-runtime-internals/src/vmctx.rs index a959e38c0..b666f3574 100644 --- a/lucet-runtime/lucet-runtime-internals/src/vmctx.rs +++ b/lucet-runtime/lucet-runtime-internals/src/vmctx.rs @@ -79,10 +79,6 @@ pub trait VmctxInternal { /// The dynamic type checks used by the other yield methods should make this explicit option /// type redundant, however this interface is used to avoid exposing a panic to the C API. fn yield_val_try_val(&mut self, val: A) -> Option; - - fn is_running_start_func(&self) -> bool { - self.instance().state.is_running_start_func() - } } impl VmctxInternal for Vmctx { diff --git a/lucet-runtime/lucet-runtime-macros/src/lib.rs b/lucet-runtime/lucet-runtime-macros/src/lib.rs index d0ad5f91f..6c42aa11b 100644 --- a/lucet-runtime/lucet-runtime-macros/src/lib.rs +++ b/lucet-runtime/lucet-runtime-macros/src/lib.rs @@ -104,10 +104,6 @@ pub fn lucet_hostcall(_attr: TokenStream, item: TokenStream) -> TokenStream { #hostcall let mut vmctx = #vmctx_mod::Vmctx::from_raw(vmctx_raw); - // import functions are not available during start func execution - if #vmctx_mod::VmctxInternal::is_running_start_func(&vmctx) { - vmctx.terminate_no_unwind(#termination_details::StartCalledImportFunc); - } #vmctx_mod::VmctxInternal::instance_mut(&mut vmctx).uninterruptable(|| { let res = std::panic::catch_unwind(move || { #hostcall_ident(&mut #vmctx_mod::Vmctx::from_raw(vmctx_raw), #(#impl_args),*) diff --git a/lucet-runtime/lucet-runtime-tests/guests/start/bindings.json b/lucet-runtime/lucet-runtime-tests/guests/start/bindings.json index 2d24dc657..0967ef424 100644 --- a/lucet-runtime/lucet-runtime-tests/guests/start/bindings.json +++ b/lucet-runtime/lucet-runtime-tests/guests/start/bindings.json @@ -1,5 +1 @@ -{ - "env": { - "not_allowed_during_start": "not_allowed_during_start" - } -} +{} diff --git a/lucet-runtime/lucet-runtime-tests/guests/start/call_import_func.wat b/lucet-runtime/lucet-runtime-tests/guests/start/call_import_func.wat deleted file mode 100644 index e98bb96e3..000000000 --- a/lucet-runtime/lucet-runtime-tests/guests/start/call_import_func.wat +++ /dev/null @@ -1,11 +0,0 @@ -;; sets the `start` section to a function that we also call from -;; normal user code - -(module - (type $v (func)) - (func $not_allowed_during_start (import "env" "not_allowed_during_start")) - (start $bad_start) - (func $bad_start (; 1 ;) (type $v) - (call $not_allowed_during_start) - ) -) diff --git a/lucet-runtime/lucet-runtime-tests/src/start.rs b/lucet-runtime/lucet-runtime-tests/src/start.rs index 6ffc19a40..cf6a006ce 100644 --- a/lucet-runtime/lucet-runtime-tests/src/start.rs +++ b/lucet-runtime/lucet-runtime-tests/src/start.rs @@ -1,19 +1,7 @@ -#[macro_export] -macro_rules! start_common_defs { - () => { - use lucet_runtime::lucet_hostcall; - use lucet_runtime::vmctx::Vmctx; - - #[lucet_hostcall] - #[no_mangle] - pub fn not_allowed_during_start(_vmctx: &mut Vmctx) {} - }; -} - #[macro_export] macro_rules! start_tests { ( $TestRegion:path ) => { - use lucet_runtime::{DlModule, Error, Limits, Region, TerminationDetails}; + use lucet_runtime::{DlModule, Error, Limits, Region}; use std::sync::Arc; use $TestRegion as TestRegion; use $crate::build::test_module_wasm; @@ -273,23 +261,5 @@ macro_rules! start_tests { assert_eq!(afterstack.ss_sp, our_sigstack_alloc.as_mut_ptr() as *mut _); }); } - - #[test] - fn no_import_funcs_in_start() { - test_nonex(|| { - let module = test_module_wasm("start", "call_import_func.wat") - .expect("module compiled and loaded"); - let region = - TestRegion::create(1, &Limits::default()).expect("region can be created"); - let mut inst = region - .new_instance(module) - .expect("instance can be created"); - - match inst.run_start().unwrap_err() { - Error::RuntimeTerminated(TerminationDetails::StartCalledImportFunc) => (), - e => panic!("unexpected error: {}", e), - } - }); - } }; } diff --git a/lucet-runtime/tests/start.rs b/lucet-runtime/tests/start.rs index fb1fccf18..6b1f2a26c 100644 --- a/lucet-runtime/tests/start.rs +++ b/lucet-runtime/tests/start.rs @@ -1,4 +1,3 @@ -use lucet_runtime_tests::{start_common_defs, start_tests}; +use lucet_runtime_tests::start_tests; -start_common_defs!(); start_tests!(lucet_runtime::MmapRegion);