Change Rust integration to emit a single static library (#212)

Instead of compiling multiple crates to static libs and trying to link all of them into the final binary, we now synthesize a crate that bundles all individual crates into a single staticlib. We maintain a single `Cargo.lock` file for this synthesized crate, which means that we can have a situation in which not all dependencies are tracked. That's "okay" in that we can live with the lockfile being best-effort.

This also replaces linking SpiderMonkey's Rust static library with including the required crates into our own library, achieving an end result of having a single static Rust library to link, and enables thin-LTO for all libraries: this is in general useful, but even more relevant with more Rust code.

Rolled into this is an update to newer version of Corrosion: the release of Rustup 1.28.0 broke the version of Corrosion we were previously using, so an update is required regardless of whatever else we do with our Rust integration. Since the newer version of Corrosion didn't work out-of-the-box with the previous Rust setup, I decided to fold it in here.

And finally, the PR includes a bugfix in TextDecoder. The fixed issue is preexisting, but is only getting caught with the Rust changes. Either something changed in Rust's stdlib between 1.77.1 and 1.80.0, or the encoding_rs crate got bumped and changed in a way that exposes this issue whereas before it papered over it.

Author: tschneidereit

.github/workflows/main.yml

@@ -27,10 +27,10 @@ jobs:
     steps:
     - uses: actions/checkout@v2
 
-    - name: Install Rust 1.77.1
+    - name: Install Rust 1.80.0
       run: |
-        rustup toolchain install 1.77.1
-        rustup target add wasm32-wasi --toolchain 1.77.1
+        rustup toolchain install 1.80.0
+        rustup target add wasm32-wasip1 --toolchain 1.80.0
 
     - uses: actions/setup-node@v2
       with:
@@ -58,10 +58,10 @@ jobs:
       with:
         submodules: recursive
 
-    - name: Install Rust 1.77.1
+    - name: Install Rust 1.80.0
       run: |
-        rustup toolchain install 1.77.1
-        rustup target add wasm32-wasi --toolchain 1.77.1
+        rustup toolchain install 1.80.0
+        rustup target add wasm32-wasip1 --toolchain 1.80.0
 
     - uses: actions/setup-node@v2
       with:

CMakeLists.txt

@@ -1,6 +1,7 @@
 cmake_minimum_required(VERSION 3.27)
 set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
 set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
+set(CMAKE_INTERPROCEDURAL_OPTIMIZATION TRUE)
 
 if (DEFINED ENV{HOST_API})
     set(HOST_API $ENV{HOST_API})
@@ -66,7 +67,7 @@ add_executable(starling-raw.wasm
         runtime/debugger.cpp
 )
 
-target_link_libraries(starling-raw.wasm PRIVATE host_api extension_api builtins spidermonkey rust-url multipart)
+target_link_libraries(starling-raw.wasm PRIVATE host_api extension_api builtins spidermonkey rust-crates)
 
 option(USE_WASM_OPT "use wasm-opt to optimize the StarlingMonkey binary" ON)
 
@@ -119,11 +120,10 @@ endif()
 
 set(RUNTIME_FILE "starling-raw.wasm")
 set(ADAPTER_FILE "preview1-adapter.wasm")
-configure_file("componentize.sh" "${CMAKE_CURRENT_BINARY_DIR}/componentize.sh" @ONLY)
+configure_file("componentize.sh.in" "${CMAKE_CURRENT_BINARY_DIR}/componentize.sh" @ONLY)
 if(EXISTS ${ADAPTER})
     configure_file(${ADAPTER} "${CMAKE_CURRENT_BINARY_DIR}/${ADAPTER_FILE}" COPYONLY)
 endif()
-configure_file(spin.toml spin.toml COPYONLY)
 
 function(componentize OUTPUT)
     set(options)

builtins/web/text-codec/text-decoder.cpp

@@ -18,14 +18,24 @@ bool TextDecoder::decode(JSContext *cx, unsigned argc, JS::Value *vp) {
 
   auto source_value = args.get(0);
   std::optional<std::span<uint8_t>> src;
+  uint8_t *src_ptr = nullptr;
 
+  // If the input is undefined, we use an empty buffer. We can't return early though,
+  // because the decoder might have state that needs to be flushed in streaming mode.
   if (source_value.isUndefined()) {
     src = std::span<uint8_t, 0>();
+    // Quoting from the encoding_rs docs:
+    // `src` must be non-`NULL` even if `src_len` is zero. When`src_len` is zero,
+    // it is OK for `src` to be something non-dereferencable, such as `0x1`.
+    // Likewise for `dst` when `dst_len` is zero. This is required due to Rust's
+    // optimization for slices within `Option`.
+    src_ptr = reinterpret_cast<uint8_t *>(0x1);
   } else {
     src = value_to_buffer(cx, source_value, "TextDecoder#decode: input");
-  }
-  if (!src.has_value()) {
-    return false;
+    if (!src.has_value()) {
+      return false;
+    }
+    src_ptr = src->data();
   }
 
   bool stream = false;
@@ -60,14 +70,14 @@ bool TextDecoder::decode(JSContext *cx, unsigned argc, JS::Value *vp) {
     return false;
   }
   if (fatal) {
-    result = jsencoding::decoder_decode_to_utf16_without_replacement(decoder, src->data(), &srcLen,
+    result = jsencoding::decoder_decode_to_utf16_without_replacement(decoder, src_ptr, &srcLen,
                                                                      dest.get(), &destLen, !stream);
     if (result != 0) {
       return api::throw_error(cx, TextCodecErrors::DecodingFailed);
     }
   } else {
     bool hadReplacements;
-    result = jsencoding::decoder_decode_to_utf16(decoder, src->data(), &srcLen, dest.get(),
+    result = jsencoding::decoder_decode_to_utf16(decoder, src_ptr, &srcLen, dest.get(),
                                                  &destLen, !stream, &hadReplacements);
   }
   MOZ_ASSERT(result == 0);

cmake/build-crates.cmake

@@ -1,5 +1,49 @@
-corrosion_import_crate(MANIFEST_PATH ${CMAKE_CURRENT_SOURCE_DIR}/crates/rust-url/Cargo.toml NO_LINKER_OVERRIDE)
-set_property(TARGET rust-url PROPERTY INTERFACE_INCLUDE_DIRECTORIES ${CMAKE_CURRENT_SOURCE_DIR}/crates/rust-url/)
+# To avoid conflicts caused by multiple definitions of the same symbols,
+# we need to build all Rust crates as part of a single staticlib.
+# This is done by synthesizing a Rust crate that depends on all the others,
+# and contains `pub use` statements for each of them.
+#
+# You might ask why we don't just do all of this in Rust itself, without CMake
+# involved at all.
+# A key reason is that this setup enables us to add Rust crates wherever we'd also
+# be able to add C++ libraries, and have them all be built together.
+# Notably, this way we can have host-API implementations that include Rust code,
+# while keeping the host-API implementation fully self-contained, without changes
+# outside its own folder.
 
-corrosion_import_crate(MANIFEST_PATH ${CMAKE_CURRENT_SOURCE_DIR}/crates/rust-multipart/Cargo.toml NO_LINKER_OVERRIDE CRATES multipart FEATURES capi)
-set_property(TARGET multipart PROPERTY INTERFACE_INCLUDE_DIRECTORIES ${CMAKE_CURRENT_SOURCE_DIR}/crates/rust-multipart/)
+set(RUST_STATICLIB_RS "${CMAKE_CURRENT_BINARY_DIR}/rust-staticlib.rs" CACHE INTERNAL "Path to the Rust staticlibs bundler source file" FORCE)
+set(RUST_STATICLIB_TOML "${CMAKE_CURRENT_BINARY_DIR}/Cargo.toml" CACHE INTERNAL "Path to the Rust staticlibs bundler Cargo.toml file" FORCE)
+set(RUST_STATICLIB_LOCK "${CMAKE_CURRENT_BINARY_DIR}/Cargo.lock" CACHE INTERNAL "Path to the Rust staticlibs bundler Cargo.toml file" FORCE)
+
+configure_file("runtime/crates/staticlib-template/rust-staticlib.rs.in" "${RUST_STATICLIB_RS}" COPYONLY)
+configure_file("runtime/crates/staticlib-template/Cargo.toml.in" "${RUST_STATICLIB_TOML}")
+configure_file("runtime/crates/staticlib-template/Cargo.lock" "${RUST_STATICLIB_LOCK}" COPYONLY)
+
+corrosion_import_crate(
+        MANIFEST_PATH ${RUST_STATICLIB_TOML}
+        CRATES "rust-staticlib"
+        NO_LINKER_OVERRIDE
+)
+
+# Add a Rust library to the staticlib bundle.
+function(add_rust_lib name path)
+    add_library(${name} INTERFACE)
+    target_include_directories(${name} INTERFACE "${path}")
+    file(APPEND $CACHE{RUST_STATICLIB_TOML} "${name} = { path = \"${path}\", features = [${ARGN}] }\n")
+    string(REPLACE "-" "_" name ${name})
+    file(APPEND $CACHE{RUST_STATICLIB_RS} "pub use ${name};\n")
+endfunction()
+
+# These two crates are needed by SpiderMonkey:
+add_rust_lib(rust-encoding "${CMAKE_CURRENT_SOURCE_DIR}/crates/rust-encoding")
+add_rust_lib(rust-hooks "${CMAKE_CURRENT_SOURCE_DIR}/crates/rust-hooks")
+
+# The rust-hooks crate needs a supporting CPP file
+add_library(rust-hooks-wrappers STATIC "${CMAKE_CURRENT_SOURCE_DIR}/crates/rust-hooks/src/wrappers.cpp")
+target_link_libraries(rust-hooks-wrappers PRIVATE spidermonkey)
+add_library(rust-crates STATIC ${CMAKE_CURRENT_BINARY_DIR}/null.cpp)
+target_link_libraries(rust-crates PRIVATE rust_staticlib rust-hooks-wrappers)
+
+# Add crates as needed here:
+add_rust_lib(rust-url "${CMAKE_CURRENT_SOURCE_DIR}/crates/rust-url")
+add_rust_lib(multipart "${CMAKE_CURRENT_SOURCE_DIR}/crates/rust-multipart" "\"capi\", \"simd\"")

cmake/init-corrosion.cmake

@@ -2,7 +2,7 @@
 # NOTE: This file must not be called `corrosion.cmake`, because otherwise it'll be called recursively instead of the
 # same-named one coming with Corrosion.
 
-set(Rust_CARGO_TARGET wasm32-wasi)
+set(Rust_CARGO_TARGET wasm32-wasip1)
 # Necessary to make cross-compiling to wasm32-wasi work for crates with -sys dependencies.
 set(Rust_CARGO_TARGET_LINK_NATIVE_LIBS "")
 
@@ -11,5 +11,5 @@ string(REGEX MATCH "[0-9.]+" Rust_TOOLCHAIN "${Rust_TOOLCHAIN}")
 execute_process(COMMAND rustup toolchain install ${Rust_TOOLCHAIN})
 execute_process(COMMAND rustup target add --toolchain ${Rust_TOOLCHAIN} wasm32-wasi)
 
-CPMAddPackage("gh:corrosion-rs/corrosion#be76480232216a64f65e3b1d9794d68cbac6c690")
+CPMAddPackage("gh:corrosion-rs/corrosion@0.5.1")
 string(TOLOWER ${Rust_CARGO_HOST_ARCH} HOST_ARCH)

cmake/spidermonkey.cmake

@@ -38,7 +38,7 @@ file(GLOB SM_OBJS ${SM_SOURCE_DIR}/lib/*.o)
 
 add_library(spidermonkey STATIC)
 target_sources(spidermonkey PRIVATE ${SM_OBJS} ${CMAKE_CURRENT_BINARY_DIR}/null.cpp)
-set_property(TARGET spidermonkey PROPERTY INTERFACE_INCLUDE_DIRECTORIES ${SM_INCLUDE_DIR})
-target_link_libraries(spidermonkey PUBLIC ${SM_SOURCE_DIR}/lib/libjs_static.a ${SM_SOURCE_DIR}/lib/libjsrust.a)
+target_include_directories(spidermonkey PUBLIC ${SM_INCLUDE_DIR})
+target_link_libraries(spidermonkey PUBLIC ${SM_SOURCE_DIR}/lib/libjs_static.a)
 
 add_compile_definitions("MOZ_JS_STREAMS")

cmake/toolchain.cmake

@@ -1,4 +1,6 @@
 string(TOLOWER ${CMAKE_HOST_SYSTEM_NAME} HOST_OS)
+cmake_host_system_information(RESULT HOST_CPU QUERY OS_PLATFORM)
+
 
 if (${HOST_OS} STREQUAL "darwin")
     set(HOST_OS "macos")

componentize.sh componentize.sh.incomponentize.sh renamed to componentize.sh.in

@@ -0,0 +1,11 @@
+[package]
+name = "rust-encoding"
+version = "0.1.0"
+edition = "2021"
+description = "Character encoding library used by SpiderMonkey and web builtins"
+
+[lib]
+
+[dependencies]
+encoding_c = "0.9.8"
+encoding_c_mem = "0.2.6"

crates/rust-encoding/Cargo.toml

@@ -0,0 +1,3 @@
+// Ensure that the encoding modules are built, as SpiderMonkey relies on them.
+pub use encoding_c;
+pub use encoding_c_mem;