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

cmd/compile: Maximum call stack size exceeded testing go/parser with Node.js v18.7.0 #56498

Closed
bcmills opened this issue Oct 31, 2022 · 6 comments
Assignees
Labels
arch-wasm WebAssembly issues compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-JS
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Oct 31, 2022

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

$ gotip version
go version devel go1.20-e09bbaec69 Sat Oct 29 04:48:07 2022 +0000 linux/amd64

Does this issue reproduce with the latest release?

Yes: reproduces with go1.19.2.

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

go env Output
$ gotip env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/usr/local/google/home/bcmills/.cache/go-build"
GOENV="/usr/local/google/home/bcmills/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/tmp/tmp.BkSnRHYFHZ/.gopath/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/tmp/tmp.BkSnRHYFHZ/.gopath"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/google/home/bcmills/sdk/gotip"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/google/home/bcmills/sdk/gotip/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="devel go1.20-e09bbaec69 Sat Oct 29 04:48:07 2022 +0000"
GCCGO="/usr/local/google/home/bcmills/bin/gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="c++"
CGO_ENABLED="1"
GOMOD="/tmp/tmp.BkSnRHYFHZ/go.mod"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2991106879=/tmp/go-build -gno-record-gcc-switches"

What did you do?

$ GOOS=js GOARCH=wasm go test go/parser

What did you expect to see?

Tests passing.

What did you see instead?

It appears that some function being executed by the test is compiled in a way that runs afoul of Node's default limits on stack depth.

$ GOOS=js GOARCH=wasm go test go/parser
wasm://wasm/01129cfe:1


RangeError: Maximum call stack size exceeded
    at wasm://wasm/01129cfe:wasm-function[2020]:0x1e73d9
    at wasm://wasm/01129cfe:wasm-function[2333]:0x23f11a
    at wasm://wasm/01129cfe:wasm-function[2336]:0x23fd71
    at wasm://wasm/01129cfe:wasm-function[2406]:0x254b40
    at wasm://wasm/01129cfe:wasm-function[2425]:0x25a280
    at wasm://wasm/01129cfe:wasm-function[2428]:0x25bdf9
    at wasm://wasm/01129cfe:wasm-function[2431]:0x25cb53
    at wasm://wasm/01129cfe:wasm-function[2434]:0x25d618
    at wasm://wasm/01129cfe:wasm-function[2436]:0x25d8ac
    at wasm://wasm/01129cfe:wasm-function[2360]:0x244ad8

Node.js v18.7.0
FAIL    go/parser       0.461s
FAIL

(attn @golang/js; CC @golang/compiler)

@bcmills bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. arch-wasm WebAssembly issues OS-JS labels Oct 31, 2022
@bcmills bcmills added this to the Backlog milestone Oct 31, 2022
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Oct 31, 2022
@bcmills
Copy link
Contributor Author

bcmills commented Oct 31, 2022

I should note that this failure mode is 100% reproducible for me. (Please let me know if you need me to run any commands for diagnostics!)

@rsc
Copy link
Contributor

rsc commented Oct 31, 2022

Reproducing on https://go.dev/cl/446178 now too. Not sure what the workaround is.

@rsc rsc self-assigned this Oct 31, 2022
@rsc
Copy link
Contributor

rsc commented Oct 31, 2022

Ran go test -v encoding/xml on a js-wasm builder and it is dying in one of the very deep unmarshal tests. Cutting the depth limit in half fixes the problem, which as far as I am concerned is good enough.

@gopherbot
Copy link

Change https://go.dev/cl/446639 mentions this issue: encoding/xml: reduce depth limit on wasm

gopherbot pushed a commit that referenced this issue Oct 31, 2022
Wasm can't handle the recusion for XML nested to depth 10,000.
Cut it off at 5,000 instead. This fixes TestCVE202228131 on trybots
in certain conditions.

Also disable TestCVE202230633 to fix 'go test -v encoding/xml' on gomotes.

Also rename errExeceededMaxUnmarshalDepth [misspelled and unwieldy]
to errUnmarshalDepth.

For #56498.

Change-Id: I7cc337ccfee251bfd9771497be0e5272737114f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/446639
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Nov 3, 2022
Wasm can't handle the recusion for XML nested to depth 10,000.
Cut it off at 5,000 instead. This fixes TestCVE202228131 on trybots
in certain conditions.

Also disable TestCVE202230633 to fix 'go test -v encoding/xml' on gomotes.

Also rename errExeceededMaxUnmarshalDepth [misspelled and unwieldy]
to errUnmarshalDepth.

For golang#56498.

Change-Id: I7cc337ccfee251bfd9771497be0e5272737114f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/446639
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
@johanbrandhorst
Copy link
Member

This is now blocking #57614 so I'll be looking at this too.

@johanbrandhorst johanbrandhorst self-assigned this Feb 1, 2023
@gopherbot
Copy link

Change https://go.dev/cl/463994 mentions this issue: misc: increase node stack size

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Feb 1, 2023
@dmitshur dmitshur modified the milestones: Backlog, Go1.21 Feb 1, 2023
johanbrandhorst added a commit to Pryz/go that referenced this issue Feb 12, 2023
The default NodeJS V8 stack size is 984K, which is not enough to run
the regexp or go/parser tests. This commit increases the stack size
to 8192K, which removes the stack size limit error.

Fixes golang#56498
Fixes golang#57614

Change-Id: I357885d420daf259187403deab25ff587defa0fc
Reviewed-on: https://go-review.googlesource.com/c/go/+/463994
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Julien Fabre <ju.pryz@gmail.com>
Auto-Submit: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly issues compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-JS
Projects
None yet
Development

No branches or pull requests

5 participants