reflect: DeepEqual can return true for values that are not equal #39607

ailurarctos · 2020-06-16T09:19:16Z

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

$ go version
go version go1.14.4 linux/amd64

Does this issue reproduce with the latest release?

Yes, go1.14.4. It does not happen with go1.13.12.

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

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/me/.cache/go-build"
GOENV="/home/me/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/me/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build795938586=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I ran https://play.golang.org/p/h2Wt6eYJB0B:

package main

import (
	"fmt"
	"reflect"
)

type testingStruct struct {
	f1 *testingStruct
	f2 string
}

func main() {
	s1 := &testingStruct{
		f1: &testingStruct{
			f2: "foo",
		},
	}
	s2 := &testingStruct{
		f1: &testingStruct{
			f2: "foobar",
		},
	}
	fmt.Println(reflect.DeepEqual(s1, s2))
}

What did you expect to see?

I expected it to print "false".

What did you see instead?

It printed "true".

The text was updated successfully, but these errors were encountered:

deadok22 · 2020-06-16T09:48:04Z

deepValueEqual uses reflect.Value.ptr directly, which results in the same value of visit struct for (s1, s2) and (s1.f1, s2.f1). It seems deepValueEqual should call reflect.Value.Pointer instead.

Have a look at the addresses printed by this program

mvdan · 2020-06-16T10:15:41Z

This is working as intended. Please read the docs:

Pointer values are deeply equal if they are equal using Go's == operator or if they point to deeply equal values.

If you still think there's a bug, please explain why.

networkimprov · 2020-06-16T10:50:16Z

This was "false" in 1.13, which is what I'd expect from the docs you quoted. It's "true" now.

mvdan · 2020-06-16T10:52:01Z

Apologies, I read the code too fast and didn't notice the difference in the strings "foo" and "foobar".

zigo101 · 2020-06-16T14:05:08Z

It looks it doesn't matter what is the type of f2.

davecheney · 2020-06-17T01:22:50Z

Paging @randall77 and @aclements in case this is code gen or linker data structure related.

ianlancetaylor · 2020-06-17T04:38:20Z

It's a bug in reflect.DeepEqual. It doesn't correctly handle comparing two struct values when the first elements of the structs are pointers.

ianlancetaylor · 2020-06-17T04:40:16Z

I think the bug was introduced by https://golang.org/cl/191940 for #33907. It's broken in 1.14 and tip but works in 1.13 and earlier.

CC @huandu

@gopherbot Please open backport to 1.14.

gopherbot · 2020-06-17T04:40:26Z

Backport issue(s) opened: #39635 (for 1.13), #39636 (for 1.14).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

gopherbot · 2020-06-17T04:59:40Z

Change https://golang.org/cl/238361 mentions this issue: reflect: handling flagIndir in DeepEqual potential cycles

huandu · 2020-06-17T05:55:17Z

I think the bug was introduced by https://golang.org/cl/191940 for #33907. It's broken in 1.14 and tip but works in 1.13 and earlier.

Yes. This bug is introduced by that change.

As #33907 is still a valid issue and only cyclic interface value can cause stack overflow, I suggest to get ptr value by UnsafeAddr and get interface value by v1.ptr.

gopherbot · 2020-06-18T01:15:35Z

Change https://golang.org/cl/238626 mentions this issue: [release-branch.go1.14] reflect: handling flagIndir in DeepEqual potential cycles

…ntial cycles For #39607 Fixes #39636 Change-Id: Ia7e597e0da8a193a25382cc633a1c6080b4f7cbf Reviewed-on: https://go-review.googlesource.com/c/go/+/238361 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> (cherry picked from commit d872bbc) Reviewed-on: https://go-review.googlesource.com/c/go/+/238626

davecheney · 2020-07-16T23:41:21Z

@ianlancetaylor @dmitshur did this change make it into the 1.14.5 release? If not, will there be a 1.14.6 and when might we see it?

dmitshur · 2020-07-16T23:43:32Z

@davecheney 1.14.5 was a security release. 1.14.6 is being released right now (see #40256) and it does include the fix. See backport issue #39636 for more details.

davecheney · 2020-07-16T23:46:23Z

@dmitshur superb, thanks for your quick response, we have a few folks here at $DAYJOB who are waiting on this fix.

ailurarctos changed the title ~~reflect: DeepEqual can return true for values are not equal~~ reflect: DeepEqual can return true for values that are not equal Jun 16, 2020

mvdan closed this as completed Jun 16, 2020

mvdan reopened this Jun 16, 2020

This was referenced Jun 17, 2020

reflect: DeepEqual can return true for values that are not equal [1.13 backport] #39635

Closed

reflect: DeepEqual can return true for values that are not equal [1.14 backport] #39636

Closed

andybons added the NeedsFix label Jun 17, 2020

andybons added this to the Go1.15 milestone Jun 17, 2020

gopherbot closed this as completed in d872bbc Jun 18, 2020

golang locked and limited conversation to collaborators Jul 16, 2021

gopherbot added the FrozenDueToAge label Jul 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

reflect: DeepEqual can return true for values that are not equal #39607

reflect: DeepEqual can return true for values that are not equal #39607

ailurarctos commented Jun 16, 2020 •

edited

Loading

deadok22 commented Jun 16, 2020 •

edited

Loading

mvdan commented Jun 16, 2020

networkimprov commented Jun 16, 2020

mvdan commented Jun 16, 2020

zigo101 commented Jun 16, 2020

davecheney commented Jun 17, 2020

ianlancetaylor commented Jun 17, 2020

ianlancetaylor commented Jun 17, 2020

gopherbot commented Jun 17, 2020

gopherbot commented Jun 17, 2020

huandu commented Jun 17, 2020

gopherbot commented Jun 18, 2020

davecheney commented Jul 16, 2020

dmitshur commented Jul 16, 2020

davecheney commented Jul 16, 2020

reflect: DeepEqual can return true for values that are not equal #39607

reflect: DeepEqual can return true for values that are not equal #39607

Comments

ailurarctos commented Jun 16, 2020 • edited Loading

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

Does this issue reproduce with the latest release?

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

What did you do?

What did you expect to see?

What did you see instead?

deadok22 commented Jun 16, 2020 • edited Loading

mvdan commented Jun 16, 2020

networkimprov commented Jun 16, 2020

mvdan commented Jun 16, 2020

zigo101 commented Jun 16, 2020

davecheney commented Jun 17, 2020

ianlancetaylor commented Jun 17, 2020

ianlancetaylor commented Jun 17, 2020

gopherbot commented Jun 17, 2020

gopherbot commented Jun 17, 2020

huandu commented Jun 17, 2020

gopherbot commented Jun 18, 2020

davecheney commented Jul 16, 2020

dmitshur commented Jul 16, 2020

davecheney commented Jul 16, 2020

ailurarctos commented Jun 16, 2020 •

edited

Loading

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

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

deadok22 commented Jun 16, 2020 •

edited

Loading