Skip to content

cmd/go: miscompilation of codependent global var assigments in go1.12 #30977

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.

Sign up for GitHub

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

Jump to bottom
Closed
ijc opened this issue Mar 21, 2019 · 25 comments
Closed

cmd/go: miscompilation of codependent global var assigments in go1.12 #30977

ijc opened this issue Mar 21, 2019 · 25 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.13

Comments

@ijc
Copy link

ijc commented Mar 21, 2019

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

$ PATH=$HOME/tmp/go/bin:$PATH go version
go version go1.12.1 linux/amd64

Does this issue reproduce with the latest release?

It reproduces with go 1.12.1 so I believe yes. I downloaded it from https://dl.google.com/go/go1.12.1.linux-amd64.tar.gz

$ sha256sum ~/tmp/go1.12.1.linux-amd64.tar.gz
2a3fdabf665496a0db5f41ec6af7a9b15a49fbe71a85a50ca38b1f13a103aeec  /home/ijc/tmp/go1.12.1.linux-amd64.tar.gz

I've also reproduced on darwin/amd64 from https://golang.org/doc/install?download=go1.12.1.darwin-amd64.pkg:

$ sha256sum ~/Downloads/go1.12.1.darwin-amd64.pkg 
8da4d8a7c5c4fb5d144b5c18e48ca5f0a35677dc71517822ab41bdc7766d3175  /home/ijc/Downloads/go1.12.1.darwin-amd64.pkg

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

linux/amd64 (Debian).

go env Output on linux/amd64
$ PATH=$HOME/tmp/go/bin:$PATH go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/ijc/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/ijc/go"
GOPROXY=""
GORACE=""
GOROOT="/home/ijc/tmp/go"
GOTMPDIR=""
GOTOOLDIR="/home/ijc/tmp/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/ijc/development/Docker/go1.12-initvar-miscompile/go.mod"
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-build327430398=/tmp/go-build -gno-record-gcc-switches"

I have also reproduced on darwin/amd64:

go env Output on darwin/amd64
$ /usr/local/go/bin/go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/ijc/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/ijc/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/ijc/go1.12-initvar-miscompile/go.mod"
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/4k/lv46d_kx3sn71k23yltwyhf40000gn/T/go-build732469803=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I have a program which includes:

import "github.com/blang/semver"

var (
	Version0_2_0  = semver.MustParse("0.2.0")
	VersionLatest = Version0_2_0
)

The types are semver.Version which is a struct with a handful of fields (Major, Minor etc).

I am seeing corruption of VersionLatest where the major field is 824638865200 (always that number, AFAICS) after init and not 0. Version0_2_0 is never corrupted.

I have narrowed it down to this minimal case: go1.12-initvar-miscompile.zip

$ PATH=$HOME/tmp/go/bin:$PATH go build ./
$ ./miscompile  # this is a pass
"0.2.0"
$ ./miscompile # this is a fail
panic: VersionLatest corrupt: "0.2.0" "824638766896.2.0"

goroutine 1 [running]:
main.main()
	/home/ijc/development/Docker/go1.12-initvar-miscompile/main.go:186 +0x1b5

Note that the test program includes a massive bunch of anonymous imports, these seem to be necessary to trigger the runtime/gc load and cause the issue (they are derived from all the vendoring in my actual project which have init() functions). Trying to minimise the set of imports is non-linear (since the issue is non-deterministic AFAICT) and also reduces the probability of the issue occurring. I got it down to a dozen or so imports and a 1/10,000 incidence, since it is much easier to reproduce with the larger import set that is what I included here.

What did you expect to see?

On success VersionLatest is "0.2.0".

What did you see instead?

It is corrupted to "824638766896.2.0".

Analysis

After the call to semver.MustParse there is a runtime.writeBarrier slow path:

  main.go:177		0x1799233		e8182f30ff		CALL github.com/blang/semver.MustParse(SB)	
  main.go:177		0x1799238		833d31cc050100		CMPL $0x0, runtime.writeBarrier(SB)		
  main.go:177		0x179923f		0f8588000000		JNE 0x17992cd

The issue occurs precisely when this jump is taken and is during this sequence:

// Result of semver.MustParse is on the stack at 0x10(SP). First copy it
// to `Version0_2_0`, effectively `memcpy(Version0_2_0, «stack temporary»)`

  main.go:177		0x17992cd		488b442410		MOVQ 0x10(SP), AX				
  main.go:177		0x17992d2		48898424a0000000	MOVQ AX, 0xa0(SP)				
  main.go:177		0x17992da		0f10442418		MOVUPS 0x18(SP), X0				
  main.go:177		0x17992df		0f118424a8000000	MOVUPS X0, 0xa8(SP)				
  main.go:177		0x17992e7		0f10442428		MOVUPS 0x28(SP), X0				
  main.go:177		0x17992ec		0f118424b8000000	MOVUPS X0, 0xb8(SP)				
  main.go:177		0x17992f4		0f10442438		MOVUPS 0x38(SP), X0				
  main.go:177		0x17992f9		0f118424c8000000	MOVUPS X0, 0xc8(SP)				
  main.go:177		0x1799301		0f10442448		MOVUPS 0x48(SP), X0				
  main.go:177		0x1799306		0f118424d8000000	MOVUPS X0, 0xd8(SP)				

// This sequence overwrites 0x10(SP) with some value before calling
// `runtime.typedmemmove`

  main.go:177		0x179930e		488d058bd22800		LEAQ type.JyaB60kV(SB), AX			
  main.go:177		0x1799315		48890424		MOVQ AX, 0(SP)					
  main.go:177		0x1799319		488d0d00f00301		LEAQ main.Version0_2_0(SB), CX			
  main.go:177		0x1799320		48894c2408		MOVQ CX, 0x8(SP)				
  main.go:177		0x1799325		488d8c24a0000000	LEAQ 0xa0(SP), CX				
  main.go:177		0x179932d		48894c2410		MOVQ CX, 0x10(SP)				
  main.go:177		0x1799332		e8b9e520ff		CALL runtime.typedmemmove(SB)			

// Now we try to copy the result of `semver.MustParse` from the stack into
// `VersionLatest`, effectively `memcpy(VersionLatest, «stack temporary»)`,
// but the values at 0x10(SP) has been changed by the `CALL` above,
// thus `VersionLatest` is corrupted.

  main.go:178		0x1799337		488b442410		MOVQ 0x10(SP), AX				
  main.go:178		0x179933c		4889442458		MOVQ AX, 0x58(SP)				
  main.go:178		0x1799341		0f10442418		MOVUPS 0x18(SP), X0				
  main.go:178		0x1799346		0f11442460		MOVUPS X0, 0x60(SP)				
  main.go:178		0x179934b		0f10442428		MOVUPS 0x28(SP), X0				
  main.go:178		0x1799350		0f11442470		MOVUPS X0, 0x70(SP)				
  main.go:178		0x1799355		0f10442438		MOVUPS 0x38(SP), X0				
  main.go:178		0x179935a		0f11842480000000	MOVUPS X0, 0x80(SP)				
  main.go:178		0x1799362		0f10442448		MOVUPS 0x48(SP), X0				
  main.go:178		0x1799367		0f11842490000000	MOVUPS X0, 0x90(SP)

On go1.11.4 the same code is compiled differently and the second copy is memcpy(VersionLatest, Version0_2_0) instead which does not use the corrupted stack copy

  main.go:178		0x170888a		488b056fbcef00		MOVQ main.Version0_2_0(SB), AX								
  main.go:178		0x1708891		488905c8bcef00		MOVQ AX, main.VersionLatest(SB)								
  main.go:178		0x1708898		0f100569bcef00		MOVUPS main.Version0_2_0+8(SB), X0							
  main.go:178		0x170889f		0f1105c2bcef00		MOVUPS X0, main.VersionLatest+8(SB)							
  main.go:178		0x17088a6		0f10056bbcef00		MOVUPS main.Version0_2_0+24(SB), X0							
  main.go:178		0x17088ad		0f1105c4bcef00		MOVUPS X0, main.VersionLatest+24(SB)							
  main.go:178		0x17088b4		0f10056dbcef00		MOVUPS main.Version0_2_0+40(SB), X0							
  main.go:178		0x17088bb		0f1105c6bcef00		MOVUPS X0, main.VersionLatest+40(SB)							
  main.go:178		0x17088c2		0f10056fbcef00		MOVUPS main.Version0_2_0+56(SB), X0							
  main.go:178		0x17088c9		0f1105c8bcef00		MOVUPS X0, main.VersionLatest+56(SB)

BTW, the code pattern is the same in the fast pass, but that lacks the call to runtime.typedmemmove which overwrites the stack value, thus the issue only occurs if we manager to hit the slow path.

Full go tool objdump -s '^main.init.ializers' Output for go 1.12.1 compilation
```
$ go tool objdump -s '^main.init.ializers$' miscompile 
TEXT main.init.ializers(SB) «...»/go1.12-initvar-miscompile/main.go
  main.go:177		0x17991f0		64488b0c25f8ffffff	MOVQ FS:0xfffffff8, CX				
  main.go:177		0x17991f9		488d442490		LEAQ -0x70(SP), AX				
  main.go:177		0x17991fe		483b4110		CMPQ 0x10(CX), AX				
  main.go:177		0x1799202		0f8692010000		JBE 0x179939a					
  main.go:177		0x1799208		4881ecf0000000		SUBQ $0xf0, SP					
  main.go:177		0x179920f		4889ac24e8000000	MOVQ BP, 0xe8(SP)				
  main.go:177		0x1799217		488dac24e8000000	LEAQ 0xe8(SP), BP				
  main.go:177		0x179921f		488d05a63a2d00		LEAQ 0x2d3aa6(IP), AX				
  main.go:177		0x1799226		48890424		MOVQ AX, 0(SP)					
  main.go:177		0x179922a		48c744240805000000	MOVQ $0x5, 0x8(SP)				
  main.go:177		0x1799233		e8182f30ff		CALL github.com/blang/semver.MustParse(SB)	
  main.go:177		0x1799238		833d31cc050100		CMPL $0x0, runtime.writeBarrier(SB)		
  main.go:177		0x179923f		0f8588000000		JNE 0x17992cd					
  main.go:177		0x1799245		488b442410		MOVQ 0x10(SP), AX				
  main.go:177		0x179924a		488905cff00301		MOVQ AX, main.Version0_2_0(SB)			
  main.go:177		0x1799251		0f10442418		MOVUPS 0x18(SP), X0				
  main.go:177		0x1799256		0f1105cbf00301		MOVUPS X0, main.Version0_2_0+8(SB)		
  main.go:177		0x179925d		0f10442428		MOVUPS 0x28(SP), X0				
  main.go:177		0x1799262		0f1105cff00301		MOVUPS X0, main.Version0_2_0+24(SB)		
  main.go:177		0x1799269		0f10442438		MOVUPS 0x38(SP), X0				
  main.go:177		0x179926e		0f1105d3f00301		MOVUPS X0, main.Version0_2_0+40(SB)		
  main.go:177		0x1799275		0f10442448		MOVUPS 0x48(SP), X0				
  main.go:177		0x179927a		0f1105d7f00301		MOVUPS X0, main.Version0_2_0+56(SB)		
  main.go:178		0x1799281		488b442410		MOVQ 0x10(SP), AX				
  main.go:178		0x1799286		488905f3f00301		MOVQ AX, main.VersionLatest(SB)			
  main.go:178		0x179928d		0f10442418		MOVUPS 0x18(SP), X0				
  main.go:178		0x1799292		0f1105eff00301		MOVUPS X0, main.VersionLatest+8(SB)		
  main.go:178		0x1799299		0f10442428		MOVUPS 0x28(SP), X0				
  main.go:178		0x179929e		0f1105f3f00301		MOVUPS X0, main.VersionLatest+24(SB)		
  main.go:178		0x17992a5		0f10442438		MOVUPS 0x38(SP), X0				
  main.go:178		0x17992aa		0f1105f7f00301		MOVUPS X0, main.VersionLatest+40(SB)		
  main.go:178		0x17992b1		0f10442448		MOVUPS 0x48(SP), X0				
  main.go:178		0x17992b6		0f1105fbf00301		MOVUPS X0, main.VersionLatest+56(SB)		
  main.go:178		0x17992bd		488bac24e8000000	MOVQ 0xe8(SP), BP				
  main.go:178		0x17992c5		4881c4f0000000		ADDQ $0xf0, SP					
  main.go:178		0x17992cc		c3			RET						
  main.go:177		0x17992cd		488b442410		MOVQ 0x10(SP), AX				
  main.go:177		0x17992d2		48898424a0000000	MOVQ AX, 0xa0(SP)				
  main.go:177		0x17992da		0f10442418		MOVUPS 0x18(SP), X0				
  main.go:177		0x17992df		0f118424a8000000	MOVUPS X0, 0xa8(SP)				
  main.go:177		0x17992e7		0f10442428		MOVUPS 0x28(SP), X0				
  main.go:177		0x17992ec		0f118424b8000000	MOVUPS X0, 0xb8(SP)				
  main.go:177		0x17992f4		0f10442438		MOVUPS 0x38(SP), X0				
  main.go:177		0x17992f9		0f118424c8000000	MOVUPS X0, 0xc8(SP)				
  main.go:177		0x1799301		0f10442448		MOVUPS 0x48(SP), X0				
  main.go:177		0x1799306		0f118424d8000000	MOVUPS X0, 0xd8(SP)				
  main.go:177		0x179930e		488d058bd22800		LEAQ type.JyaB60kV(SB), AX			
  main.go:177		0x1799315		48890424		MOVQ AX, 0(SP)					
  main.go:177		0x1799319		488d0d00f00301		LEAQ main.Version0_2_0(SB), CX			
  main.go:177		0x1799320		48894c2408		MOVQ CX, 0x8(SP)				
  main.go:177		0x1799325		488d8c24a0000000	LEAQ 0xa0(SP), CX				
  main.go:177		0x179932d		48894c2410		MOVQ CX, 0x10(SP)				
  main.go:177		0x1799332		e8b9e520ff		CALL runtime.typedmemmove(SB)			
  main.go:178		0x1799337		488b442410		MOVQ 0x10(SP), AX				
  main.go:178		0x179933c		4889442458		MOVQ AX, 0x58(SP)				
  main.go:178		0x1799341		0f10442418		MOVUPS 0x18(SP), X0				
  main.go:178		0x1799346		0f11442460		MOVUPS X0, 0x60(SP)				
  main.go:178		0x179934b		0f10442428		MOVUPS 0x28(SP), X0				
  main.go:178		0x1799350		0f11442470		MOVUPS X0, 0x70(SP)				
  main.go:178		0x1799355		0f10442438		MOVUPS 0x38(SP), X0				
  main.go:178		0x179935a		0f11842480000000	MOVUPS X0, 0x80(SP)				
  main.go:178		0x1799362		0f10442448		MOVUPS 0x48(SP), X0				
  main.go:178		0x1799367		0f11842490000000	MOVUPS X0, 0x90(SP)				
  main.go:178		0x179936f		488d052ad22800		LEAQ type.JyaB60kV(SB), AX			
  main.go:178		0x1799376		48890424		MOVQ AX, 0(SP)					
  main.go:178		0x179937a		488d05ffef0301		LEAQ main.VersionLatest(SB), AX			
  main.go:178		0x1799381		4889442408		MOVQ AX, 0x8(SP)				
  main.go:178		0x1799386		488d442458		LEAQ 0x58(SP), AX				
  main.go:178		0x179938b		4889442410		MOVQ AX, 0x10(SP)				
  main.go:178		0x1799390		e85be520ff		CALL runtime.typedmemmove(SB)			
  main.go:177		0x1799395		e923ffffff		JMP 0x17992bd					
  main.go:177		0x179939a		e8611a25ff		CALL local.runtime.morestack_noctxt(SB)		
  main.go:177		0x179939f		e94cfeffff		JMP local.main.init.ializers(SB)	
```
@ijc
Copy link
Author

ijc commented Mar 21, 2019

The types are semver.Version which is a struct with a handful of fields (Major, Minor etc).

For completeness here is the actual full type which is slightly more complex than I let on above:

// Version represents a semver compatible version
type Version struct {
	Major uint64
	Minor uint64
	Patch uint64
	Pre   []PRVersion
	Build []string //No Precedence
}

@thaJeztah
Copy link
Contributor

ping @bradfitz PTAL

@ALTree ALTree added this to the Go1.12.2 milestone Mar 21, 2019
@ALTree ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 21, 2019
@ALTree ALTree changed the title miscompilation of codependent global var assigments in go1.12 cmd/go: miscompilation of codependent global var assigments in go1.12 Mar 21, 2019
@ALTree
Copy link
Member

ALTree commented Mar 21, 2019 •
edited
Loading

Tentatively milestoning as 1.12 since it appears to be a recent regression.

cc @randall77 @cherrymui

@thaJeztah
Copy link
Contributor

Thanks!

@josharian
Copy link
Contributor

Thanks for the bug report! If you/anyone wants to try to minimize further, setting GOGC=1 might help reproduce more reliably. (Bisection would also be super useful.)

@ijc
Copy link
Author

ijc commented Mar 21, 2019

FWIW I just tested on linux/arm64 using a binary I cross-built with the linux/amd64 toolchain described above and the issue did not occur neither with qemu-aarch64-state nor on a real machine (I tried 10,000 iterations on the latter).

The go tool objdump output on arm64 lacks most of the symbolic information to (easily) eyeball the code.

@ijc
Copy link
Author

ijc commented Mar 21, 2019

GOGC=1 doesn't seem to make a noticeable difference to the reproducibility.

@cherrymui
Copy link
Member

Interesting. I'll take a careful look.

The go tool objdump output on arm64 lacks most of the symbolic information to (easily) eyeball the code.

On ARM64, many address operations are split into multiple instructions, as it cannot load a 64-bit address in a single instruction, and currently objdump is not smart enough to recombine them.

@cherrymui
Copy link
Member

The same "volatile" stack temporary is used in two stores, both with write barriers. I think we don't generate this before, so the write barrier pass doesn't handle this case. Now we optimize X =f(); Y = X to storing the result of f() to both X and Y directly. I think we can fix this in the write barrier pass.

@ijc
Copy link
Author

ijc commented Mar 21, 2019

My bisect fingered 6c631ae "cmd/compile: in wasm, allocate approximately right number of locals for functions", but that doesn't pass the sniff test and I couldn't confirm manually. I suspect that is a misbisect (I probably got distracted and failed to test some rev but still entered a result).

I've scripted it this time and git bisect run is underway now, although I'll have to head off soon so it might be tomorrow before I see the result.

@ijc
Copy link
Author

ijc commented Mar 21, 2019

It's not done but I really have to run. It's narrowed it down so far to:

$ git log --oneline bisect/bad...bisect/good-6c631ae227cb6f15d5b0b8cc39c1715b8f4e9187~1
3bf9b77c0c (refs/bisect/bad) encoding/gob: increase "tooBig" from 1GB to 8GB on 64-bit machines
7c2718b12a doc: tweak example in Effective Go
f5df0a9575 cmd/internal/xcoff: don't use io.SeekCurrent for go1.4 compatibility
035f9e8102 (HEAD) doc: update docs.html with new tour import path
ecccdccf3e misc/wasm: fix panic on os.Stdout.Sync() in the browser
2e88689168 go/types: temporarily disable a verification in Stdlib test
2578ac54eb cmd/compile: move argument stack construction to SSA generation
6c631ae227 (refs/bisect/good-6c631ae227cb6f15d5b0b8cc39c1715b8f4e9187) cmd/compile: in wasm, allocate approximately right number of locals for functions

At this point I'd probably guess at 2578ac5 "cmd/compile: move argument stack construction to SSA generation".

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/168677 mentions this issue: cmd/compile: copy volatile values before emitting write barrier call

@cherrymui
Copy link
Member

@ijc thanks for the bisects. I think CL https://golang.org/cl/168677 should fix this. You don't have to do more.

@cherrymui
Copy link
Member

https://play.golang.org/p/5DpJ673vnSJ

On tip (700e969), this fails quite reliably.

@cherrymui
Copy link
Member

@gopherbot please backport this to Go 1.12.

@gopherbot gopherbot mentioned this issue Mar 22, 2019
Closed
@gopherbot
Copy link
Contributor

Backport issue(s) opened: #30996 (for 1.12).

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
Copy link
Contributor

Change https://golang.org/cl/168817 mentions this issue: [release-branch.go1.12] cmd/compile: copy volatile values before emitting write barrier call

@ALTree ALTree modified the milestones: Go1.12.2, Go1.13 Mar 22, 2019
@ijc
Copy link
Author

ijc commented Mar 22, 2019

The bisect was automated so I may as well report the result which was that it blamed 2578ac5 "cmd/compile: move argument stack construction to SSA generation", which seems legit.

git bisect log
git bisect start
# bad: [05e77d41914d247a1e7caf37d7125ccaa5a53505] [release-branch.go1.12] go1.12
git bisect bad 05e77d41914d247a1e7caf37d7125ccaa5a53505
# good: [41e62b8c49d21659b48a95216e3062032285250f] [release-branch.go1.11] runtime: mark sigInitIgnored nosplit
git bisect good 41e62b8c49d21659b48a95216e3062032285250f
# good: [e0faedbb5344eb6f8f704005fe88961cdc6cf5f8] cmd/go: add missing newlines in printf formats
git bisect good e0faedbb5344eb6f8f704005fe88961cdc6cf5f8
# bad: [2dda040f19aa6a7551f090d8c5a3941e416b21df] cmd/compile/internal/gc: represent labels as bare Syms
git bisect bad 2dda040f19aa6a7551f090d8c5a3941e416b21df
# good: [9a033bf9d3f5f7485d82836ec95e51a3fa74a926] cmd/compile: optimize 386's assembly generator
git bisect good 9a033bf9d3f5f7485d82836ec95e51a3fa74a926
# good: [29907b13db0455eded50263b4e37445045c82e6e] crypto: add AIX operating system
git bisect good 29907b13db0455eded50263b4e37445045c82e6e
# good: [2d6a7593b5cde33546be7bb69f420d32df060a47] cmd/doc: minor code simplification
git bisect good 2d6a7593b5cde33546be7bb69f420d32df060a47
# bad: [6994731ec2babb6a4f2bbbb08dbe649767a25942] internal/bytealg: improve asm for memequal on ppc64x
git bisect bad 6994731ec2babb6a4f2bbbb08dbe649767a25942
# skip: [e9b39417e4448d6001b2707d9cf42bba4673e9ab] go/types: copy embedded methods unchanged when completing interfaces
git bisect skip e9b39417e4448d6001b2707d9cf42bba4673e9ab
# bad: [ddf83eeb2359f28f81bce78a6d4521852d5e6dfe] cmd/compile: s/eqtype/types.Identical/ (fix build)
git bisect bad ddf83eeb2359f28f81bce78a6d4521852d5e6dfe
# good: [6c631ae227cb6f15d5b0b8cc39c1715b8f4e9187] cmd/compile: in wasm, allocate approximately right number of locals for functions
git bisect good 6c631ae227cb6f15d5b0b8cc39c1715b8f4e9187
# bad: [3bf9b77c0c8f123728fcfc802599232e9bd95476] encoding/gob: increase "tooBig" from 1GB to 8GB on 64-bit machines
git bisect bad 3bf9b77c0c8f123728fcfc802599232e9bd95476
# bad: [035f9e8102d3b46877b7462fcd365324272d1d0e] doc: update docs.html with new tour import path
git bisect bad 035f9e8102d3b46877b7462fcd365324272d1d0e
# bad: [2e88689168a57ae550ddae7ad0966fa14c877c5f] go/types: temporarily disable a verification in Stdlib test
git bisect bad 2e88689168a57ae550ddae7ad0966fa14c877c5f
# bad: [2578ac54eb417488c70324e7f3fc25565ec3f03d] cmd/compile: move argument stack construction to SSA generation
git bisect bad 2578ac54eb417488c70324e7f3fc25565ec3f03d
# first bad commit: [2578ac54eb417488c70324e7f3fc25565ec3f03d] cmd/compile: move argument stack construction to SSA generation

I also confirmed that at f23c601 both my test case and my actual usecase are fixed. Thanks for the super fast turnaround!

Is this sort of miscompile something which might precipitate a quick turnaround on a go 1.12.2? We need to decide if we should roll back our projects to 1.11.x or wait a few days.

@cherrymui
Copy link
Member

Is this sort of miscompile something which might precipitate a quick turnaround on a go 1.12.2? We need to decide if we should roll back our projects to 1.11.x or wait a few days.

@katiehockman and @andybons about the 1.12.2 release. (I don't know the answer.)

@andybons
Copy link
Member

We try to do minor point releases on the first of the month, so the next one would be April 1 ideally.

@ijc
Copy link
Author

ijc commented Mar 25, 2019

Thanks @andybons that should help us figure out what to do.

This was referenced Mar 26, 2019
Merged
Merged
@thaJeztah
Copy link
Contributor

@ALTree I see the milestone was changed from 1.12.2 to 1.13

Screenshot 2019-03-29 at 11 03 56

Is a separate tracking-issue needed to track it for Go 1.12.2?

@ALTree
Copy link
Member

ALTree commented Mar 29, 2019

@thaJeztah That's #30996. It was opened a week ago, and that's the reason I moved this one to 1.13.

@thaJeztah
Copy link
Contributor

oh! I see now; I overlooked the bot's comment as it's usually commenting related to changes in Gerrit.

Apologies; still getting the hang of the bots and flow in this repository 🤗

@ALTree
Copy link
Member

ALTree commented Mar 29, 2019

@thaJeztah No problem. Feel free to

  1. monitor that issue
  2. ping Cherry on the linked CL (https://golang.org/cl/168817) in gerrit to merge the fix in the release branch

if you are worried we may forget it before the 1.12.2 release gets finalized. It happened once, unfortunately. The cherry-picked fix for an issues was ready, but it didn't get merged and the release scripts ignored it.

gopherbot pushed a commit that referenced this issue Apr 2, 2019
@cherrymui @bradfitz
[release-branch.go1.12] cmd/compile: copy volatile values before emit…
8acc2ea 
…ting write barrier call

It is possible that a "volatile" value (one that can be clobbered
by preparing args of a call) to be used in multiple write barrier
calls. We used to copy the volatile value right before each call.
But this doesn't work if the value is used the second time, after
the first call where it is already clobbered. Copy it before
emitting any call.

Updates #30977.
Fixes #30996.

Change-Id: Iedcc91ad848d5ded547bf37a8359c125d32e994c
Reviewed-on: https://go-review.googlesource.com/c/go/+/168677
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit f23c601)
Reviewed-on: https://go-review.googlesource.com/c/go/+/168817
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
@ijc ijc mentioned this issue Apr 3, 2019
Merged
3 tasks
@golang golang locked and limited conversation to collaborators Mar 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.13
Development

No branches or pull requests
7 participants
@josharian @andybons @thaJeztah @ALTree @gopherbot @ijc @cherrymui