Skip to content
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.

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

go/compile: corrupted function return value #23812

Closed
cznic opened this issue Feb 13, 2018 · 11 comments
Closed

go/compile: corrupted function return value #23812

cznic opened this issue Feb 13, 2018 · 11 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge release-blocker
Milestone

Comments

@cznic
Copy link
Contributor

cznic commented Feb 13, 2018

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

go version go1.9.4 linux/amd64

Does this issue reproduce with the latest release?

Yes (go version devel +2010189407 Tue Feb 13 16:34:46 2018 +0000 linux/amd64)

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/jnml"
GORACE=""
GOROOT="/home/jnml/go"
GOTOOLDIR="/home/jnml/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build334481324=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

What did you do?

// Minimized reproducer for the (incorrect) translation of github.com/gcc-mirror/gcc/gcc/testsuite/gcc.c-torture/execute/pr70429.c

package main

func main() {
	if foo(1) != int32(0x3edae8) {
		panic(42)
	}
}

func foo(_a int32) (r int32) {
	//defer func() { println("r ", r) }() // <- Uncommenting this defer statement makes the panic go away.

	return shr1(int32(shr2(int64(0x14ff6e2207db5d1f), int(_a))), 4)
}

func shr1(n int32, m int) int32 { return n >> uint(m) }

func shr2(n int64, m int) int64 {
	if m < 0 { // <- Commenting out this if statement makes the panic go away.
		m = -m
	}
	if m >= 64 { // <- Commenting out this if statement makes the panic go away.
		return n
	}

	return n >> uint(m)
}

Playground

What did you expect to see?

No panic.

What did you see instead?

Panic.

@bradfitz bradfitz added this to the Go1.11 milestone Feb 13, 2018
@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label Feb 13, 2018
@bradfitz
Copy link
Contributor

To @randall77 for codegen.

@randall77
Copy link
Contributor

Probably introduced here: https://go-review.googlesource.com/c/go/+/13641
@brtzsnr
Fix coming in a sec.

@randall77
Copy link
Contributor

Possibly worth backporting to 1.9 and 1.10.
Pretty obscure though.

@gopherbot
Copy link

Change https://golang.org/cl/93716 mentions this issue: cmd/compile: fix constant folding of right shifts

@randall77
Copy link
Contributor

Reopening for possible 1.10 final and 1.9.5.

@randall77 randall77 reopened this Feb 14, 2018
@randall77 randall77 modified the milestones: Go1.11, Go1.10 Feb 14, 2018
@gopherbot
Copy link

Change https://golang.org/cl/94028 mentions this issue: cmd/compile: fix constant folding of right shifts on s390x

gopherbot pushed a commit that referenced this issue Feb 14, 2018
Repeat previous fix on amd64 for s390x.
Sub-word right shifts should sign extend before shifting.

Update #23812

Change-Id: I2d770190c7d8a22310b0dbd9facb3fb05afa362a
Reviewed-on: https://go-review.googlesource.com/94028
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@ianlancetaylor ianlancetaylor modified the milestones: Go1.10, Go1.9.5 Feb 15, 2018
@gopherbot
Copy link

Change https://golang.org/cl/94215 mentions this issue: [release-branch.go1.10] cmd/compile: fix constant folding of right shifts

@gopherbot
Copy link

Change https://golang.org/cl/94216 mentions this issue: [release-branch.go1.10] cmd/compile: fix constant folding of right shifts on s390x

gopherbot pushed a commit that referenced this issue Feb 15, 2018
…ifts

The sub-word shifts need to sign-extend before shifting, to avoid
bringing in data from higher in the argument.

Fixes #23812

Change-Id: I0a95a0b49c48f3b40b85765bb4a9bb492be0cd73
Reviewed-on: https://go-review.googlesource.com/93716
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
(cherry picked from commit 755b36a)
Reviewed-on: https://go-review.googlesource.com/94215
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
gopherbot pushed a commit that referenced this issue Feb 15, 2018
…ifts on s390x

Repeat previous fix on amd64 for s390x.
Sub-word right shifts should sign extend before shifting.

Update #23812

Change-Id: I2d770190c7d8a22310b0dbd9facb3fb05afa362a
Reviewed-on: https://go-review.googlesource.com/94028
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit 1566bf9)
Reviewed-on: https://go-review.googlesource.com/94216
neelance pushed a commit to neelance/go that referenced this issue Feb 27, 2018
…ifts

The sub-word shifts need to sign-extend before shifting, to avoid
bringing in data from higher in the argument.

Fixes golang#23812

Change-Id: I0a95a0b49c48f3b40b85765bb4a9bb492be0cd73
Reviewed-on: https://go-review.googlesource.com/93716
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
(cherry picked from commit 755b36a)
Reviewed-on: https://go-review.googlesource.com/94215
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
neelance pushed a commit to neelance/go that referenced this issue Feb 27, 2018
…ifts on s390x

Repeat previous fix on amd64 for s390x.
Sub-word right shifts should sign extend before shifting.

Update golang#23812

Change-Id: I2d770190c7d8a22310b0dbd9facb3fb05afa362a
Reviewed-on: https://go-review.googlesource.com/94028
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit 1566bf9)
Reviewed-on: https://go-review.googlesource.com/94216
@andybons
Copy link
Member

andybons commented Mar 27, 2018

CL 93716 OK for Go 1.9.5
CL 94028 OK for Go 1.9.5

@andybons andybons added CherryPickApproved Used during the release process for point releases and removed NeedsFix The path to resolution is known, but the work has not been done. labels Mar 27, 2018
@gopherbot
Copy link

Change https://golang.org/cl/102775 mentions this issue: [release-branch.go1.9] cmd/compile: fix constant folding of right shifts

@gopherbot
Copy link

Change https://golang.org/cl/102777 mentions this issue: [release-branch.go1.9] cmd/compile: fix constant folding of right shifts on s390x

gopherbot pushed a commit that referenced this issue Mar 29, 2018
The sub-word shifts need to sign-extend before shifting, to avoid
bringing in data from higher in the argument.

Fixes #23812

Change-Id: I0a95a0b49c48f3b40b85765bb4a9bb492be0cd73
Reviewed-on: https://go-review.googlesource.com/93716
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-on: https://go-review.googlesource.com/102775
Run-TryBot: Andrew Bonventre <andybons@golang.org>
gopherbot pushed a commit that referenced this issue Mar 29, 2018
…fts on s390x

Repeat previous fix on amd64 for s390x.
Sub-word right shifts should sign extend before shifting.

Update #23812

Change-Id: I2d770190c7d8a22310b0dbd9facb3fb05afa362a
Reviewed-on: https://go-review.googlesource.com/94028
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-on: https://go-review.googlesource.com/102777
Run-TryBot: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
@golang golang locked and limited conversation to collaborators Mar 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge release-blocker
Projects
None yet
Development

No branches or pull requests

6 participants