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/gc: Method values + slicing arrays + inlining == Broken strings #5753

Closed
dominikh opened this issue Jun 22, 2013 · 6 comments
Closed

cmd/gc: Method values + slicing arrays + inlining == Broken strings #5753

dominikh opened this issue Jun 22, 2013 · 6 comments
Labels
FrozenDueToAge
Milestone
Go1.1.2

Comments

@dominikh
Copy link
Member 

What steps will reproduce the problem?
If possible, include a link to a program on play.golang.org.
1. Compile and run http://play.golang.org/p/Yl6EXET0fc (can reproduce on the playground
as well)
2. Notice how the length for the second string becomes ridiculously big
3.

What is the expected output?
Not breaking the string

What do you see instead?
Broken strings that will cause Go to read memory it shouldn't.

Which compiler are you using (5g, 6g, 8g, gccgo)?
6g

Which operating system are you using?
Linux

Which version are you using?  (run 'go version')
This happens both with Go 1.1 and current tip (go version devel +56909cb770fe Fri Jun 21
18:07:57 2013 -0700 linux/amd64)

Please provide any additional information below.

1) This only happens when using method values. Function values are fine
2) This only happens when manually creating the backing array
3) Disabling inlining avoids the issue.

(Reporting this on behalf of Mikhail Panchenko who initially discovered this issue in a
more complex scenario)
@minux
Copy link
Member

minux commented Jun 22, 2013

Comment 1:

Issue #5754 has been merged into this issue.

@DanielMorsing
Copy link
Contributor

Comment 2:

Looks like a case of bad escape analysis. The broken method thunk allocates foo on the
stack.

Status changed to Accepted.

@ianlancetaylor
Copy link
Contributor

Comment 3:

Labels changed: added priority-asap, go1.1.2, removed priority-triage.

@DanielMorsing
Copy link
Contributor

Comment 4:

Fix at https://golang.org/cl/10383048

Owner changed to @DanielMorsing.

Status changed to Started.

@DanielMorsing
Copy link
Contributor

Comment 5:

This issue was closed by revision 7cfa831.

Status changed to Fixed.

@adg
Copy link
Contributor

adg commented Jul 22, 2013

Comment 6:

This issue was closed by revision e503b72350b2.

@minux minux mentioned this issue Dec 8, 2014
Closed
@rsc rsc added this to the Go1.1.2 milestone Apr 14, 2015
@rsc rsc removed the go1.1.2 label Apr 14, 2015
adg added a commit that referenced this issue May 11, 2015
@adg
[release-branch.go1.1] cmd/gc: fix issue with method wrappers not hav…
b82120b 
…ing escape analysis run on them.

««« CL 10383048 / 58e15340e78f
cmd/gc: fix issue with method wrappers not having escape analysis run on them.

Escape analysis needs the right curfn value on a dclfunc node, otherwise it will not analyze the function.
When generating method value wrappers, we forgot to set the curfn correctly.

Fixes #5753.

R=golang-dev, rsc
CC=golang-dev
https://golang.org/cl/10383048
»»»

Update #5928

R=golang-dev, dave
CC=golang-dev
https://golang.org/cl/11669043
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
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.1.2
Development

No branches or pull requests
7 participants
@dominikh @rsc @minux @DanielMorsing @ianlancetaylor @adg @gopherbot