Merge #2692

2692: Make module serialization deterministic r=Amanieu a=Amanieu

Fixes #2173

Includes the tests from #2184.

Co-authored-by: Amanieu d'Antras <amanieu@gmail.com>
Co-authored-by: Anbang Wen <anbang@cloudflare.com>

fuzz/Cargo.toml

@@ -57,3 +57,8 @@ required-features = ["universal", "cranelift"]
 name = "dylib_cranelift"
 path = "fuzz_targets/dylib_cranelift.rs"
 required-features = ["dylib", "cranelift"]
+
+[[bin]]
+name = "deterministic"
+path = "fuzz_targets/deterministic.rs"
+required-features = ["universal", "dylib", "cranelift", "llvm", "singlepass"]

fuzz/fuzz_targets/deterministic.rs

@@ -0,0 +1,81 @@
+#![no_main]
+
+use libfuzzer_sys::{arbitrary, arbitrary::Arbitrary, fuzz_target};
+use wasm_smith::{Config, ConfiguredModule};
+use wasmer::{CompilerConfig, Engine, Module, Store};
+use wasmer_compiler_cranelift::Cranelift;
+use wasmer_compiler_llvm::LLVM;
+use wasmer_compiler_singlepass::Singlepass;
+use wasmer_engine_dylib::Dylib;
+use wasmer_engine_universal::Universal;
+
+#[derive(Arbitrary, Debug, Default, Copy, Clone)]
+struct NoImportsConfig;
+impl Config for NoImportsConfig {
+    fn max_imports(&self) -> usize {
+        0
+    }
+    fn max_memory_pages(&self) -> u32 {
+        // https://github.com/wasmerio/wasmer/issues/2187
+        65535
+    }
+    fn allow_start_export(&self) -> bool {
+        false
+    }
+}
+
+fn compile_and_compare(name: &str, engine: impl Engine, wasm: &[u8]) {
+    let store = Store::new(&engine);
+
+    // compile for first time
+    let module = Module::new(&store, wasm).unwrap();
+    let first = module.serialize().unwrap();
+
+    // compile for second time
+    let module = Module::new(&store, wasm).unwrap();
+    let second = module.serialize().unwrap();
+
+    if first != second {
+        panic!("non-deterministic compilation from {}", name);
+    }
+}
+
+fuzz_target!(|module: ConfiguredModule<NoImportsConfig>| {
+    let wasm_bytes = module.to_bytes();
+
+    let mut compiler = Cranelift::default();
+    compiler.canonicalize_nans(true);
+    compiler.enable_verifier();
+    compile_and_compare(
+        "universal-cranelift",
+        Universal::new(compiler.clone()).engine(),
+        &wasm_bytes,
+    );
+    //compile_and_compare(
+    //    "dylib-cranelift",
+    //    Dylib::new(compiler).engine(),
+    //    &wasm_bytes,
+    //);
+
+    let mut compiler = LLVM::default();
+    compiler.canonicalize_nans(true);
+    compiler.enable_verifier();
+    compile_and_compare(
+        "universal-llvm",
+        Universal::new(compiler.clone()).engine(),
+        &wasm_bytes,
+    );
+    //compile_and_compare("dylib-llvm", Dylib::new(compiler).engine(), &wasm_bytes);
+
+    let compiler = Singlepass::default();
+    compile_and_compare(
+        "universal-singlepass",
+        Universal::new(compiler.clone()).engine(),
+        &wasm_bytes,
+    );
+    //compile_and_compare(
+    //    "dylib-singlepass",
+    //    Dylib::new(compiler).engine(),
+    //    &wasm_bytes,
+    //);
+});

lib/api/src/js/externals/function.rs

@@ -1288,7 +1288,7 @@ mod inner {
 
         #[test]
         fn test_into_array() {
-            assert_eq!(().into_array(), []);
+            assert_eq!(().into_array(), [0i128; 0]);
             assert_eq!((1).into_array(), [1]);
             assert_eq!((1i32, 2i64).into_array(), [1, 2]);
             assert_eq!(

lib/api/src/sys/externals/function.rs

@@ -1472,7 +1472,7 @@ mod inner {
 
         #[test]
         fn test_into_array() {
-            assert_eq!(().into_array(), []);
+            assert_eq!(().into_array(), [0i128; 0]);
             assert_eq!((1).into_array(), [1]);
             assert_eq!((1i32, 2i64).into_array(), [1, 2]);
             assert_eq!(

lib/compiler-cranelift/src/compiler.rs

@@ -92,14 +92,13 @@ impl Compiler for CraneliftCompiler {
             // FDEs will cause some issues in Linux.
             None
         } else {
-            use std::sync::Mutex;
             match target.triple().default_calling_convention() {
                 Ok(CallingConvention::SystemV) => {
                     match isa.create_systemv_cie() {
                         Some(cie) => {
                             let mut dwarf_frametable = FrameTable::default();
                             let cie_id = dwarf_frametable.add_cie(cie);
-                            Some((Arc::new(Mutex::new(dwarf_frametable)), cie_id))
+                            Some((dwarf_frametable, cie_id))
                         }
                         // Even though we are in a SystemV system, Cranelift doesn't support it
                         None => None,
@@ -134,7 +133,7 @@ impl Compiler for CraneliftCompiler {
         #[cfg(all(target_arch = "x86_64", target_os = "linux"))]
         let probestack_trampoline_relocation_target = SectionIndex::new(custom_sections.len() - 1);
 
-        let functions = function_body_inputs
+        let (functions, fdes): (Vec<CompiledFunction>, Vec<_>) = function_body_inputs
             .iter()
             .collect::<Vec<(LocalFunctionIndex, &FunctionBodyData<'_>)>>()
             .par_iter()
@@ -190,31 +189,31 @@ impl Compiler for CraneliftCompiler {
                         CompileError::Codegen(pretty_error(&context.func, Some(&*isa), error))
                     })?;
 
-                let unwind_info = match compiled_function_unwind_info(&*isa, &context)? {
+                let (unwind_info, fde) = match compiled_function_unwind_info(&*isa, &context)? {
                     #[cfg(feature = "unwind")]
                     CraneliftUnwindInfo::FDE(fde) => {
-                        if let Some((dwarf_frametable, cie_id)) = &dwarf_frametable {
-                            dwarf_frametable
-                                .lock()
-                                .expect("Can't write into DWARF frametable")
-                                .add_fde(
-                                    *cie_id,
-                                    fde.to_fde(Address::Symbol {
-                                        // The symbol is the kind of relocation.
-                                        // "0" is used for functions
-                                        symbol: WriterRelocate::FUNCTION_SYMBOL,
-                                        // We use the addend as a way to specify the
-                                        // function index
-                                        addend: i.index() as _,
-                                    }),
-                                );
+                        if dwarf_frametable.is_some() {
+                            let fde = fde.to_fde(Address::Symbol {
+                                // The symbol is the kind of relocation.
+                                // "0" is used for functions
+                                symbol: WriterRelocate::FUNCTION_SYMBOL,
+                                // We use the addend as a way to specify the
+                                // function index
+                                addend: i.index() as _,
+                            });
                             // The unwind information is inserted into the dwarf section
-                            Some(CompiledFunctionUnwindInfo::Dwarf)
+                            (Some(CompiledFunctionUnwindInfo::Dwarf), Some(fde))
                         } else {
-                            None
+                            (None, None)
                         }
                     }
-                    other => other.maybe_into_to_windows_unwind(),
+                    #[cfg(feature = "unwind")]
+                    other => (other.maybe_into_to_windows_unwind(), None),
+
+                    // This is a bit hacky, but necessary since gimli is not
+                    // available when the "unwind" feature is disabled.
+                    #[cfg(not(feature = "unwind"))]
+                    other => (other.maybe_into_to_windows_unwind(), None::<()>),
                 };
 
                 let range = reader.range();
@@ -223,40 +222,41 @@ impl Compiler for CraneliftCompiler {
                 // We transform the Cranelift JumpTable's into compiler JumpTables
                 let func_jt_offsets = transform_jump_table(context.func.jt_offsets);
 
-                Ok(CompiledFunction {
-                    body: FunctionBody {
-                        body: code_buf,
-                        unwind_info,
+                Ok((
+                    CompiledFunction {
+                        body: FunctionBody {
+                            body: code_buf,
+                            unwind_info,
+                        },
+                        jt_offsets: func_jt_offsets,
+                        relocations: reloc_sink.func_relocs,
+                        frame_info: CompiledFunctionFrameInfo {
+                            address_map,
+                            traps: trap_sink.traps,
+                        },
                     },
-                    jt_offsets: func_jt_offsets,
-                    relocations: reloc_sink.func_relocs,
-                    frame_info: CompiledFunctionFrameInfo {
-                        address_map,
-                        traps: trap_sink.traps,
-                    },
-                })
+                    fde,
+                ))
             })
             .collect::<Result<Vec<_>, CompileError>>()?
             .into_iter()
-            .collect::<PrimaryMap<LocalFunctionIndex, _>>();
+            .unzip();
 
         #[cfg(feature = "unwind")]
-        let dwarf = {
-            let dwarf = if let Some((dwarf_frametable, _cie_id)) = dwarf_frametable {
-                let mut eh_frame = EhFrame(WriterRelocate::new(target.triple().endianness().ok()));
-                dwarf_frametable
-                    .lock()
-                    .unwrap()
-                    .write_eh_frame(&mut eh_frame)
-                    .unwrap();
+        let dwarf = if let Some((mut dwarf_frametable, cie_id)) = dwarf_frametable {
+            for fde in fdes {
+                if let Some(fde) = fde {
+                    dwarf_frametable.add_fde(cie_id, fde);
+                }
+            }
+            let mut eh_frame = EhFrame(WriterRelocate::new(target.triple().endianness().ok()));
+            dwarf_frametable.write_eh_frame(&mut eh_frame).unwrap();
 
-                let eh_frame_section = eh_frame.0.into_section();
-                custom_sections.push(eh_frame_section);
-                Some(Dwarf::new(SectionIndex::new(custom_sections.len() - 1)))
-            } else {
-                None
-            };
-            dwarf
+            let eh_frame_section = eh_frame.0.into_section();
+            custom_sections.push(eh_frame_section);
+            Some(Dwarf::new(SectionIndex::new(custom_sections.len() - 1)))
+        } else {
+            None
         };
         #[cfg(not(feature = "unwind"))]
         let dwarf = None;
@@ -289,7 +289,7 @@ impl Compiler for CraneliftCompiler {
             .collect::<PrimaryMap<FunctionIndex, FunctionBody>>();
 
         Ok(Compilation::new(
-            functions,
+            functions.into_iter().collect(),
             custom_sections,
             function_call_trampolines,
             dynamic_function_trampolines,

lib/compiler/Cargo.toml

@@ -20,8 +20,8 @@ hashbrown = { version = "0.11", optional = true }
 serde = { version = "1.0", features = ["derive"], optional = true }
 thiserror = "1.0"
 serde_bytes = { version = "0.11", optional = true }
-smallvec = "1.6" 
-rkyv = { version = "0.6.1", optional = true }
+smallvec = "1.6"
+rkyv = { version = "0.7.20", optional = true }
 loupe = "0.1"
 
 [features]

lib/compiler/src/section.rs

@@ -22,7 +22,7 @@ use wasmer_types::entity::entity_impl;
 )]
 #[cfg_attr(
     feature = "enable-rkyv",
-    archive(derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Debug))
+    archive_attr(derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Debug))
 )]
 #[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Debug, MemoryUsage)]
 pub struct SectionIndex(u32);

lib/engine-dylib/Cargo.toml

@@ -23,7 +23,7 @@ leb128 = "0.2"
 libloading = "0.7"
 tempfile = "3.1"
 which = "4.0"
-rkyv = "0.6.1"
+rkyv = "0.7.20"
 loupe = "0.1"
 
 [features]

lib/engine-dylib/src/serialize.rs

@@ -1,10 +1,8 @@
 use loupe::MemoryUsage;
 use rkyv::{
-    archived_value,
-    de::{adapters::SharedDeserializerAdapter, deserializers::AllocDeserializer},
-    ser::adapters::SharedSerializerAdapter,
-    ser::{serializers::WriteSerializer, Serializer as RkyvSerializer},
-    Archive, Deserialize as RkyvDeserialize, Serialize as RkyvSerialize,
+    archived_value, de::deserializers::SharedDeserializeMap, ser::serializers::AllocSerializer,
+    ser::Serializer as RkyvSerializer, Archive, Deserialize as RkyvDeserialize,
+    Serialize as RkyvSerialize,
 };
 use serde::{Deserialize, Serialize};
 use std::error::Error;
@@ -59,11 +57,11 @@ impl ModuleMetadata {
     }
 
     pub fn serialize(&mut self) -> Result<Vec<u8>, CompileError> {
-        let mut serializer = SharedSerializerAdapter::new(WriteSerializer::new(vec![]));
+        let mut serializer = AllocSerializer::<4096>::default();
         let pos = serializer.serialize_value(self).map_err(to_compile_error)? as u64;
-        let mut serialized_data = serializer.into_inner().into_inner();
+        let mut serialized_data = serializer.into_serializer().into_inner();
         serialized_data.extend_from_slice(&pos.to_le_bytes());
-        Ok(serialized_data)
+        Ok(serialized_data.to_vec())
     }
 
     pub unsafe fn deserialize(metadata_slice: &[u8]) -> Result<Self, DeserializeError> {
@@ -86,7 +84,7 @@ impl ModuleMetadata {
     pub fn deserialize_from_archive(
         archived: &ArchivedModuleMetadata,
     ) -> Result<Self, DeserializeError> {
-        let mut deserializer = SharedDeserializerAdapter::new(AllocDeserializer);
+        let mut deserializer = SharedDeserializeMap::new();
         RkyvDeserialize::deserialize(archived, &mut deserializer)
             .map_err(|e| DeserializeError::CorruptedBinary(format!("{:?}", e)))
     }