From a5c3809290c5567e026730324984399e3dcab99f Mon Sep 17 00:00:00 2001 From: Brian Carroll Date: Mon, 15 Nov 2021 11:58:30 +0000 Subject: [PATCH 1/3] Delete DataCountSection. Not well-supported and only needed for instructions we don't use https://webassembly.github.io/spec/core/binary/modules.html#binary-datacountsec Tools like wasm2wat and wasm-validate reject the module when this section is included! Its purpose is to enable single-pass validation for two specific instructions that were not in the original Wasm MVP: memory.init and data.drop. We don't use them in our Roc backend. It seems to make sense just to drop the section. --- compiler/gen_wasm/src/wasm_module/sections.rs | 43 +------------------ 1 file changed, 2 insertions(+), 41 deletions(-) diff --git a/compiler/gen_wasm/src/wasm_module/sections.rs b/compiler/gen_wasm/src/wasm_module/sections.rs index 2ff0805604..855d5062ee 100644 --- a/compiler/gen_wasm/src/wasm_module/sections.rs +++ b/compiler/gen_wasm/src/wasm_module/sections.rs @@ -29,6 +29,8 @@ pub enum SectionId { Element = 9, Code = 10, Data = 11, + /// DataCount section is unused. Only needed for single-pass validation of + /// memory.init and data.drop, which we don't use DataCount = 12, } @@ -525,42 +527,6 @@ impl Serialize for DataSection<'_> { } } -/******************************************************************* - * - * Data Count section - * - * Pre-declares the number of segments in the Data section. - * This helps the runtime to validate the module in a single pass. - * The order of sections is DataCount -> Code -> Data - * - *******************************************************************/ - -#[derive(Debug)] -struct DataCountSection { - count: u32, -} - -impl DataCountSection { - fn new(data_section: &DataSection<'_>) -> Self { - let count = data_section - .segments - .iter() - .filter(|seg| !seg.init.is_empty()) - .count() as u32; - DataCountSection { count } - } -} - -impl Serialize for DataCountSection { - fn serialize(&self, buffer: &mut T) { - if self.count > 0 { - let header_indices = write_section_header(buffer, SectionId::DataCount); - buffer.encode_u32(self.count); - update_section_size(buffer, header_indices); - } - } -} - /******************************************************************* * * Module @@ -658,11 +624,6 @@ impl<'a> WasmModule<'a> { counter.serialize_and_count(buffer, &self.start); counter.serialize_and_count(buffer, &self.element); - // Data Count section forward-declares the size of the Data section - // so that Code section can be validated in one pass - let data_count_section = DataCountSection::new(&self.data); - counter.serialize_and_count(buffer, &data_count_section); - // Code section is the only one with relocations so we can stop counting let code_section_index = counter.section_index; let code_section_body_index = self From 7ac1c7a72fbe72538905e2676528ec625812b279 Mon Sep 17 00:00:00 2001 From: Brian Carroll Date: Mon, 15 Nov 2021 12:11:36 +0000 Subject: [PATCH 2/3] Get long string literals working in gen_wasm --- compiler/gen_wasm/src/backend.rs | 4 ++-- .../gen_wasm/src/wasm_module/code_builder.rs | 11 +++++++---- compiler/test_gen/src/wasm_str.rs | 19 +++++++++---------- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/compiler/gen_wasm/src/backend.rs b/compiler/gen_wasm/src/backend.rs index 1d0928611e..8b91379120 100644 --- a/compiler/gen_wasm/src/backend.rs +++ b/compiler/gen_wasm/src/backend.rs @@ -626,8 +626,8 @@ impl<'a> WasmBackend<'a> { self.lookup_string_constant(string, sym, layout); self.code_builder.get_local(local_id); - self.code_builder.insert_memory_relocation(linker_sym_index); - self.code_builder.i32_const(elements_addr as i32); + self.code_builder + .i32_const_mem_addr(elements_addr, linker_sym_index); self.code_builder.i32_store(Align::Bytes4, offset); self.code_builder.get_local(local_id); diff --git a/compiler/gen_wasm/src/wasm_module/code_builder.rs b/compiler/gen_wasm/src/wasm_module/code_builder.rs index 5468784926..2109f3e436 100644 --- a/compiler/gen_wasm/src/wasm_module/code_builder.rs +++ b/compiler/gen_wasm/src/wasm_module/code_builder.rs @@ -9,7 +9,7 @@ use super::opcodes::{OpCode, OpCode::*}; use super::serialize::{SerialBuffer, Serialize}; use crate::{round_up_to_alignment, FRAME_ALIGNMENT_BYTES, STACK_POINTER_GLOBAL_ID}; -const ENABLE_DEBUG_LOG: bool = true; +const ENABLE_DEBUG_LOG: bool = false; macro_rules! log_instruction { ($($x: expr),+) => { if ENABLE_DEBUG_LOG { println!($($x,)*); } @@ -572,11 +572,14 @@ impl<'a> CodeBuilder<'a> { ); } - /// Insert a linker relocation for a memory address - pub fn insert_memory_relocation(&mut self, symbol_index: u32) { + /// Insert a const reference to a memory address + pub fn i32_const_mem_addr(&mut self, addr: u32, symbol_index: u32) { + self.inst_base(I32CONST, 0, true); + let offset = self.code.len() as u32; + self.code.encode_padded_u32(addr); self.relocations.push(RelocationEntry::Offset { type_id: OffsetRelocType::MemoryAddrLeb, - offset: self.code.len() as u32, + offset, symbol_index, addend: 0, }); diff --git a/compiler/test_gen/src/wasm_str.rs b/compiler/test_gen/src/wasm_str.rs index d893f362f1..fcba6143c1 100644 --- a/compiler/test_gen/src/wasm_str.rs +++ b/compiler/test_gen/src/wasm_str.rs @@ -12,7 +12,7 @@ use crate::helpers::wasm::assert_evals_to; #[allow(unused_imports)] use indoc::indoc; -// use roc_std::RocStr; +use roc_std::RocStr; // #[test] // fn str_split_bigger_delimiter_small_str() { @@ -287,15 +287,14 @@ fn small_str_zeroed_literal() { ); } -// TODO: fix linking errors for undefined symbols roc_alloc, roc_dealloc -// #[test] -// fn long_str_literal() { -// assert_evals_to!( -// "\"0123456789 123456789 123456789\"", -// RocStr::from_slice(b"0123456789 123456789 123456789"), -// RocStr -// ); -// } +#[test] +fn long_str_literal() { + assert_evals_to!( + "\"0123456789 123456789 123456789\"", + RocStr::from_slice(b"0123456789 123456789 123456789"), + RocStr + ); +} // #[test] // fn small_str_concat_empty_first_arg() { From 36f2ef301f03d6e89be19639cce8aee61b1482de Mon Sep 17 00:00:00 2001 From: Brian Carroll Date: Tue, 16 Nov 2021 23:41:15 +0000 Subject: [PATCH 3/3] Only enable wasm_str tests for gen-wasm feature --- compiler/test_gen/src/wasm_str.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/test_gen/src/wasm_str.rs b/compiler/test_gen/src/wasm_str.rs index fcba6143c1..8780a1a5bf 100644 --- a/compiler/test_gen/src/wasm_str.rs +++ b/compiler/test_gen/src/wasm_str.rs @@ -1,6 +1,6 @@ // Wasm pointers are only 32bit. This effects RocStr. // These are versions of the str tests assuming 32bit pointers. -#![cfg(not(feature = "gen-dev"))] +#![cfg(feature = "gen-wasm")] // TODO: We need to make these tests work with the llvm wasm backend.