reflect: MakeMap() and not native map type wrong behavior

[maxatome]

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

$ go version
go version go1.14 linux/amd64

Does this issue reproduce with the latest release?

Yes.

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

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/msoule/.cache/go-build"
GOENV="/home/msoule/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/msoule/Projet/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/msoule/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/msoule/local/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-build869392983=/tmp/go-build -gno-record-gcc-switches"

What did you do?

package main

import (
	"fmt"
	"reflect"
)

type Final map[int64]string

func main() {
	var out Final
	outVal := reflect.ValueOf(&out).Elem()

	valType := outVal.Type()
	valKeyType := valType.Key()
	valElemType := valType.Elem()
	mapType := reflect.MapOf(valKeyType, valElemType)

	valMap := reflect.MakeMap(mapType)

	currentKey := reflect.Indirect(reflect.New(reflect.TypeOf(int64(0))))
	currentKey.Set(reflect.ValueOf(int64(0)))
	currentVal := reflect.Indirect(reflect.New(reflect.TypeOf("")))
	currentVal.Set(reflect.ValueOf("pipo"))

	valMap.SetMapIndex(currentKey, currentVal)

	outVal.Set(valMap)
	fmt.Printf("> %s\n", valMap.MapIndex(reflect.ValueOf(int64(0))).Kind())
	fmt.Printf("> %s\n", outVal.MapIndex(reflect.ValueOf(int64(0))).Kind())
}

What did you expect to see?

> string
> string

What did you see instead?

> string
> invalid

Hints

• It works on the same host with go1.13.7;
• same wrong behavior on FreeBSD/amd64;
• but works on https://play.golang.org/;
• if we replace var out Final by var out map[int64]string it works;
• var out = map[int64]string{} works, but var out = Final{} still fails;
• type Final map[string]string (string key instead of int64 key), and replacing all int64(0) occurrences by "0", works.

[maxatome]

I forgot adding a final:

fmt.Printf("> %s\n", out[0])

prints:

pipo

:)

[randall77]

Strange. I can reproduce with 1.14 on darwin/amd64.
I cannot reproduce on tip, though. I guess a bisect might illuminate something.
I'll keep looking.

[randall77]

Looks like a hash function mismatch.
It was accidentally fixed at tip by https://go-review.googlesource.com/c/go/+/219338 . But not really fixed, just fixed for int32 and int64 keys.
The named and unnamed maps have different hash functions. When assigning one to the other, we just copy the map, the type (which contains the hash function) doesn't come along for the ride.
Still thinking about how to fix.

[randall77]

Here's a somewhat simpler reproducer, including at tip.

package main

import "reflect"

// complicated enough to require a compile-generated hash function
type K struct {
     a, b int32 // these get merged by the compiler into a single field, something typehash doesn't do
     c float64  // make the key not plain memory
}

func main() {
     k := K{a:1,b:2,c:3}

     // Make a reflect map.
     m := reflect.MakeMap(reflect.MapOf(reflect.TypeOf(K{}), reflect.TypeOf(true)))
     m.SetMapIndex(reflect.ValueOf(k), reflect.ValueOf(true))

     // The binary must not contain the type map[K]bool anywhere, or reflect.MapOf
     // will use that type instead of making a new one. So use an equivalent named type.
     type M map[K]bool
     var x M
     reflect.ValueOf(&x).Elem().Set(m)
     println(x[k])
}

This should print true, but prints false for 1.14 and tip.

[gopherbot]

Change https://golang.org/cl/222357 mentions this issue: runtime: make typehash match compiler generated hashes exactly

[randall77]

@gopherbot please open a backport issue for 1.14.

[gopherbot]

Backport issue(s) opened: #37721 (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.