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

go/types: types.StdSizes and unsafe.Sizeof disagree about size of struct #14909

Closed
dominikh opened this issue Mar 22, 2016 · 7 comments
Closed

go/types: types.StdSizes and unsafe.Sizeof disagree about size of struct #14909

dominikh opened this issue Mar 22, 2016 · 7 comments
Assignees
Milestone

Comments

@dominikh
Copy link
Member

@dominikh dominikh commented Mar 22, 2016

  1. What version of Go are you using (go version)?
    go version go1.6 linux/amd64
  2. What operating system and processor architecture are you using (go env)?
    GOHOSTARCH="amd64"
    GOHOSTOS="linux"
  3. What did you do?
    foo.go is http://play.golang.org/p/MwjE9tNkB4

I'm using guru describe as a simple client of types.StdSizes.

$ guru describe /tmp/src/foo/foo.go:\#765 | grep size
/tmp/src/foo/foo.go:43.6-43.7: definition of type pp (size 195, align 8)

$ go run /tmp/src/foo/foo.go                  
2016/03/22 09:08:30 200
  1. What did you expect to see?
    I expect guru (types.StdSizes) and unsafe.Sizeof to agree
  2. What did you see instead?
    They disagree.

The maligned tool has its own (partial) types.Sizes implementation that only differs in the way it calculates the size of a *types.Struct, and produces the correct result.

/cc @alandonovan @griesemer

@dominikh dominikh changed the title x/tools/cmd/guru: guru and unsafe.Sizeof disagree about size of struct go/types: types.StdSizes and unsafe.Sizeof disagree about size of struct Mar 22, 2016
@dominikh
Copy link
Member Author

@dominikh dominikh commented Mar 22, 2016

The issue here is that StdSizes doesn't compute the final padding in a struct. The pp type has 5 bytes of padding at the end, to make the struct as a whole align on 8 bytes.

While looking into this, I noticed another issue: Since Go 1.5, zero-sized fields at the end of the struct effectively have size 1 (see #9401). However, that's specific to the gc compiler, and other compilers might have a different layout. What should go/types do here?

Loading

@ianlancetaylor ianlancetaylor added this to the Go1.7 milestone Mar 22, 2016
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Mar 22, 2016

unsafe.Sizeof returns different values on different architectures. And, as you suggest, on different compilers. It's not obvious to me that there is a right answer here.

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented Mar 22, 2016

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

Loading

@dominikh
Copy link
Member Author

@dominikh dominikh commented Mar 22, 2016

@ianlancetaylor The problem that this issue tracks has nothing to do with different architectures, but the fact that go/types omits the trailing padding in a struct. AFAIK, that is dictated by the specification.

The other problem, which would get its own issue, is in fact harder to answer.

Loading

@griesemer
Copy link
Contributor

@griesemer griesemer commented Mar 22, 2016

@dominikh I disagree that this is dictated by the specification. Can you elaborate a bit more?

I've been arguing in the past that adding the padding at the end can lead to large waste when such a struct is used in an array, or inside another struct.

Loading

@griesemer
Copy link
Contributor

@griesemer griesemer commented Mar 22, 2016

Elaborating a bit more: There are reasons (as pointed out by issue #9401) why an implementation might want to add some padding. There are reasons why an implementation might want to always add regular padding at the end of a struct. But those are implementation decisions.

There is nothing in the language spec (I believe) that requires a certain padding or even demands that the field order in memory matches the field order declared in a struct declaration.

Unfortunately, too much code is depending on such an order and even the padding (or so people believe) that it's become "impossible" to change this w/o breaking code in very subtle ways (though I don't have specific evidence for this claim, either). That said, we have refrained in the past from demanding (via the spec) that the order be fixed. See issue #10014 for a discussion.

Regarding this issue specifically: go/types does not have to agree 100% with the compilers in this respect. It's a model implementation, and in this case we chose to model a more efficient layout.

If need be, it has hooks to change the size computations as needed. Clients can customize it.

Loading

@griesemer griesemer closed this Mar 22, 2016
@dominikh
Copy link
Member Author

@dominikh dominikh commented Mar 22, 2016

Thanks @griesemer – I have to agree with you. I was referring to the struct's alignment (largest alignment of the fields, but at least 1) but that doesn't necessitate the padding to be in the struct itself.

Thanks for the elaborate explanation.

Loading

@golang golang locked and limited conversation to collaborators Mar 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants