cmd/compile,runtime: go tip breaks github.com/ethereum/go-ethereum/rlp

[cuonglm]

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

$ go version
go version devel +b388d6868f Thu Jun 13 03:58:18 2019 +0000 linux/amd64

Does this issue reproduce with the latest release?

No.

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

go env Output

$ go env
GO111MODULE="off"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/cuonglm/.cache/go-build"
GOENV="/home/cuonglm/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/cuonglm/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/cuonglm/sources/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/cuonglm/sources/go/pkg/tool/linux_amd64"
GCCGO="/usr/local/bin/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-build174470444=/tmp/go-build -gno-record-gcc-switches"

What did you do?

$ cd $GOPATH/src/github.com/ethereum/go-ethereum/rlp
$ go test -count=1

What did you expect to see?

Test passed.

What did you see instead?

--- FAIL: TestDecodeWithByteReader (0.00s)
    decode_test.go:582: test 40: value mismatch
        got  [0]uint8{}
        want [0]uint8{}
        decoding into *[0]uint8
        input "80"
--- FAIL: TestDecodeWithNonByteReader (0.00s)
    decode_test.go:582: test 40: value mismatch
        got  [0]uint8{}
        want [0]uint8{}
        decoding into *[0]uint8
        input "80"
--- FAIL: TestDecodeStreamReset (0.00s)
    decode_test.go:582: test 40: value mismatch
        got  [0]uint8{}
        want [0]uint8{}
        decoding into *[0]uint8
        input "80"
FAIL
exit status 1
FAIL	github.com/ethereum/go-ethereum/rlp	0.005s

The test still works with go 1.12.6 and earlier.

I can't produce with minimal example, but cherry pick point to fff4f59

cc @randall77

[agnivade]

Found a reproducer. But very oddly, it fails only inside a test. Not in a main function.

func TestMy(t *testing.T) {
	deref := reflect.ValueOf(new([0]byte)).Elem().Interface()
	t.Log(reflect.DeepEqual(deref, [0]byte{}))
}

This gives false at tip, but true with 1.12.

But running this inside a func main() always gives true with both.

[martisch]

As I see a new([0]byte) there and an interface. Is this a case of pointers to zero sized types may or may not be equal? https://golang.org/ref/spec#Comparison_operators

One thing I remember that changed in relation to zero sized allocations in go1.13 was: https://go-review.googlesource.com/c/go/+/155840

[cuonglm]

@martisch the weird thing is that it's only occurs inside a test.

[randall77]

This is strange. It is a real bug, though.

With the stack-allocated defer CL, there are at runtime two reflect.rtype instances representing [0]uint8. The test makes two instances of a [0]uint8 in an interface, each with a type field pointing to a different reflect.rtype. Thus they don't compare equal, even though they should. We should never make two reflect.rtype instances representing the same type.

The second [0]uint8 type I think comes from when we build a defer struct for a stack-allocated defer. For a defer with no args (which I think is the one at github.com/ethereum/go-ethereum/rlp/typecache.go:80), one of the fields of that struct is a zero-sized array.

Somehow a real [0]uint8 type in the program is getting replaced with the [0]uint8 type that was used to build the defer struct. That shouldn't happen. We construct non-user-visible types in the compiler in lots of places (0-sized and otherwise), and I've never seen this before.

Next up, try to figure out why we're promoting one of these internal-only types to a real type. Particularly, why this happens with defer structs and not with other similar types (map buckets, map iteration structs, temp bufs for string ops).

[cherrymui]

I guess we need to mark the compiler-created type NoAlg? (as map buckets do) So it won't get pulled into typelink.

[randall77]

Seems like a Noalg issue, there's one [0]uint8 with an algorithm table and one without.
I can fix the bug by not setting Noalg on the [0]uint8 created for the defer struct.
Seems related to #17752.
From that CL, it looks like the fix for #17752 doesn't quite work here.

I think what is going on is that we have a regular [0]byte type and a noalg [0]byte type. But there is also a *[0]byte type, and it only has one Elem field. In this case, the pointer in the *[0]byte Elem field points to the noalg [0]byte, not the regular one. Thus it doesn't compare equal with the regular one.

Not marking the byte array in the defer struct as noalg is probably the simplest fix, and I will do that for 1.13. (It isn't a big deal, as the alg pointer for [N]byte things will be shared.) We may need a deeper fix, though. One thing that does work is to change types.NewPtr to make a noalg pointer type when the base is noalg.

[gopherbot]

Change https://golang.org/cl/182357 mentions this issue: cmd/compile: don't mark argument array as noptr