search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.03k stars 17.67k forks source link

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

Closed ijc closed 5 years ago

ijc commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

ping @bradfitz PTAL

ALTree commented 5 years ago

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

cc @randall77 @cherrymui

thaJeztah commented 5 years ago

Thanks!

josharian commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

cherrymui commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

My bisect fingered 6c631ae227cb6f15d5b0b8cc39c1715b8f4e9187 "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 commented 5 years ago

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 2578ac54eb "cmd/compile: move argument stack construction to SSA generation".

gopherbot commented 5 years ago

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

cherrymui commented 5 years ago

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

cherrymui commented 5 years ago

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

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

cherrymui commented 5 years ago

@gopherbot please backport this to Go 1.12.

gopherbot commented 5 years ago

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 commented 5 years ago

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

ijc commented 5 years ago

The bisect was automated so I may as well report the result which was that it blamed 2578ac54eb417488c70324e7f3fc25565ec3f03d "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 f23c601bf9f66ef4a0d9d2dcd003b95e78fb28f8 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 commented 5 years ago

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 commented 5 years ago

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

ijc commented 5 years ago

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

thaJeztah commented 5 years ago

@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 commented 5 years ago

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

thaJeztah commented 5 years ago

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 commented 5 years ago

@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.