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/go: go test behaviour different when debugging #65325

Closed
soypat opened this issue Jan 27, 2024 · 3 comments
Closed

cmd/go: go test behaviour different when debugging #65325

soypat opened this issue Jan 27, 2024 · 3 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Debugging WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.

Comments

@soypat
Copy link

soypat commented Jan 27, 2024

Go version

go version go1.21.4 linux/amd64

Delve Debugger
Version: 1.21.0
Build: Id: fec0d226b2c2cce1567d5f59169660cf61dc1efe

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/pato/.cache/go-build'
GOENV='/home/pato/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/pato/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/pato/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/linux_amd64'
GOVCS=''
GOVERSION='go1.21.4'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/pato/src/tg/fatso/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 -ffile-prefix-map=/tmp/go-build2077624849=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Ran go test . in soypat/fatfs@06cae34

What did you see happen?

The test failed earlier than in non-debug mode with following message:

--- FAIL: TestReadDir (2.89s)
    fatfs_test.go:20: 13
FAIL

This failure occurs between these lines: https://github.com/soypat/fatfs/blob/06cae34c8eee7fea47cd20c49817a4f0d8714d7f/fatfs.go#L2425-L2428

The I/O call (happens in RAM) reads into a buffer, all using unsafe. The data seems to be copied correctly without debug mode, and incorrectly when in debug mode.

What did you expect to see?

I expected to see same output than when not debugging (below).

This seems like it may be related to #64854.

--- FAIL: TestReadDir (0.00s)
    fatfs_test.go:33: {fsize:73 fdate:22583 ftime:48100 fattrib:32 altname:[68 73 82 70 73 76 69 0 0 0 0 0 0] fname:[100 105 114 102 105 108 101 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0]}
FAIL
exit status 1
FAIL    github.com/soypat/fatfs 0.001s
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jan 27, 2024
@soypat soypat changed the title cmd/compile: go test behaviour different when debugging cmd/go: go test behaviour different when debugging Jan 27, 2024
@soypat
Copy link
Author

soypat commented Jan 27, 2024

So it seems that I managed to work-around/fix the issue by applying this diff. I was wrong, it still is happening, though at times it seemed like it worked.

The diff consists of setting up the runtime more similarly to how libc does it. Note this issue may very well be me making wrong assumptions about how ccgo works. @cznic may know more.

@seankhliao seankhliao added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. Debugging labels Jan 27, 2024
@seankhliao
Copy link
Member

note that your code fails checkptr:

$ go test -race .
fatal error: checkptr: pointer arithmetic result points to invalid allocation

@soypat
Copy link
Author

soypat commented Jan 27, 2024

I've been misusing the API as cznic points out in slack.

Every uinptr-shaped C pointer you pass to the transpilled code must be either pinned (slow in my benchmarks: https://modern-c.blogspot.com/2023/07/ccgov4-experiment-trying-new.html) or it must be allocated in the C memory via libc.Xmalloc or libc.Xrealloc.
But here https://github.com/soypat/fatfs/blob/b4eba67a429e2182ef3dcc87e28cbe976d2c2cf0/ext.go#L10 the address of a local variable fss from here https://github.com/soypat/fatfs/blob/b4eba67a429e2182ef3dcc87e28cbe976d2c2cf0/fatfs_test.go#L30 is passed, for example. You can get away with package-level variables, you cannot do that with local ones as they may end up in the stack and Go can move stacks. The conversion to uintptr a) breaks the addr-pointee invariant b) allows the Go runtime to possibly free the memory while the transpiled code still uses it.
I know the ccgo ABI is not yet documented anywhere and that's my fault, sorry.
This is only about "static" analysis of the bug, I haven't looked for any logic errors.

And upon setting up a fixture to test if runtime.Pinner has any effect I found enabling pinning on pointer arguments actually fixed the bug.

As for the invalid allocation result, I'm not sure what's the root cause. I looked at the stack trace and the stuff that's going on in ccgo is beyond my expertise. I think we can close this as solved if that's alright with y'all.

@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Jan 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Debugging WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

4 participants
@gopherbot @seankhliao @soypat and others