karrick/godirwalk

godirwalk causes runtime panic with -checkptr on Go 1.14

Closed this issue · 2 comments

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

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.

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