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

cmd/compile: 1-sized array's pointer typed element set to nil, after received from channel and print in a loop #22683

Closed
helinwang opened this issue Nov 13, 2017 · 15 comments

Comments

Projects
None yet
9 participants
@helinwang
Copy link

commented Nov 13, 2017

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

go version go1.9.2 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/helin/env/gopath"
GORACE=""
GOROOT="/home/helin/env/go"
GOTOOLDIR="/home/helin/env/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build809756853=/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?

package main

import (
	"fmt"
)

const arraySize = 1

// comment above and uncomment below, will work as expected.
// const arraySize = 2

type foo struct {
	bar [arraySize]*int
}

func main() {
	ch := make(chan foo, 2)
	var a int
	var b [arraySize]*int
	b[0] = &a
	ch <- foo{bar: b}
	close(ch)

	for v := range ch {
		// uncomment below, will work as expected.
		// fmt.Println(v.bar[0])
		for i := 0; i < 1; i++ {
			fmt.Println(v.bar[0], b[0])
		}
	}
}

What did you expect to see?

// Output:
// 0x10410020 0x10410020

What did you see instead?

// Output:
// <nil> 0x10410020

Playground link:

https://play.golang.org/p/Qd8fDy-ls3

@helinwang helinwang changed the title 1-sized array's point typed element set to nil, after received from channel and print in a loop 1-sized array's pointer typed element set to nil, after received from channel and print in a loop Nov 13, 2017

@as

This comment has been minimized.

Copy link
Contributor

commented Nov 13, 2017

I dont understand the issue. Why are you closing the channel and then attempting to recieve on that closed channel?

@helinwang

This comment has been minimized.

Copy link
Author

commented Nov 13, 2017

@as receiving from the closed channel is safe, it will read all the remaining values.
In this case I just don't want the for ... range to block forever.

@robpike

This comment has been minimized.

Copy link
Contributor

commented Nov 13, 2017

Given that there are no explicit goroutines and that printing a value changes the behavior, I think this is a compiler bug or perhaps a runtime bug.

@bradfitz bradfitz changed the title 1-sized array's pointer typed element set to nil, after received from channel and print in a loop cmd/compile: 1-sized array's pointer typed element set to nil, after received from channel and print in a loop Nov 13, 2017

@bradfitz bradfitz added the NeedsFix label Nov 13, 2017

@bradfitz bradfitz added this to the Go1.10 milestone Nov 13, 2017

@bradfitz

This comment has been minimized.

Copy link
Member

commented Nov 13, 2017

@mdempsky

This comment has been minimized.

Copy link
Member

commented Nov 13, 2017

I can repro this with Go 1.9, but it seems to work as expected at master.

@dr2chase

This comment has been minimized.

Copy link
Contributor

commented Nov 13, 2017

Array-of-size-1 is a special case in the SSA code, too.

@randall77

This comment has been minimized.

Copy link
Contributor

commented Nov 13, 2017

This is the buggy rule:

(ArraySelect [0] (Load ptr mem)) -> (Load ptr mem)

It allows the load to move blocks. It needs an @v.Block modifier, I think.

@randall77 randall77 self-assigned this Nov 13, 2017

@randall77 randall77 modified the milestones: Go1.10, Go1.9.3 Nov 13, 2017

@randall77

This comment has been minimized.

Copy link
Contributor

commented Nov 13, 2017

The bad rule was removed at tip during https://go-review.googlesource.com/c/go/+/71731 . Seems coincidental that it was removed, but that's why tip is ok.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Nov 13, 2017

Add test + candidate for backport?

@gopherbot

This comment has been minimized.

Copy link

commented Nov 13, 2017

Change https://golang.org/cl/77331 mentions this issue: [release-branch.go1.9] cmd/compile: fix decomposition of 1-element arrays

@gopherbot

This comment has been minimized.

Copy link

commented Nov 13, 2017

Change https://golang.org/cl/77332 mentions this issue: cmd/compile: add test for array decomposition

@dr2chase

This comment has been minimized.

Copy link
Contributor

commented Nov 13, 2017

The removal was indeed coincidental; since I was freshening someone else's old patch, I just kept the change. I'm trying to figure out if we need it, given the other make and select rules.

gopherbot pushed a commit that referenced this issue Nov 13, 2017

cmd/compile: add test for array decomposition
This test fails on 1.9.2, but is ok on tip.
CL 77331 has both the 1.9.2 fix and this test, and is on the 1.9 release branch.
This CL is just the test, and is on HEAD.  The buggy code doesn't exist on tip.

Update #22683

Change-Id: I04a24bd6c2d3068e18ca81da3347e2c1366f4447
Reviewed-on: https://go-review.googlesource.com/77332
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@andybons

This comment has been minimized.

Copy link
Member

commented Jan 18, 2018

CL 77331 OK for Go 1.9.3.

@gopherbot

This comment has been minimized.

Copy link

commented Jan 18, 2018

Change https://golang.org/cl/88320 mentions this issue: [release-branch.go1.9] cmd/compile: add test for array decomposition

gopherbot pushed a commit that referenced this issue Jan 22, 2018

[release-branch.go1.9] cmd/compile: fix decomposition of 1-element ar…
…rays

The offending rule could move the load to a different block,
which is always a bad idea.

Fixes #22683

Change-Id: I973c88389b2359f734924d9f45c3fb38e166691d
Reviewed-on: https://go-review.googlesource.com/77331
Run-TryBot: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: David Chase <drchase@google.com>
@andybons

This comment has been minimized.

Copy link
Member

commented Jan 22, 2018

go1.9.3 has been packaged and includes:

  • CL 77331 [release-branch.go1.9] cmd/compile: fix decomposition of 1-element arrays

The release is posted at golang.org/dl.

— golang.org/x/build/cmd/releasebot, Jan 22 21:02:55 UTC

@andybons andybons closed this Jan 22, 2018

@golang golang locked and limited conversation to collaborators Jan 22, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.