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.

Sign up for GitHub

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

Jump to bottom

syscall/js: call of Value.Int on undefined on win32 using node.js 10.7.0 #26524

Closed
DHowett opened this issue Jul 22, 2018 · 5 comments
Closed

syscall/js: call of Value.Int on undefined on win32 using node.js 10.7.0 #26524

DHowett opened this issue Jul 22, 2018 · 5 comments
Labels
arch-wasm WebAssembly issues FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows

Comments

@DHowett
Copy link

DHowett commented Jul 22, 2018 •
edited

What version of Go are you using (go version)?

go version go1.11beta2 windows/amd64

What operating system and processor architecture are you using (go env)?

% go1.11beta2 env
set GOARCH=wasm
set GOBIN=
set GOCACHE=C:\Users\Dustin\AppData\Local\go-build
set GOEXE=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=js
set GOPATH=C:\Users\Dustin\Projects\go
set GOPROXY=
set GORACE=
set GOROOT=C:\Users\Dustin\sdk\go1.11beta2
set GOTMPDIR=
set GOTOOLDIR=C:\Users\Dustin\sdk\go1.11beta2\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set CXX=g++
set CGO_ENABLED=0
set GOMOD=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-fPIC -fmessage-length=0 -fdebug-prefix-map=C:\Users\Dustin\AppData\Local\Temp\go-build711284174=/tmp/go-build -gno-record-gcc-switches

% node.exe --version
v10.7.0

What did you do?

% $Env:GOOS="js"; $Env:GOARCH="wasm"
% cat test.go
package main

import "fmt"

func main() {
        fmt.Println("vim-go")
}
% go1.11beta2 build -o test.wasm
% cp C:\Users\Dustin\sdk\go1.11beta2\misc\wasm\wasm_exec.js .
% node.exe wasm_exec.js test.wasm

What did you expect to see?

vim-go

What did you see instead?

panic: syscall/js: call of Value.Int on undefined

goroutine 1 [running]:
syscall/js.Value.float(0x7ff8000000000001, 0x2adde, 0x9, 0x7ff8000000000001)
        C:/Users/Dustin/sdk/go1.11beta2/src/syscall/js/js.go:306 +0x35
syscall/js.Value.Int(0x7ff8000000000001, 0x2afad)
        C:/Users/Dustin/sdk/go1.11beta2/src/syscall/js/js.go:318 +0x2

Surprisingly, the same wasm binary on the same version of node works correctly on Node on Linux:

% docker run -ti --rm -v ${PWD}:/n node:latest bash
root@dbc7fd32409d:/# uname
Linux
root@dbc7fd32409d:/# node --version
v10.7.0
root@dbc7fd32409d:/# node /n/wasm_exec.js /n/test.wasm
vim-go
@agnivade agnivade changed the title js/wasm build products fail to launch on win32 node.js 10.7.0 syscall/js: call of Value.Int on undefined on win32 using node.js 10.7.0 Jul 22, 2018
@agnivade agnivade added OS-Windows arch-wasm WebAssembly issues NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jul 22, 2018
@agnivade
Copy link
Contributor

The stack trace coming from syscall/js is itself a bit strange when the code does not use syscall/js at all. It seems like the nodejs windows environment is somehow operating on an undefined js.Value, which is not very clear from the stack trace.

@DHowett
Copy link
Author

DHowett commented Jul 28, 2018 •
edited

It turns out that (node) fs on Windows does not include constants for O_NONBLOCK and O_SYNC. The initializer here demands that they exist, and we panic during syscall.init if they don't.

This is sufficient to ensure that all constants are present:

diff --git a/misc/wasm/wasm_exec.js b/misc/wasm/wasm_exec.js
index 02a753c..71200cb 100644
--- a/misc/wasm/wasm_exec.js
+++ b/misc/wasm/wasm_exec.js
@@ -4,11 +4,21 @@
 
 (() => {
 	// Map web browser API and Node.js API to a single common API (preferring web standards over Node.js API).
+	const requiredFsConstants = { O_WRONLY: -1, O_RDWR: -1, O_CREAT: -1, O_TRUNC: -1, O_APPEND: -1, O_EXCL: -1, O_NONBLOCK: -1, O_SYNC: -1 };
 	const isNodeJS = typeof process !== "undefined";
 	if (isNodeJS) {
 		global.require = require;
 		global.fs = require("fs");
 
+		Object.entries(requiredFsConstants).forEach(([key, value]) => {
+			if (!(key in global.fs.constants)) {
+				Object.defineProperty(global.fs.constants, key, {
+					value: value,
+					writable: false
+				});
+			}
+		});
+
 		const nodeCrypto = require("crypto");
 		global.crypto = {
 			getRandomValues(b) {
@@ -37,7 +47,7 @@
 
 		let outputBuf = "";
 		global.fs = {
-			constants: { O_WRONLY: -1, O_RDWR: -1, O_CREAT: -1, O_TRUNC: -1, O_APPEND: -1, O_EXCL: -1, O_NONBLOCK: -1, O_SYNC: -1 }, // unused
+			constants: requiredFsConstants, // unused
 			writeSync(fd, buf) {
 				outputBuf += decoder.decode(buf);
 				const nl = outputBuf.lastIndexOf("\n");

@DHowett
Copy link
Author

DHowett commented Jul 28, 2018

(The comment that the constants are unused is also manifestly false 😄)

@agnivade
Copy link
Contributor

Thanks for the analysis @DHowett ! Would you like to send a patch and then @neelance can take a look at it ?

@bradfitz - Does it make sense to run wasm builders on windows machines too so that we can weed out issues like this ?

@gopherbot
Copy link

Change https://golang.org/cl/126600 mentions this issue: syscall: remove support for O_NONBLOCK and O_SYNC on js/wasm

jeet-parekh pushed a commit to jeet-parekh/go that referenced this issue Jul 31, 2018
@neelance @jeet-parekh
syscall: remove support for O_NONBLOCK and O_SYNC on js/wasm
db2afe7 
This commit removes O_NONBLOCK on js/wasm. O_SYNC can't be
removed, because it is referenced by the os package, so instead
its use returns an error.

On Windows, the options O_NONBLOCK and O_SYNC are not available
when opening a file with Node.js. This caused the initialization
of the syscall package to panic.

The simplest solution is to not support these two options on js/wasm
at all. Code written for js/wasm is supposed to be portable,
so platform-specific options should not be used.

Fixes golang#26524.

Change-Id: I366aa3cdcfa59dfa9dc513368259f363ca090f00
Reviewed-on: https://go-review.googlesource.com/126600
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Jul 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
arch-wasm WebAssembly issues FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@DHowett @agnivade @gopherbot