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: struct interface{} value lost passing by value #30956
Comments
Note that it only occurs when not assigned to a variable.
It also seems to occur only when certain fields are added to |
It seems that now we generate static data for X. However, the symbol
(no content for the stmp_0 symbol)
(the address is overlapping with the next symbol.) |
git bisect points to 0e9f8a2 as the first bad commit |
Thanks for bisecting! Based on where that landed, I suspect that that commit uncovered a latent bug. Looking at sinit.go on my phone, it looks like we need to add special handling for pointer-to-zero-width-val when constructing static interface data. |
Looks like somewhere around line 522 in sinit.go we need to use runtime.zerobase for this case. |
It looks to me that Maybe we should teach Alternatively, maybe |
@josharian @cherrymui I did some investigation, and found the culprit is this line 0e9f8a2#diff-f429d083e18b2e96d5c8ebeae38f3c54R1052 Before that change, everything except interface will be added temporary address. So when go/src/cmd/compile/internal/gc/sinit.go Line 739 in 4906a00
With that change, the struct does not have temp address, the process become:
Interestingly, if I added an addressable field, then problem gone, see: https://play.golang.org/p/DnCxNoFUB0P I simply fix by this change:
The problem gone, If both of you feel ok with that fix, I will send a CL. Any idea? cc @randall77 |
@Gnouc Thanks for digging into this! I think this is probably on the right direction. But special-casing structs seems not the best way. At least, I think we should also handle arrays. And not all structs need addrTemp. Also, ideally, I think this code is supposed to handle static data and generate static data. With your change, it doesn't generate static data for X. This may be fine, as Go 1.11 doesn't either. Maybe conditioning on |
I think this is because with the map field, X is not static any more. |
@cherrymui |
Change https://golang.org/cl/168858 mentions this issue: |
@gopherbot, please consider this for backport to 1.12, it's a regression. |
Backport issue(s) opened: #31209 (for 1.12). Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases. |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
yes
What operating system and processor architecture are you using (
go env
)?Linux amd64
What did you do?
Consider this program (https://play.golang.org/p/lblyBSvvbis):
What did you expect to see?
{42}
printed.What did you see instead?
With Go 1.12.1, this program prints
<nil>
. With Go 1.11.6, it prints{42}
as expected.The text was updated successfully, but these errors were encountered: