This works with 0.35.0 and panics on 0.38.0:

        let wasm: Vec<u8> = FromHex::from_hex("0061736d0100000001080260017e0060000002170103656e760f657468657265756d5f757365476173000003030201010503010001071102046d61696e0001066d656d6f727902000801020a070202000b02000b0020046e616d65010e0201046d61696e02056d61696e320209030001000001000200").unwrap();

        let mut module: Module = parity_wasm::deserialize_buffer::<Module>(&wasm).unwrap();
        module = module.parse_names().unwrap(); // panic here

Error snippet:

panicked at 'called `Result::unwrap()` on an `Err` value: ([(7, Other("index is larger than expected"))], Module { magic: 1836278016, version: 1, sections: [Type(TypeSe
ction([Function(FunctionType { form: 96, params: [I64], return_type: None }), Function(FunctionType { form: 96, params: [], return_type: None })])), Import(ImportSection([ImportEntry { module_str: "env", field_str
: "ethereum_useGas", external: Function(0) }])), Function(FunctionSection([Func(1), Func(1)])), Memory(MemorySection([MemoryType(ResizableLimits { initial: 1, maximum: None, shared: false })])), Export(ExportSecti
on([ExportEntry { field_str: "main", internal: Function(1) }, ExportEntry { field_str: "memory", internal: Memory(0) }])), Start(2), Code(CodeSection([FuncBody { locals: [], instructions: Instructions([End]) }, Fu
ncBody { locals: [], instructions: Instructions([End]) }])), Custom(CustomSection { name: "name", payload: [1, 14, 2, 1, 4, 109, 97, 105, 110, 2, 5, 109, 97, 105, 110, 50, 2, 9, 3, 0, 1, 0, 0, 1, 0, 2, 0] })] })',
 src/libcore/result.rs:997:5

I'm using Ubuntu 18.0.4.

I pointed out in https://github.com/paritytech/parity-wasm/pull/196#issuecomment-371278025 that LLVM/lld sometimes compiles integers to larger representations than they need. That particular case was an i32.const 0 compiled as 0x41 0x80 0x80 0x80 0x80 0x00 instead of just 0x41 0x00. parity-wasm can read it just fine, but it serializes it back out as 0x41 0x00 instead of the original representation.

This is a problem for relocations since relocations are measured in byte offsets from the start of the corresponding section. So a round-trip breaks all relocations after such a value.

It seems to me from testing that lld does this for every relocatable value, probably intentionally so that loaders / linkers doing have space to write the relocated value without needing to move bytes around.

How do you think this should be fixed?

1. Each RelocationEntry could have some reference to the corresponding Var*Int* (or to the section containing the Var*Int*. But this crosses section boundaries and be brittle.

2. Or, each Var*Int could store the width it originally had, and the serializer could artificially expand it to that width. But this breaks the From and Into impls for these types.

---- elements::module::integration_tests::const_ stdout ----
thread 'elements::module::integration_tests::const_' panicked at 'Should be deserialized: HeapOther("Can\'t read from the file: Os { code: 2, kind: NotFound, message: \"No such file or directory\" }")', libcore/result.rs:945:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.

---- elements::module::integration_tests::hello stdout ----
thread 'elements::module::integration_tests::hello' panicked at 'Should be deserialized: HeapOther("Can\'t read from the file: Os { code: 2, kind: NotFound, message: \"No such file or directory\" }")', libcore/result.rs:945:5

---- elements::module::integration_tests::peek stdout ----
thread 'elements::module::integration_tests::peek' panicked at 'Should be deserialized: HeapOther("Can\'t read from the file: Os { code: 2, kind: NotFound, message: \"No such file or directory\" }")', libcore/result.rs:945:5

---- elements::module::integration_tests::memory_space stdout ----
thread 'elements::module::integration_tests::memory_space' panicked at 'failed to deserialize: HeapOther("Can\'t read from the file: Os { code: 2, kind: NotFound, message: \"No such file or directory\" }")', libcore/result.rs:945:5

---- elements::module::integration_tests::names stdout ----
thread 'elements::module::integration_tests::names' panicked at 'Should be deserialized: HeapOther("Can\'t read from the file: Os { code: 2, kind: NotFound, message: \"No such file or directory\" }")', libcore/result.rs:945:5

---- elements::module::integration_tests::serde stdout ----
thread 'elements::module::integration_tests::serde' panicked at 'Should be deserialized: HeapOther("Can\'t read from the file: Os { code: 2, kind: NotFound, message: \"No such file or directory\" }")', libcore/result.rs:945:5

---- elements::module::integration_tests::peek_3 stdout ----
thread 'elements::module::integration_tests::peek_3' panicked at 'Should be deserialized: HeapOther("Can\'t read from the file: Os { code: 2, kind: NotFound, message: \"No such file or directory\" }")', libcore/result.rs:945:5

---- elements::module::integration_tests::peek_2 stdout ----
thread 'elements::module::integration_tests::peek_2' panicked at 'Should be deserialized: HeapOther("Can\'t read from the file: Os { code: 2, kind: NotFound, message: \"No such file or directory\" }")', libcore/result.rs:945:5

---- elements::module::integration_tests::serde_code stdout ----
thread 'elements::module::integration_tests::serde_code' panicked at 'Should be deserialized: HeapOther("Can\'t read from the file: Os { code: 2, kind: NotFound, message: \"No such file or directory\" }")', libcore/result.rs:945:5

---- elements::module::integration_tests::serde_import stdout ----
thread 'elements::module::integration_tests::serde_import' panicked at 'Should be deserialized: HeapOther("Can\'t read from the file: Os { code: 2, kind: NotFound, message: \"No such file or directory\" }")', libcore/result.rs:945:5

---- elements::module::integration_tests::serde_type stdout ----
thread 'elements::module::integration_tests::serde_type' panicked at 'Should be deserialized: HeapOther("Can\'t read from the file: Os { code: 2, kind: NotFound, message: \"No such file or directory\" }")', libcore/result.rs:945:5

---- elements::module::integration_tests::store stdout ----
thread 'elements::module::integration_tests::store' panicked at 'Should be deserialized: HeapOther("Can\'t read from the file: Os { code: 2, kind: NotFound, message: \"No such file or directory\" }")', libcore/result.rs:945:5

---- elements::module::integration_tests::varuint1_case stdout ----
thread 'elements::module::integration_tests::varuint1_case' panicked at 'Maybe shouldn't be deserialized: HeapOther("Can\'t read from the file: Os { code: 2, kind: NotFound, message: \"No such file or directory\" }")', libcore/result.rs:945:5

---- elements::reloc_section::tests::reloc_section stdout ----
thread 'elements::reloc_section::tests::reloc_section' panicked at 'Module should be deserialized: HeapOther("Can\'t read from the file: Os { code: 2, kind: NotFound, message: \"No such file or directory\" }")', libcore/result.rs:945:5

---- elements::section::tests::import_section stdout ----
thread 'elements::section::tests::import_section' panicked at 'Should be deserialized: HeapOther("Can\'t read from the file: Os { code: 2, kind: NotFound, message: \"No such file or directory\" }")', libcore/result.rs:945:5

Please either include them or somehow disable those tests by default. Thanks!

I suppose, looking at the code, a bool is always represented as u8, which is what we want anyway, but since these values are not bools but placeholder integers that must currently always be 0, having them be bools is kind of strange.

Replaced the reading/writing of them with VarUint7, which is maybe almost as invalid, but works.

Resolves #145.

Sorry I keep PR'ing API-breaking changes! 😅

First of all, thank you for providing this high quality crate.

I'm experimenting with adding UserFunctions that interpret a function that takes a pointer argument, e.g. a WASM program that calls extern void puts(const char*, size_t).

As far as I can tell, from the CallerContext<> argument to UserFunctionExecutor::execute there is no way to access the module (and therefore memory) from which the function was called.

Am I missing something, conceptually? Or is this functionality that can be added?

The parameters of a function should be put in the vector of local variables for the given function because the get_local instruction gets them like regular local variables. Any function which has local variables and parameters will not have them in the correct index of the locals vector because the index of a local variable should be offset by the parameters, which wasm counts as locals, as it should.

The solution is simple: add the parameters to the locals vector for each FuncBody. I would implement this change myself, but I'm not familiar with the code and I'm working on my final project. My final uses this library, which is how I stumbled upon this bug. For now I'll just implement a hack fix in my code, but a permanent fix would be nice.

This handles the recent changes to the upstream proposal, which renumbered and reorganized most of the opcodes, but AFAIK no functional change was intended.

The structure is described in this proposal. Rust can be used to create such modules by using a recent nightly that uses lld for the wasm32-unknown-unknown target and setting RUSTFLAGS = '-C link-arg=--relocatable'.

That said, this is not so useful without also parsing the linking metadata section described in the same doc. I implemented this one first and was going to implement that one next, but the design changed quite a bit as recently as two weeks ago so the section that Rust's version of lld emits is very different from what the document describes.

Given that all this relocation-related stuff is not finalized / standardized yet, I'll understand if you don't want to merge this as of this time. You can close it if you want.

I just hit a panic for 'attempt to negate with overflow' when deserializing a binary, the root cause was in the negation at the following location:

https://github.com/NikVolf/parity-wasm/blob/a4e2ec0adb7bf39da9558681f87c32095827c768/src/elements/primitives.rs#L303

This occurs when shift = 63.

I think you want to treat the value as unsigned until the bits are all populated, and then cast it to signed. Could the VarUint64 deserialize be reused here?

This issue might affect more of the VarInt deserializers as well.

To add additional UserFunction-s - now there are 16 and compiler/emscripten (?) uses this table to hold their indices.

It is interesting that memory has its own limits and instructions to increase actual memory size. Table has limits, but do not have any increase-like instructions. So it looks like we either have to reserve a number of slots for as much functions as we're going to have (I've increased this to 64), or grow table dynamically - when reading/writing from/to (haven't found anything about this in specification).

The simd! macro is mainly (133 times) used with 2 expressions. Sometimes (18 times) however, a 3rd is needed.

This PR does not fix all the clippy issues, the rest is addressed in #306 and #305.

Initially, simd! was defined as:

#[cfg(feature="simd")]
macro_rules! simd {
	($writer: expr, $byte: expr, $other:expr) => ({
		$writer.write(&[SIMD_PREFIX])?;
		VarUint32::from($byte).serialize($writer)?;
		$other;
	})
}

Linting the expansion of the macro results in grumbles in many cases similar to the one we can see below.

warning: statement with no effect
    --> src/elements/ops.rs:1711:3
     |
1711 |         $other;
     |         ^^^^^^^
...
2296 |             I32x4TruncUF32x4Sat => simd!(writer, I32X4_TRUNC_U_F32X4_SAT, ()),
     |                                    ------------------------------------------ in this macro invocation
     |
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#no_effect
     = note: this warning originates in the macro `simd` (in Nightly builds, run with -Z macro-backtrace for more info)

We have 2 options:

• disable the linting warnings
• have an additionnal macro

This PR proposes the second option. What do you think @pepyakin ? If you agree with the option and have better naming ideas, please let me know.