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: panicking composite literals not properly flushed to globals #4608

Closed
DanielMorsing opened this issue Jan 3, 2013 · 12 comments
Closed

Comments

@DanielMorsing
Copy link
Contributor

This program causes an inconsistent memory state:

package main

import "fmt"

type L struct {
        x, y int
}

var x L

func main() {
        foo(0)
        fmt.Println(x)
}

func foo(bar int) {
        defer func() {
                _ = recover()
        }()
        x = L{ 1, 1/bar}
}

The expected output from this program is {0 0}, but it outputs {1 0} instead.

Potential solution is evaluating the non-const parts of the literal into temporaries
before copying the static data.
@rsc
Copy link
Contributor

rsc commented Jan 4, 2013

Comment 1:

I am not convinced that the spec disallows this behavior. Perhaps it does.

Labels changed: added priority-later, removed priority-triage.

Status changed to Thinking.

@DanielMorsing
Copy link
Contributor Author

Comment 2:

From the section on assignments:
"The assignment proceeds in two phases. First, the operands of index expressions and
pointer indirections (including implicit pointer indirections in selectors) on the left
and the expressions on the right are all evaluated in the usual order. Second, the
assignments are carried out in left-to-right order."
This seems to state that any evaluation must be completed before an assignment happens.

@rsc
Copy link
Contributor

rsc commented Mar 12, 2013

Comment 3:

Labels changed: added go1.1maybe, removed go1.1.

@robpike
Copy link
Contributor

robpike commented May 18, 2013

Comment 4:

Labels changed: added go1.2maybe, removed go1.1maybe.

@DanielMorsing
Copy link
Contributor Author

Comment 5:

Are we finished thinking?
Considering how expensive it is to evaluate into variables, I think the current behavior
is ok, but I can't find any justification for it in the spec.

@rsc
Copy link
Contributor

rsc commented Jul 29, 2013

Comment 6:

Nope, still thinking. :-)

@rsc
Copy link
Contributor

rsc commented Jul 30, 2013

Comment 7:

Labels changed: added feature.

@rsc
Copy link
Contributor

rsc commented Sep 9, 2013

Comment 8:

Labels changed: added go1.3maybe, removed go1.2maybe.

@rsc
Copy link
Contributor

rsc commented Nov 27, 2013

Comment 9:

Labels changed: removed feature.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 10:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 11:

Labels changed: added repo-main.

@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@rsc rsc changed the title cmd/gc: panicking composite literals not properly flushed to globals cmd/compile: panicking composite literals not properly flushed to globals Jun 8, 2015
@odeke-em
Copy link
Member

odeke-em commented Feb 9, 2020

Thank you for reporting this issue @DanielMorsing and for the patience!

Indeed this was an inconsistency, but @cherrymui's CL https://go-review.googlesource.com/c/go/+/36410/ for Go1.8 in 2017, appears to have fixed the issue. To me the issue seems to have been incorrect writer barriers for globals before panics were evaluated.

In Go1.8 and later, we now get {0, 0} as per https://play.golang.org/p/f2SppbMf9bc which as per Go1.13.7 prints

go1.13.7
{0 0}

I shall close this issue now.

@odeke-em odeke-em closed this as completed Feb 9, 2020
@golang golang locked and limited conversation to collaborators Feb 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants