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

panic diffing map with struct value type #10

Closed
bmizerany opened this issue May 17, 2022 · 2 comments · Fixed by #12
Closed

panic diffing map with struct value type #10

bmizerany opened this issue May 17, 2022 · 2 comments · Fixed by #12

Comments

@bmizerany
Copy link
Contributor

bmizerany commented May 17, 2022

Go Version

go version go1.18.2 darwin/arm64

Current kr.dev/diff commit 6c69dcc

I'm using diff.Test with a map[string]someStruct and seeing this panic:

diff % go test
--- FAIL: TestBug (0.00s)
panic: reflect.Value.UnsafeAddr of unaddressable value [recovered]
	panic: reflect.Value.UnsafeAddr of unaddressable value

goroutine 321 [running]:
testing.tRunner.func1.2({0x10119ed80, 0x1011ea1d0})
	/usr/local/go/src/testing/testing.go:1389 +0x1c8
testing.tRunner.func1()
	/usr/local/go/src/testing/testing.go:1392 +0x384
panic({0x10119ed80, 0x1011ea1d0})
	/usr/local/go/src/runtime/panic.go:838 +0x204
reflect.Value.UnsafeAddr(...)
	/usr/local/go/src/reflect/value.go:2528
kr.dev/diff.access({0x10119e380?, 0x1400029fbc0?, 0x1400029fbc0?})
	/Users/bmizerany/src/diff/diff.go:494 +0x90
kr.dev/diff.(*differ).walk(0x1400029d580, {0x1011ecd20, 0x140002e24e0}, {0x1011b7f60?, 0x1400029fbc0?, 0x1400029fbc8?}, {0x1011b7f60?, 0x1400029fbc8?, 0x14000050b68?}, 0x1, ...)
	/Users/bmizerany/src/diff/diff.go:330 +0x239c
kr.dev/diff.(*differ).walk(0x1400029d580, {0x1011ecd20, 0x140002e2340}, {0x1011adc40?, 0x14000010bd8?, 0x1011ba660?}, {0x1011adc40?, 0x14000010be0?, 0x14000043e78?}, 0x1, ...)
	/Users/bmizerany/src/diff/diff.go:360 +0x2110
kr.dev/diff.(*differ).each(0x1400029d580, {0x1011adc40, 0x140002e40c0}, {0x1011adc40?, 0x140002e40f0})
	/Users/bmizerany/src/diff/diff.go:242 +0x27c
kr.dev/diff.Test({0x1011eb408, 0x140002dc680}, 0x101131ee0?, {0x1011adc40, 0x140002e40c0}, {0x1011adc40, 0x140002e40f0}, {0x0, 0x0, 0x0})
	/Users/bmizerany/src/diff/diff.go:75 +0x138
kr.dev/diff_test.TestBug(0x140002dc680)
	/Users/bmizerany/src/diff/diff_test.go:388 +0xe4
testing.tRunner(0x140002dc680, 0x1011e94d0)
	/usr/local/go/src/testing/testing.go:1439 +0x110
created by testing.(*T).Run
	/usr/local/go/src/testing/testing.go:1486 +0x300
exit status 2
FAIL	kr.dev/diff	0.160s

A minimal test to reproduce:

func TestBug(t *testing.T) {
	type T struct {
		F int
	}

	got := map[string]T{
		"k": {},
	}

	want := map[string]T{
		"k": {},
	}

	diff.Test(t, t.Errorf, got, want)
}
@bmizerany
Copy link
Contributor Author

This change to diff_test.go produces the same result:

NOTE: The ab(...) version passes, but the non-copy panics.

--- a/diff_test.go
+++ b/diff_test.go
@@ -62,6 +62,11 @@ func TestEqual(t *testing.T) {
                ab(make(chan int)),
                ab(unsafe.Pointer(new(int))),
                ab(unsafe.Pointer(nil)),
+               ab(map[int]struct{ F int }{0: {}}),
+               {
+                       map[int]struct{ F int }{0: {}},
+                       map[int]struct{ F int }{0: {}},
+               },
        }

@bmizerany
Copy link
Contributor Author

bmizerany commented May 17, 2022

Changing the value to a pointer causes the test to pass:

{
    map[int]*struct{ F int }{0: {}},
    map[int]*struct{ F int }{0: {}},
},

bmizerany added a commit to bmizerany/diff that referenced this issue May 18, 2022
bmizerany added a commit to bmizerany/diff that referenced this issue May 18, 2022
@kr kr closed this as completed in #12 May 18, 2022
kr pushed a commit that referenced this issue May 18, 2022
kr pushed a commit that referenced this issue Oct 9, 2022
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