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

x/tools/gopls: panic in aeshashbody on behalf of mapaccess2_faststr: string memory corrupted #71367

Open
adonovan opened this issue Jan 21, 2025 · 6 comments
Labels
BugReport Issues describing a possible bug in the Go implementation. gopls/telemetry-wins gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@adonovan
Copy link
Member

adonovan commented Jan 21, 2025

#!stacks
"sigpanic" && "aeshashbody" && "mapaccess2_faststr" && "gopls"

Issue created by stacks.

The source line aeshashbody:+233 is nominally 1468, but AESENC is infallible. Assuming there's an off-by-one error in computing the relative lines for assembly functions, it could conceivably be the following load instruction, which would mean that a Go string points to data that runs off the end of a memory mapping. I have no idea what is going on. @prattmic

[Update: a bad string was passed to a map. Perhaps produced by a data race?]

  asm_amd64.s:1462      0x47a76c                660f38dcc9              AESENC X1, X1                           
  asm_amd64.s:1463      0x47a771                660f38dcd2              AESENC X2, X2                           
  asm_amd64.s:1464      0x47a776                660f38dcdb              AESENC X3, X3                           
  asm_amd64.s:1465      0x47a77b                660f38dce4              AESENC X4, X4                           
  asm_amd64.s:1466      0x47a780                660f38dced              AESENC X5, X5                           
  asm_amd64.s:1467      0x47a785                660f38dcf6              AESENC X6, X6                           
  asm_amd64.s:1468      0x47a78a                660f38dcff              AESENC X7, X7                           
  asm_amd64.s:1471      0x47a78f                f3440f6f440880          MOVDQU -0x80(AX)(CX*1), X8              
  asm_amd64.s:1472      0x47a796                f3440f6f4c0890          MOVDQU -0x70(AX)(CX*1), X9              
  asm_amd64.s:1473      0x47a79d                f3440f6f5408a0          MOVDQU -0x60(AX)(CX*1), X10             
  asm_amd64.s:1474      0x47a7a4                f3440f6f5c08b0          MOVDQU -0x50(AX)(CX*1), X11             
  asm_amd64.s:1475      0x47a7ab                f3440f6f6408c0          MOVDQU -0x40(AX)(CX*1), X12             
  asm_amd64.s:1476      0x47a7b2                f3440f6f6c08d0          MOVDQU -0x30(AX)(CX*1), X13             
  asm_amd64.s:1477      0x47a7b9                f3440f6f7408e0          MOVDQU -0x20(AX)(CX*1), X14             
  asm_amd64.s:1478      0x47a7c0                f3440f6f7c08f0          MOVDQU -0x10(AX)(CX*1), X15            

This stack 8jk_dA was reported by telemetry:

golang.org/x/tools/[email protected] go1.23.4 linux/amd64 vscode (1)
@adonovan adonovan added gopls Issues related to the Go language server, gopls. gopls/telemetry-wins NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository. labels Jan 21, 2025
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jan 21, 2025
@gabyhelp gabyhelp added the BugReport Issues describing a possible bug in the Go implementation. label Jan 21, 2025
@prattmic
Copy link
Member

In person, Alan was saying that some of this string must have already been read (hence the assumption that it crosses a page boundary), but actually https://cs.opensource.google/go/go/+/master:src/runtime/asm_amd64.s;l=1471;drc=7f9edb42259114020c67eb51643e43cf5a2cf9a7 (the off-by-one load) is the first load of AX in the function, so it could be any arbitrary bad address.

@randall77
Copy link
Contributor

I can reproduce a crash at the suspected line with:

package main

import "unsafe"

type str struct {
	b *byte
	n int
}

func main() {
	var s string
	(*str)(unsafe.Pointer(&s)).n = 200
	m := map[string]int{}
	m[s] = 7
}

Alan, can you reproduce the off-by-one-instruction thing with stacks using this program? Just to check that we're actually looking at the right problem.

@adonovan
Copy link
Member Author

actually https://cs.opensource.google/go/go/+/master:src/runtime/asm_amd64.s;l=1471;drc=7f9edb42259114020c67eb51643e43cf5a2cf9a7 (the off-by-one load) is the first load of AX in the function, so it could be any arbitrary bad address.

Good catch! Sorry, I was lazy reading the code and assumed from the large func-relative offset and the AESENC before a load that we were in the middle of the loop, but in fact we're just priming the pump.

Alan, can you reproduce the off-by-one-instruction thing with stacks using this program? Just to check that we're actually looking at the right problem.

Yeah, it would be good to definitively resolve some of these questions. I feel bad looping in compiler+runtime folks each time we get an unexplained crash, which has started happening a lot in recent weeks, and Rob has a plausible theory about a data race tearing a string (which is even more plausible if this crash works for any bad string pointer).

Will test presently.

@adonovan
Copy link
Member Author

I spliced that code into gopls' main and got this system traceback:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x48 pc=0x102d73914]

goroutine 1 gp=0x140000021c0 m=0 mp=0x10439d300 [running]:
panic({0x103ad7b80?, 0x10435f7f0?})
	/Users/adonovan/w/goroot/src/runtime/panic.go:806 +0x154 fp=0x14000043c80 sp=0x14000043bd0 pc=0x102d6e0e4
runtime.panicmem(...)
	/Users/adonovan/w/goroot/src/runtime/panic.go:262
runtime.sigpanic()
	/Users/adonovan/w/goroot/src/runtime/signal_unix.go:925 +0x300 fp=0x14000043ce0 sp=0x14000043c80 pc=0x102d707c0
aeshashbody()
	/Users/adonovan/w/goroot/src/runtime/asm_arm64.s:855 +0x2d4 fp=0x14000043cf0 sp=0x14000043cf0 pc=0x102d73914
runtime.mapassign_faststr(0x103af3480, 0x14000043ef8, {0x0, 0xc8})
	/Users/adonovan/w/goroot/src/internal/runtime/maps/runtime_faststr_swiss.go:277 +0x78 fp=0x14000043dc0 sp=0x14000043cf0 pc=0x102d030a8
main.main()
	/Users/adonovan/w/xtools/gopls/main.go:46 +0xec fp=0x14000043f40 sp=0x14000043dc0 pc=0x103683cec
runtime.main()
	/Users/adonovan/w/goroot/src/runtime/proc.go:283 +0x284 fp=0x14000043fd0 sp=0x14000043f40 pc=0x102d38eb4
runtime.goexit({})
	/Users/adonovan/w/goroot/src/runtime/asm_arm64.s:1260 +0x4 fp=0x14000043fd0 sp=0x14000043fd0 pc=0x102d766f4

where asm_arm64.s:855 denotes the indicated load instruction:

aes129plus:
	PRFM (R0), PLDL1KEEP
	VLD1.P	64(R4), [V1.B16, V2.B16, V3.B16, V4.B16]
	VLD1	(R4), [V5.B16, V6.B16, V7.B16]
	AESE	V30.B16, V1.B16
	AESMC	V1.B16, V1.B16
	AESE	V30.B16, V2.B16
	AESMC	V2.B16, V2.B16
	AESE	V30.B16, V3.B16
	AESMC	V3.B16, V3.B16
	AESE	V30.B16, V4.B16
	AESMC	V4.B16, V4.B16
	AESE	V30.B16, V5.B16
	AESMC	V5.B16, V5.B16
	AESE	V30.B16, V6.B16
	AESMC	V6.B16, V6.B16
	AESE	V30.B16, V7.B16
	AESMC	V7.B16, V7.B16
	ADD	R0, R2, R10
	SUB	$128, R10, R10
	VLD1.P	64(R10), [V8.B16, V9.B16, V10.B16, V11.B16] <--- here
	VLD1	(R10), [V12.B16, V13.B16, V14.B16, V15.B16]
	SUB	$1, R2, R2
	LSR	$7, R2, R2

In the telemetry database, we get:

{
        "Meta": {
                "GOARCH": "arm64",
                "GOOS": "darwin",
                "GoVersion": "devel",
                "Program": "golang.org/x/tools/gopls",
                "TimeBegin": "2025-01-21T00:00:00Z",
                "TimeEnd": "2025-01-25T00:00:00Z",
                "Version": "devel"
        },
        "Count": {
                "crash/crash\nruntime.gopanic:+69\nruntime.panicmem:=262\nruntime.sigpanic:+19\n.aeshashbody:+220\nruntime.mapassign_faststr:+14\nmain.main:+16\nruntime.main:+136\nruntime.goexit:+0": 1,
                "gopls/client:eglot": 1,
                "gopls/definition/latency:\u003c10ms": 1,
                "gopls/gotoolchain:auto": 1,
                "gopls/goversion:1.24": 1,
                "gopls/hover/latency:\u003c10ms": 14,
                "gopls/telemetryprompt/attempts:0": 1
        }
}

where the line aeshashbody:+220, starting counting at zero for the first instruction in the function body, corresponds to the same VLD1.P instruction. So that's good: the telemetry data is consistent with the traceback (no off-by-one), and the implicated instruction is the load, and it fails for any invalid string data.

Thanks for your help and sorry for the false alarm.

@adonovan adonovan changed the title runtime: panic in aeshashbody (!) on behalf of mapaccess2_faststr x/tools/gopls: panic in aeshashbody on behalf of mapaccess2_faststr: string memory corrupted Jan 21, 2025
@gopherbot gopherbot added this to the Unreleased milestone Jan 21, 2025
@adonovan adonovan removed the compiler/runtime Issues related to the Go compiler and/or runtime. label Jan 21, 2025
@randall77
Copy link
Contributor

amd64 might have different line number behavior than arm64.

@findleyr findleyr modified the milestones: Unreleased, gopls/v0.18.1 Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BugReport Issues describing a possible bug in the Go implementation. gopls/telemetry-wins gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

6 participants