internal/coverage: runtime.(*wbBuf).putFast SEGV in cover-related cmd/go tests on 386 #56044

thanm · 2022-10-04T21:34:32Z

#!watchflakes
post <- pkg == "cmd/go" && test == "TestScript/cover_atomic_pkgall" && `SIGSEGV`

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

$ go version
go version devel go1.20-c2ac7aa67d Tue Oct 4 11:45:13 2022 -0400 linux/amd64

Does this issue reproduce with the latest release?

Only on tip.

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

This is running with GOARCH=386 on a linux/amd64 machine.

What did you do?

$ cd $GOROOT/src/cmd/go
$ GOARCH=386 go test -c -o t.exe .
$  GOARCH=386 ./t.exe -test.count=1000000 -test.run="Script/^cover_atomic_pkgall$" 
...
$

What did you expect to see?

Clean run.

What did you see instead?

Failures of this form (note: the details of interest are elided by watchflakes, click through to the log to see the real failures):

2022-09-30 23:00 linux-386-longtest go@76c1a501 cmd/go.TestScript (log)

go test proxy running at GOPROXY=http://127.0.0.1:38617/mod
--- FAIL: TestScript (0.01s)
    --- FAIL: TestScript/cover_atomic_pkgall (6.28s)
        script_test.go:302: 
            # (2022-09-30T23:33:47Z)
            > env GO111MODULE=off
            > [short] skip
            > go test -coverpkg=all -covermode=atomic x
            [stdout]
            PASS
            panic: runtime error: invalid memory address or nil pointer dereference
            [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x80a8991]

            goroutine 1 [running]:
            panic({0x81f4880, 0x8334110})
            	/workdir/go/src/runtime/panic.go:987 +0x673 fp=0x88558b0 sp=0x8855854 pc=0x80b0f73
            runtime.panicmem()
            	/workdir/go/src/runtime/panic.go:260 +0x7e fp=0x88558c0 sp=0x88558b0 pc=0x80ae67e
            runtime.sigpanic()
            	/workdir/go/src/runtime/signal_unix.go:837 +0x270 fp=0x88558d8 sp=0x88558c0 pc=0x80d9ec0
...
            testing.(*M).Run.func4()
            	/workdir/go/src/testing/testing.go:1775 +0x27 fp=0x8855e84 sp=0x8855e7c pc=0x81c05e7
            testing.(*M).Run(0x886e0a0)
            	/workdir/go/src/testing/testing.go:1822 +0xbbc fp=0x8855f6c sp=0x8855e84 pc=0x81c023c
            main.main()
            	_testmain.go:78 +0x105 fp=0x8855fc4 sp=0x8855f6c pc=0x81df725
            runtime.main()
            	/workdir/go/src/runtime/proc.go:250 +0x3c8 fp=0x8855ff0 sp=0x8855fc4 pc=0x80b5f68
            runtime.goexit()
            	/workdir/go/src/runtime/asm_386.s:1326 +0x1 fp=0x8855ff4 sp=0x8855ff0 pc=0x8108dd1

2022-09-30 23:03 linux-386-longtest go@aeab76f0 cmd/go.TestScript (log)

go test proxy running at GOPROXY=http://127.0.0.1:46409/mod
--- FAIL: TestScript (0.01s)
    --- FAIL: TestScript/cover_atomic_pkgall (8.22s)
        script_test.go:302: 
            # (2022-09-30T23:35:17Z)
            > env GO111MODULE=off
            > [short] skip
            > go test -coverpkg=all -covermode=atomic x
            [stdout]
            PASS
            panic: runtime error: invalid memory address or nil pointer dereference
            [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x80a8991]

            goroutine 1 [running]:
            panic({0x81f4880, 0x8334110})
            	/workdir/go/src/runtime/panic.go:987 +0x673 fp=0x88558b0 sp=0x8855854 pc=0x80b0f73
            runtime.panicmem()
            	/workdir/go/src/runtime/panic.go:260 +0x7e fp=0x88558c0 sp=0x88558b0 pc=0x80ae67e
            runtime.sigpanic()
            	/workdir/go/src/runtime/signal_unix.go:837 +0x270 fp=0x88558d8 sp=0x88558c0 pc=0x80d9ec0
...
            testing.(*M).Run.func4()
            	/workdir/go/src/testing/testing.go:1775 +0x27 fp=0x8855e84 sp=0x8855e7c pc=0x81c05e7
            testing.(*M).Run(0x88660a0)
            	/workdir/go/src/testing/testing.go:1822 +0xbbc fp=0x8855f6c sp=0x8855e84 pc=0x81c023c
            main.main()
            	_testmain.go:78 +0x105 fp=0x8855fc4 sp=0x8855f6c pc=0x81df725
            runtime.main()
            	/workdir/go/src/runtime/proc.go:250 +0x3c8 fp=0x8855ff0 sp=0x8855fc4 pc=0x80b5f68
            runtime.goexit()
            	/workdir/go/src/runtime/asm_386.s:1326 +0x1 fp=0x8855ff4 sp=0x8855ff0 pc=0x8108dd1

These were found by watchflakes and noticed by @bcmills.

By running the test in question in a loop, I've seen a couple of failures out of about 1000 runs, so it is definitely possible to reproduce by hand with some patience.

I am not immediately clear on what's happening here, or how this might related to code coverage, but it needs to be looked at.

The text was updated successfully, but these errors were encountered:

thanm · 2022-10-10T17:43:48Z

I was able to narrow this down to just the runtime package, and then eventually with some debugging code (to selectively turn off coverage instrumentation) to the method runtime.arenaIdx.l1 in mheap.go. There's really not much to this method:

func (i arenaIdx) l1() uint {
	if arenaL1Bits == 0 {
		// Let the compiler optimize this away if there's no
		// L1 map.
		return 0
	} else {
		return uint(i) >> arenaL1Shift
	}
}

but more crucially what seems to be happening here is that the presence of coverage instrumentation was preventing the method from being inlined. This in turn was due to a weakness in the code that I added to the inliner back in CL 401235.

The real problem however is that arenaIdx.l1() is not marked as nosplit, even though it is called from other nosplit functions (e.g. spanOf and friends).

gopherbot · 2022-10-10T18:52:16Z

Change https://go.dev/cl/441858 mentions this issue: cmd/compile: tweak inliners handling of coverage counter updates

gopherbot · 2022-10-10T18:52:16Z

Change https://go.dev/cl/441859 mentions this issue: runtime: mark arenaIdx.l1 and arenaIdx.l2 methods as nosplit

This patch fixes up a bug in the inliner's special case code for coverage counter updates, which was not properly working for -covermode=atomic compilations. Updates #56044. Change-Id: I9e309312b123121c3df02862623bdbab1f6c6a4b Reviewed-on: https://go-review.googlesource.com/c/go/+/441858 Reviewed-by: David Chase <drchase@google.com> Run-TryBot: Than McIntosh <thanm@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>

This patch fixes up a bug in the inliner's special case code for coverage counter updates, which was not properly working for -covermode=atomic compilations. Updates golang#56044. Change-Id: I9e309312b123121c3df02862623bdbab1f6c6a4b Reviewed-on: https://go-review.googlesource.com/c/go/+/441858 Reviewed-by: David Chase <drchase@google.com> Run-TryBot: Than McIntosh <thanm@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>

Mark the "l1" and "l2" methods on "arenaIdx" with //go:nosplit, since these methods are called from a nosplit context (for example, from "spanOf"). Fixes golang#56044. Updates golang#21314. Change-Id: I48c7aa756b59a13162c89ef21066f83371ae50f1 Reviewed-on: https://go-review.googlesource.com/c/go/+/441859 Reviewed-by: Cherry Mui <cherryyz@google.com> Run-TryBot: Than McIntosh <thanm@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>

thanm added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker labels Oct 4, 2022

thanm added this to the Go1.20 milestone Oct 4, 2022

thanm self-assigned this Oct 4, 2022

thanm mentioned this issue Oct 10, 2022

cmd/compile, runtime: add and use go:nosplitrec compilation directive #21314

Open

gopherbot closed this as completed in 506e690 Oct 10, 2022

This was referenced Oct 17, 2022

runtime: SIGSEGV in runtime.sigpanic during cmd/go.TestScript/cover_atomic_pkgall #56277

Closed

cmd/go: TestScript failures #55311

Closed

golang locked and limited conversation to collaborators Oct 17, 2023

gopherbot added the FrozenDueToAge label Oct 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

internal/coverage: runtime.(*wbBuf).putFast SEGV in cover-related cmd/go tests on 386 #56044

internal/coverage: runtime.(*wbBuf).putFast SEGV in cover-related cmd/go tests on 386 #56044

thanm commented Oct 4, 2022 •

edited by bcmills

thanm commented Oct 10, 2022

gopherbot commented Oct 10, 2022

gopherbot commented Oct 10, 2022

internal/coverage: runtime.(*wbBuf).putFast SEGV in cover-related cmd/go tests on 386 #56044

internal/coverage: runtime.(*wbBuf).putFast SEGV in cover-related cmd/go tests on 386 #56044

Comments

thanm commented Oct 4, 2022 • edited by bcmills

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

Does this issue reproduce with the latest release?

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

What did you do?

What did you expect to see?

What did you see instead?

thanm commented Oct 10, 2022

gopherbot commented Oct 10, 2022

gopherbot commented Oct 10, 2022

thanm commented Oct 4, 2022 •

edited by bcmills

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

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