runtime: gccheckmark does nothing?

MikeMitchellWebDev commented 2 months ago

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

$ go version
go version go1.23.0 darwin/amd64

Does this issue reproduce with the latest release?

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

go env Output

$ go env
GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/Users/mm/Library/Caches/go-build'
GOENV='/Users/mm/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/mm/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/mm/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/darwin_amd64'
GOVCS=''
GOVERSION='go1.23.0'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/mm/Library/Application Support/go/telemetry'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='/usr/bin/clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/mm/go/src/github.com/MikeMitchellWebDev/gc_knobs/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 -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/5y/wtzzmjlj5v52pg7wr8ptbg_m0000gp/T/go-build534087183=/tmp/go-build -gno-record-gcc-switches -fno-common'
uname -v: Darwin Kernel Version 20.6.0: Thu Jul  6 22:12:47 PDT 2023; root:xnu-7195.141.49.702.12~1/RELEASE_X86_64
ProductName:    macOS
ProductVersion: 11.7.10
BuildVersion:   20G1427
lldb --version: lldb-1300.0.42.3
Swift version 5.5.2-dev

What did you do?

2 things: First I ran a program withGODEBUG=gccheckmark=1. When setCheckmarkwasn't called, I tried to build the go source code after manually setting var useCheckmark = true

What did you expect to see?

It depends on whether I had simply run a program with GODEBUG=gccheckmark=1 or altered the source code as described above.

setCheckmark to be called at some point
The source code to build after I set useCheckmark = true

What did you see instead?

With the source code "as is", setCheckmark is never called. There is a very narrow window in which useCheckmark is set to true inside gcMarkTermination, so this line of code in mgcmark.go never runs
```
if useCheckmark {
    if setCheckmark(obj, base, off, mbits) {
```
When I manually set useCheckmark = true, the source code wouldn't build. There's always a fatal error "fatal error: checkmark found unmarked object"

gabyhelp commented 2 months ago

Related Issues and Documentation

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

prattmic commented 2 months ago

cc @golang/runtime @aclements @mknyszek

prattmic commented 2 months ago

When I manually set useCheckmark = true, the source code wouldn't build. There's always a fatal error "fatal error: checkmark found unmarked object"

I don't think this part is surprising. Note that useCheckmark is typically only enabled temporarily during each GC: https://cs.opensource.google/go/go/+/master:src/runtime/mgc.go;l=999-1005;drc=96d8ff00c2d6a88384863a656fb5e53716b614d3;bpv=1;bpt=1

mknyszek commented 2 months ago

+1 to what @prattmic said, setting useCheckmark directly is not going to have the intended effect. That is a bit of internal state to the checkmark mode.

As for:

When setCheckmark wasn't called

What did you use to check to see if setCheckmark was called?

There is a very narrow window in which useCheckmark is set to true

I think you misunderstand what that code is doing. That is not a narrow window, it's effectively running an entire second single-threaded GC cycle while the world is stopped.

mknyszek commented 2 months ago

Thanks for reporting, there does seem to be an issue. In particular, what I'm finding is that gcResetMarkState doesn't reset the root marking job counter, so no roots get scanned, and therefore setCheckmark is never called. I think this is easily fixed; I'll give it a shot.

Also, this code really should have at least a trivial test...

gopherbot commented 2 months ago

Change https://go.dev/cl/608915 mentions this issue: runtime: fix GODEBUG=gccheckmark=1 and add smoke test

golang / go