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.

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

cmd/compile: comma-ok form with interface{} seems to read garbage #16870

Closed
tv42 opened this issue Aug 24, 2016 · 9 comments
Closed

cmd/compile: comma-ok form with interface{} seems to read garbage #16870

tv42 opened this issue Aug 24, 2016 · 9 comments
Labels
FrozenDueToAge
Milestone
Go1.8

Comments

@tv42
Copy link

tv42 commented Aug 24, 2016

Please answer these questions before submitting your issue. Thanks!

  1. What version of Go are you using (go version)?
  • playground
  • go version go1.7 linux/amd64
  • go version devel +35f5517 2016-08-16 01:04:17 +0000 linux/amd64
  1. What operating system and processor architecture are you using (go env)?

playground and

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/tv/go"
GORACE=""
GOROOT="/home/tv/src/go-1.7"
GOTOOLDIR="/home/tv/src/go-1.7/pkg/tool/linux_amd64"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/home/tv/tmp/go-build074452883=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
  1. What did you do?

Inspired by #15782 (comment) I decided to explore what giving types to the comma-ok form even means. It seems I got things to break:

A) https://play.golang.org/p/rogSaQq7Hf panics on playground, 1.7 and tip say ok=<invalid reflect.Value>. I was curious how fmt ends up with invalid reflect.Values, and explored further.

B) echlebek on IRC came up with https://play.golang.org/p/LOXGni0AH9 which results in ok being a complex on playground! Behavior seen on Go1.7: ok=(1.3109511e-38+0i), ok=1.3109511e-38,

tv@brute ~/tmp$ go-tip run LOXGni0AH9.go
v=0
fatal error: unexpected signal during runtime execution
[signal SIGSEGV: segmentation violation code=0x1 addr=0xc440097ec0 pc=0x40a7dc]

goroutine 1 [running]:
runtime.throw(0x4ab6f3, 0x2a)
    /home/tv/src/go/src/runtime/panic.go:566 +0x95 fp=0xc42004b898 sp=0xc42004b878
runtime.sigpanic()
    /home/tv/src/go/src/runtime/sigpanic_unix.go:12 +0x2cc fp=0xc42004b8f0 sp=0xc42004b898
runtime.additab(0x7fd05f4c1000, 0x101)
    /home/tv/src/go/src/runtime/iface.go:98 +0x8c fp=0xc42004b9e0 sp=0xc42004b8f0
runtime.getitab(0x491d80, 0xc42004bf00, 0x1, 0x0)
    /home/tv/src/go/src/runtime/iface.go:79 +0x1af fp=0xc42004ba78 sp=0xc42004b9e0
runtime.assertE2I2(0x491d80, 0xc42004bf00, 0x7fd05f51d000, 0xc42004bb38, 0xc420000258)
    /home/tv/src/go/src/runtime/iface.go:383 +0x7a fp=0xc42004baa8 sp=0xc42004ba78
fmt.(*pp).handleMethods(0xc42008c000, 0xc400000076, 0x200000000)
    /home/tv/src/go/src/fmt/print.go:554 +0xb4 fp=0xc42004bb58 sp=0xc42004baa8
fmt.(*pp).printArg(0xc42008c000, 0xc42004bf00, 0x7fd05f51d000, 0x76)
    /home/tv/src/go/src/fmt/print.go:665 +0x17b fp=0xc42004bc50 sp=0xc42004bb58
fmt.(*pp).doPrintf(0xc42008c000, 0x4a6489, 0x7, 0xc42004bef8, 0x1, 0x1)
    /home/tv/src/go/src/fmt/print.go:985 +0x1240 fp=0xc42004bd38 sp=0xc42004bc50
...
@bradfitz
Copy link
Contributor

Nice!

/Cc @randall77

@griesemer
Copy link
Contributor

@tv42 re: what it means:

var x, ok T = m[k]

is just a short form of

var x, ok T
x, ok = m[k]

This latter version was always present in the spec. Note that we only talk about the syntactic form here - the usual rules for types etc. must be followed independently.

But nice error indeed!

@tv42
Copy link
Author

tv42 commented Aug 25, 2016

@griesemer Yeah, I understand the syntax, I was trying to see what possible use would that form ever have, considering that ok is going to get a boolean value. And then there were fireworks.

@griesemer griesemer added this to the Go1.8 milestone Aug 25, 2016
@griesemer
Copy link
Contributor

Related: https://play.golang.org/p/G3dH79rD_Y also doesn't work right.

At tip:

package main

import (
    "fmt"
)

func main() {
    var x interface{} = 0
    var v, ok interface{} = x.(int)
    fmt.Printf("v=%#v\n", v)
    fmt.Printf("ok=%#v\n", ok)
}

produces

$ go run x.go
# command-line-arguments
./x.go:9: non-bool ok (type interface {}) used as if condition

Looks like a compiler frontend issue.

@griesemer
Copy link
Contributor

Same for channel ops:

func main() {
    ch := make(chan int, 1)
    ch <- 1
    var v, ok interface{} = <-ch
    fmt.Printf("v=%#v\n", v)
    fmt.Printf("ok=%#v\n", ok)
}

produces

$ go run x.go
v=1
ok=0x0

(the ok should be printed as false rather than 0x0). And finally:

func main() {
    ch := make(chan int)
    close(ch)
    var v, ok interface{} = <-ch
    fmt.Printf("v=%#v\n", v)
    fmt.Printf("ok=%#v\n", ok)
}

produces

$ go run x.go
v=0
ok=<invalid reflect.Value>

Looks like nobody has ever used the comma-ok form outside a short variable declaration (or perhaps with a non-bool ok).

@mdempsky
Copy link
Member

mdempsky commented Aug 25, 2016 •
edited

The issues all appear to be in walk.go's handling of various OAS2xxx nodes. In particular, they all assume ok to be of boolean type.

OAS2RECV generates a call to runtime.chanrecv2 and pretends its return type is ok's, which is problematic if ok is interface{}.

OAS2MAPR does something similar, rewriting mapaccess2*'s second result parameter type to match ok's.

OAS2DOTTYPE for conversions to a concrete type generates an expression to assign to ok. The assignment succeeds by creating an implicit conversion. But the ok variable is used in an OIF node as the condition later, which fails.

@bradfitz
Copy link
Contributor

Looks like this is also broken on Go 1.6 (but with different results? or different luck) so this doesn't seem to be a Go 1.7.1 candidate. Others agree?

@josharian
Copy link
Contributor

Agreed. The usage is so unusual and the failure mode is so spectacular (nice find!) that I doubt this is on the critical path for 1.7.1.

@josharian josharian changed the title comma-ok form with interface{} seems to read garbage cmd/compile: comma-ok form with interface{} seems to read garbage Aug 25, 2016
@mdempsky mdempsky self-assigned this Aug 25, 2016
@gopherbot
Copy link

CL https://golang.org/cl/27910 mentions this issue.

@golang golang locked and limited conversation to collaborators Sep 14, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.8
Development

No branches or pull requests
6 participants
@bradfitz @mdempsky @josharian @tv42 @griesemer @gopherbot