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

godirwalk causes runtime panic with -checkptr on Go 1.14 #51

Closed
quasilyte opened this issue Apr 8, 2020 · 2 comments · Fixed by #52
Closed

godirwalk causes runtime panic with -checkptr on Go 1.14 #51

quasilyte opened this issue Apr 8, 2020 · 2 comments · Fixed by #52

Comments

@quasilyte
Copy link
Contributor

quasilyte commented Apr 8, 2020

Steps to reproduce:

# go should be at least 1.14
go test -race -bench . .

Result:

fatal error: checkptr: unsafe pointer conversion

goroutine 25 [running]:
runtime.throw(0x6233ac, 0x23)
	$GOROOT/src/runtime/panic.go:1112 +0x72 fp=0xc000121220 sp=0xc0001211f0 pc=0x45fd42
runtime.checkptrAlignment(0xc0006d2f00, 0x607380, 0x1)
	$GOROOT/src/runtime/checkptr.go:18 +0xb7 fp=0xc000121250 sp=0xc000121220 pc=0x433507
github.com/karrick/godirwalk.readDirents(0xc0006eeb90, 0x50, 0xc0006d2000, 0x1000, 0x1000, 0x1620, 0x162, 0x200, 0xc0006f1620, 0xc0006f0000)
	$GOPATH/src/github.com/karrick/godirwalk/readdir_unix.go:54 +0x18d fp=0xc0001213e0 sp=0xc000121250 pc=0x5ba05d
github.com/karrick/godirwalk.ReadDirents(...)
	$GOPATH/src/github.com/karrick/godirwalk/readdir.go:23
github.com/karrick/godirwalk.newSortedScanner(0xc0006eeb90, 0x50, 0xc0006d2000, 0x1000, 0x1000, 0x4f, 0x1, 0x49)
	$GOPATH/src/github.com/karrick/godirwalk/scanner.go:21 +0x86 fp=0xc000121470 sp=0xc0001213e0 pc=0x5bc6e6
github.com/karrick/godirwalk.walk(0xc0006eeb90, 0x50, 0xc0006f6840, 0xc000121d88, 0x50, 0x0)
	$GOPATH/src/github.com/karrick/godirwalk/walk.go:261 +0xb5d fp=0xc0001215c0 sp=0xc000121470 pc=0x5bde7d
github.com/karrick/godirwalk.walk(0xc0006ee820, 0x48, 0xc0006f65d0, 0xc000121d88, 0x48, 0xc000128060)
	$GOPATH/src/github.com/karrick/godirwalk/walk.go:279 +0x51c fp=0xc000121710 sp=0xc0001215c0 pc=0x5bd83c
github.com/karrick/godirwalk.walk(0xc0006ee780, 0x43, 0xc0006f65a0, 0xc000121d88, 0x43, 0xc00000ffc0)
	$GOPATH/src/github.com/karrick/godirwalk/walk.go:279 +0x51c fp=0xc000121860 sp=0xc000121710 pc=0x5bd83c
github.com/karrick/godirwalk.walk(0xc00001e540, 0x3b, 0xc0006f64b0, 0xc000121d88, 0x3b, 0xc00000ff80)
	$GOPATH/src/github.com/karrick/godirwalk/walk.go:279 +0x51c fp=0xc0001219b0 sp=0xc000121860 pc=0x5bd83c
github.com/karrick/godirwalk.walk(0xc00001e4c0, 0x31, 0xc000114330, 0xc000121d88, 0x31, 0x0)
	$GOPATH/src/github.com/karrick/godirwalk/walk.go:279 +0x51c fp=0xc000121b00 sp=0xc0001219b0 pc=0x5bd83c
github.com/karrick/godirwalk.walk(0xc00012e060, 0x26, 0xc0001142d0, 0xc000121d88, 0x0, 0x0)
	$GOPATH/src/github.com/karrick/godirwalk/walk.go:279 +0x51c fp=0xc000121c50 sp=0xc000121b00 pc=0x5bd83c
github.com/karrick/godirwalk.Walk(0xc00012e060, 0x26, 0xc000053d88, 0x76235c, 0x1)
	$GOPATH/src/github.com/karrick/godirwalk/walk.go:204 +0x36b fp=0xc000121d20 sp=0xc000121c50 pc=0x5bcf6b
github.com/karrick/godirwalk.godirwalkWalk(0x655620, 0xc0001f8380, 0xc00012e060, 0x26, 0x54487e, 0xc0001f84c9, 0x18)
	$GOPATH/src/github.com/karrick/godirwalk/walk_test.go:30 +0x15d fp=0xc000121dd8 sp=0xc000121d20 pc=0x5c2d0d
github.com/karrick/godirwalk.BenchmarkGodirwalk(0xc0001f8380)
	$GOPATH/src/github.com/karrick/godirwalk/walk_test.go:295 +0x102 fp=0xc000121e48 sp=0xc000121dd8 pc=0x5c4be2
testing.(*B).runN(0xc0001f8380, 0x1)
	$GOROOT/src/testing/benchmark.go:191 +0x1b5 fp=0xc000121f68 sp=0xc000121e48 pc=0x5451b5
testing.(*B).run1.func1(0xc0001f8380)
	$GOROOT/src/testing/benchmark.go:231 +0x76 fp=0xc000121fd8 sp=0xc000121f68 pc=0x555476
runtime.goexit()
	$GOROOT/src/runtime/asm_amd64.s:1373 +0x1 fp=0xc000121fe0 sp=0xc000121fd8 pc=0x491511
created by testing.(*B).run1
	$GOROOT/src/testing/benchmark.go:224 +0x8f

Since Go 1.14 -d=checkptr is enabled with -race.

With async preemption that is a recent Go addition, rules for unsafe became more strict.

1.14 release notes mentions this:

When converting unsafe.Pointer to *T, the resulting pointer must be aligned appropriately for T.

I believe this is a root of panic here.

godirwalk/scandir_unix.go

Lines 147 to 148 in 28c3d94

s.sde = (*syscall.Dirent)(unsafe.Pointer(&s.workBuffer[0])) // point entry to first syscall.Dirent in buffer
s.workBuffer = s.workBuffer[reclen(s.sde):] // advance buffer for next iteration through loop

See golang/go#34964

If we don't fix this, users can't run their tests/apps in -race mode if they're using godirwalk.

Benchmarks fail for other reasons even without -race:

Benchmark2ReadDirentsGodirwalk
    Benchmark2ReadDirentsGodirwalk: benchmark_test.go:24: open /mnt/ram_disk/src/linkedin/dashboards: no such file or directory
--- FAIL: Benchmark2ReadDirentsGodirwalk
Benchmark2ReadDirnamesGodirwalk
    Benchmark2ReadDirnamesGodirwalk: benchmark_test.go:38: open /mnt/ram_disk/src/linkedin/dashboards: no such file or directory
--- FAIL: Benchmark2ReadDirnamesGodirwalk
Benchmark2GodirwalkSorted
    Benchmark2GodirwalkSorted: benchmark_test.go:60: GOT: lstat /mnt/ram_disk/src: no such file or directory; WANT: nil
--- FAIL: Benchmark2GodirwalkSorted
Benchmark2GodirwalkUnsorted
    Benchmark2GodirwalkUnsorted: benchmark_test.go:81: GOT: lstat /mnt/ram_disk/src: no such file or directory; WANT: nil
@quasilyte
Copy link
Contributor Author

I don't have a good idea of how to fix this particular case.
If something will come to my mind, I'll probably send a PR.

@quasilyte
Copy link
Contributor Author

Hmm. Even though this is not Darwin-related problem, this patch fixes it:
https://go-review.googlesource.com/c/tools/+/221381/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant