Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime.InstantiateModule returns a "struct-wrapped" nil value #2353

Open
edman opened this issue Dec 25, 2024 · 0 comments · May be fixed by #2354
Open

runtime.InstantiateModule returns a "struct-wrapped" nil value #2353

edman opened this issue Dec 25, 2024 · 0 comments · May be fixed by #2354
Labels
bug Something isn't working

Comments

@edman
Copy link

edman commented Dec 25, 2024

Describe the bug
runtime.InstantiateModule sometimes returns a "struct-wrapped" nil value, instead of returning actual nil.

More specifically, this line https://github.com/tetratelabs/wazero/blob/main/runtime.go#L318 assigns mod and returns it implicitly on L324.

This allows code like the following to panic with a nil pointer dereference because m is nil:

m, _ := r.InstantiateModule(ctx, compiled, config)
if m != nil {      // this check passes
  defer m.Close()  // and yet this causes a nil pointer dereference
}

To Reproduce
Here is an example that mimics wazero's behavior: https://go.dev/play/p/66QShXidalq

Expected behavior
From https://go.dev/doc/faq#nil_error, the FAQ recommends:

To return a proper nil error to the caller, the function must return an explicit nil

Additional context
Found on sqlc-dev/sqlc#3759 (comment)

@edman edman added the bug Something isn't working label Dec 25, 2024
edman added a commit to edman/wazero that referenced this issue Dec 25, 2024
…bs#2353)

Previously InstantiateModule could return a (*wasm.ModuleInstance)(nil)
instead of a proper nil value.

So callers could get a nil dereference panic in code like:

        m, _ := r.InstantiateModule(...)
        if m != nil {           // this check passes
                defer m.Close() // but this causes a panic
        }
edman added a commit to edman/wazero that referenced this issue Dec 25, 2024
…bs#2353)

Previously InstantiateModule could return a (*wasm.ModuleInstance)(nil)
instead of a proper nil value.

So callers could get a nil dereference panic in code like:

        m, _ := r.InstantiateModule(...)
        if m != nil {           // this check passes
                defer m.Close() // but this causes a panic
        }
edman added a commit to edman/wazero that referenced this issue Dec 25, 2024
…bs#2353)

Previously InstantiateModule could return a (*wasm.ModuleInstance)(nil)
instead of a proper nil value.

So callers could get a nil dereference panic in code like:

        m, _ := r.InstantiateModule(...)
        if m != nil {           // this check passes
                defer m.Close() // but this causes a panic
        }
edman added a commit to edman/wazero that referenced this issue Dec 25, 2024
…bs#2353)

Previously InstantiateModule could return a (*wasm.ModuleInstance)(nil)
instead of a proper nil value.

So callers could get a nil dereference panic in code like:

        m, _ := r.InstantiateModule(...)
        if m != nil {           // this check passes
                defer m.Close() // but this causes a panic
        }

Signed-off-by: Edman Anjos <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant