Add support for sign-extension instructions (post MVP feature)

[achille-roussel]

Hello,

I'm opening this issue to report an error I ran into where it appeared wazero was unable to instantiate an AssemblyScript program compiled with asc.

Here is the error I got:

functions: invalid function at index 106/280: invalid instruction

Tools like wasm2wat are able to read through the file without issues, so it appears correct tho I do not know whether they perform checks similar to those in wazero.

I'm attaching the program that triggered the error:
test.wasm.gz

Let me know if you need any other information.

[mathetake]

Hi, thank you for reporting. This looks like the binary contains the new Wasm instructions beyond w3c 1.0 WebAsssembly Specification. Notably I can see the binary uses, for example, i64.extend_i32_u which is newly introduced in 1.1 specification, but the version is not formally released under w3c.

We definitely would like to support the wider instructions vs we want to wait for this library AND WebAssembly spec to reach some maturity before we move onto these new features.

In the meantime, I guess asc should be able to limit these new features and produce Wasm 1.0 spec comatible binary. Could you try that mode?

Thank you!

[mathetake]

What confuses me though is that the binary header says it is 1.0 version binary.

In any case, *.extend_* instructions are pretty simple to support in wazero, so maybe we could have support for them, but I would like to carefully choose what to support and what not to among these new features as this project is in its early stage. @codefromthecrypt thoughts?

[achille-roussel]

Thanks for the quick and useful reply!

Adding --disable sign-extension to the asc command invocation produced a wasm program that wazero has no trouble reading 👍

[MaxGraey]

What confuses me though is that the binary header says it is 1.0 version binary.

1.1 is a spec edition. Binary header even with recently standardized post-MVP features like sign-extension, multi-values and etc should use version with number 1 (not 1.1 or something else). See reference encoder / decoder for example:
https://github.com/WebAssembly/spec/blob/main/interpreter/binary/encode.ml#L3

Instead WebAssembly use special optional feature section for detect post-MVP features

[mathetake]

What confuses me though is that the binary header says it is 1.0 version binary.

1.1 is a spec edition. Binary header even with recently standardized post-MVP features like sign-extension, multi-values and etc should use version with number 1 (not 1.1 or something else). See reference encoder / decoder for example: https://github.com/WebAssembly/spec/blob/main/interpreter/binary/encode.ml#L3

Instead WebAssembly use special optional feature section for detect post-MVP features

Thanks @MaxGraey, your comment really is helpful to us. Could you give me any pointer to Instead WebAssembly use special optional feature section for detect post-MVP features? Doc or something?

[codefromthecrypt]

We are pending work to clarify the API interfaces of the project, as well harden things around spec versions.

While it is common for some projects to mix and match specs with draft features, our first goal is to implement the actual specification (w3c https://www.w3.org/TR/wasm-core-1/), not a draft or something on github.

Layering on complexity of proposed features or tracking 1.1 drift prior to a solid 1.0 is a non-goal. We should report things that feel like bugs, such as using the same version encoding for 1.0 and 1.1 features when incompatible. Since there is a version encoding, it seems odd to conflate like that. Maybe no-one raised an issue, yet.

Anyway, the main point is that we should get 1.0 fully working and in good shape, watch out for anything we can with regards to future versions, including good error reporting and also raising bugs on https://github.com/WebAssembly/spec/ when we notice compatibility problems (such as version conflation in encoding)

[codefromthecrypt]

concretely, I think we'll do a lot better with error reporting and this is a good example of it. Knowing people are mixing and matching spec with pre-spec features, we can add some nice error handling to tell people what's going on (ex this is not yet supported) vs a cryptic error.

Once we get 1.0 working and enough hands to do the vastly larger feature set of 1.1, we can start implementing those things.

[MaxGraey]

@mathetake It's not yet standardized but LLVM for example may emit it:
https://github.com/LEA0317/llvm9-half/blob/626b2ae86adca116d2e49871e8a3cb9b106fd5db/llvm9.0.1/llvm/lib/Object/WasmObjectFile.cpp#L720

Which I guess usually locate in custom section and may be stripped. So it is best and most reliable to simply detect features during decoding encountering one or the other operation related to according feature.

[nullpo-head]

Apps written in Grain language also requires this sign-extension feature. JFYI cc @pims

[codefromthecrypt]

so if I understand correctly, this feature doesn't introduce new datatypes. This would require a new feature flag for sign-extension-ops which guards parsing the 5 new opcodes. Of course, they would need to be implemented in JIT and the interpreter.

WebAssembly/spec@e308ca2
https://github.com/WebAssembly/spec/blob/main/proposals/sign-extension-ops/Overview.md

[codefromthecrypt]

#324 corrects the error message.

Only this week, we added feature flags, so we can add >1.0 support technically. The whole team here are behind on 1.0 features, but @nullpo-head agreed to make time for this one since grain can't turn it off. Help wanted on other features.

[codefromthecrypt]

@pims do you mind testing latest commit with GrainLang? You'll have to disable bulk memory with --no-bulk-memory until #321 and use this to initialize wazero:

r := wazero.NewRuntimeWithConfig(wazero.NewRuntimeConfig().WithFeatureSignExtensionOps(true))

[pims]

Will do.

[pims]

@codefromthecrypt

go run cmd/cli.go hello.gr.wasm
2022/03/07 10:50:45 signature mimatch: i32i32i32i64i32_i32 != i32i32i64i32_i32
exit status 1

import Process from "sys/process"
import Array from "array"

print("==== Environment: ====")
match(Process.env()) {
	Ok(arr) => Array.forEach(print, arr),
	Err(err) => throw err
}


//print("==== Args: ====")
//Array.forEach( print, Process.argv())

grain --version
Grain cli 0.4.7
Grain compiler 0.4.6
wazero v0.0.0-20220307120532-336ebcf43668

hello.gr.wasm.zip

I've attached the grain program based on the source pasted above.

In case it's helpful:

package main

import (
	"bytes"
	"fmt"
	"log"
	"os"
	"strings"
	"time"

	"github.com/tetratelabs/wazero"
)

func main() {
	now := time.Now()
	src := os.Args[1]
	source, _ := os.ReadFile(src)
	rt := wazero.NewRuntimeWithConfig(wazero.NewRuntimeConfig().WithFeatureSignExtensionOps(true))

	decoded, err := rt.DecodeModule(source)
	if err != nil {
		log.Fatal(err)
	}

	stdinBuf := os.Stdin
	stdoutBuf := bytes.NewBuffer(nil)
	stderrBuf := bytes.NewBuffer(nil)

	wasiConfig := &wazero.WASIConfig{
		Stdin:   stdinBuf,
		Stdout:  stdoutBuf,
		Stderr:  stderrBuf,
		Args:    []string{"foo", "bar=baz"},
		Environ: map[string]string{},
	}

	if _, err := rt.NewHostModule(wazero.WASISnapshotPreview1WithConfig(wasiConfig)); err != nil {
		log.Fatal(err)
	}
	if _, err := wazero.StartWASICommand(rt, decoded); err != nil {
		log.Fatal(err)
	}

	fmt.Print(strings.TrimSpace(stdoutBuf.String()))
	fmt.Printf("took: %s\n", time.Since(now))
}

[codefromthecrypt]

This is helpful @pims I'll use this to help get better error messages (identifying >1.0 opcodes) in the very least!

[codefromthecrypt]

ps haven't forgotten about this, just swamped with other things. I'll update by Weds or nag me!