cmd/compile: "runtime: bad pointer in frame" in riscv64 with complier optimizations #51101
Comments
|
The js_parser is using unsafe to do pointer operation. |
|
Is this mostly deterministic? Does this always or almost always happen? If so, you could try just removing the unsafe code to see if it still happens. There is only a small amount of unsafe code in // The name is temporarily stored in the ref until the scope traversal pass
// happens, at which point a symbol will be generated and the ref will point
// to the symbol instead.
//
// The scope traversal pass will reconstruct the name using one of two methods.
// In the common case, the name is a slice of the file itself. In that case we
// can just store the slice and not need to allocate any extra memory. In the
// rare case, the name is an externally-allocated string. In that case we store
// an index to the string and use that index during the scope traversal pass.
func (p *parser) storeNameInRef(name string) js_ast.Ref {
- c := (*reflect.StringHeader)(unsafe.Pointer(&p.source.Contents))
- n := (*reflect.StringHeader)(unsafe.Pointer(&name))
-
- // Is the data in "name" a subset of the data in "p.source.Contents"?
- if n.Data >= c.Data && n.Data+uintptr(n.Len) < c.Data+uintptr(c.Len) {
- // The name is a slice of the file contents, so we can just reference it by
- // length and don't have to allocate anything. This is the common case.
- //
- // It's stored as a negative value so we'll crash if we try to use it. That
- // way we'll catch cases where we've forgotten to call loadNameFromRef().
- // The length is the negative part because we know it's non-zero.
- return js_ast.Ref{SourceIndex: -uint32(n.Len), InnerIndex: uint32(n.Data - c.Data)}
- } else {
// The name is some memory allocated elsewhere. This is either an inline
// string constant in the parser or an identifier with escape sequences
// in the source code, which is very unusual. Stash it away for later.
// This uses allocations but it should hopefully be very uncommon.
ref := js_ast.Ref{SourceIndex: 0x80000000, InnerIndex: uint32(len(p.allocatedNames))}
p.allocatedNames = append(p.allocatedNames, name)
return ref
- }
}The unsafe code is used to recover the index and length of a substring within the string. Reusing the memory with the unsafe code saves about 20mb of memory in esbuild's primary benchmark but it's not necessary for esbuild to function, and can be commented out. If the problem still happens with that commented out then it's not due to unsafe. |
|
Update: I just removed use of the |
|
Hi @mengzhuo, After removing the usage of
Yes. This always happens. To whom wants to test but without a development board (cc @evanw), This wiki page Setup Arch Linux RISC-V Development Environment can helps you setup an environment with QEMU. After the setup, run |
|
Updated: I got the same failure that just run js_parser test. Could you provide a mininal test (without yarn/nodejs parts) ? I can't install yarn on my unmatched box. |
|
Kindly cc @mknyszek @thanm @cherrymui |
cherrymui
added
NeedsInvestigation
|
The bad pointer is from a subtraction of two (valid) pointer values, from The temporary value |
|
Change https://go.dev/cl/385655 mentions this issue: |
|
@gopherbot please backport this to previous releases. This can cause runtime crashes. |
|
Backport issue(s) opened: #51198 (for 1.16), #51199 (for 1.17). Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases. |
dmitshur
added
NeedsFix
|
Change https://go.dev/cl/386474 mentions this issue: |
|
Change https://go.dev/cl/386475 mentions this issue: |
…ce on RISCV64 Pointer comparison is lowered to the following on RISCV64 (EqPtr x y) => (SEQZ (SUB <x.Type> x y)) The difference of two pointers (the SUB) should not be pointer type. Otherwise it can cause the GC to find a bad pointer. Updates #51101. Fixes #51198. Change-Id: I7e73c2155c36ff403c032981a9aa9cccbfdf0f64 Reviewed-on: https://go-review.googlesource.com/c/go/+/385655 Trust: Cherry Mui <cherryyz@google.com> Run-TryBot: Cherry Mui <cherryyz@google.com> Reviewed-by: Keith Randall <khr@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> (cherry picked from commit 1ed30ca) Reviewed-on: https://go-review.googlesource.com/c/go/+/386475
…ce on RISCV64 Pointer comparison is lowered to the following on RISCV64 (EqPtr x y) => (SEQZ (SUB <x.Type> x y)) The difference of two pointers (the SUB) should not be pointer type. Otherwise it can cause the GC to find a bad pointer. Updates #51101. Fixes #51199. Change-Id: I7e73c2155c36ff403c032981a9aa9cccbfdf0f64 Reviewed-on: https://go-review.googlesource.com/c/go/+/385655 Trust: Cherry Mui <cherryyz@google.com> Run-TryBot: Cherry Mui <cherryyz@google.com> Reviewed-by: Keith Randall <khr@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> (cherry picked from commit 1ed30ca) Reviewed-on: https://go-review.googlesource.com/c/go/+/386474
What version of Go are you using (
go version)?Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env)?go envOutputWhat did you do?
Reproduce: (you should have Node.js and Yarn pre-installed)
What did you expect to see?
esbuildworks as intended, like inamd64.What did you see instead?
The full output: http://fars.ee/4nkQ
I've tested the build options below. It seems the bug is caused by the optimizer?
esbuildcrashes-gcflags=all="-N"esbuildworks fine-gcflags=all="-l"esbuildworks fine-gcflags=all="-N -l"esbuildworks fineAlso, I'm working on a minimal reproduce.
The text was updated successfully, but these errors were encountered: