Skip to content

Commit 2aa473c

Browse files
committed
go/types, types2: correct alignment of atomic.Int64
atomic.Int64 has special logic in the compiler to ensure it's 8-byte aligned on 32-bit architectures. The equivalent logic is missing in go/types, which means the compiler and go/types can come to different conclusions about the layout of types. Fix this by mirroring the compiler's logic into go/types. Fixes #53884. Change-Id: I3f58a56babb76634839a161ca174c8f085fe3ba4 Reviewed-on: https://go-review.googlesource.com/c/go/+/417555 Reviewed-by: Robert Findley <[email protected]>
1 parent 4651ebf commit 2aa473c

File tree

5 files changed

+128
-2
lines changed

5 files changed

+128
-2
lines changed

src/cmd/compile/internal/types/size.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,8 @@ func calcStructOffset(errtype *Type, t *Type, o int64, flag int) int64 {
169169
}
170170
// Special case: sync/atomic.align64 is an empty struct we recognize
171171
// as a signal that the struct it contains must be 64-bit-aligned.
172+
//
173+
// This logic is duplicated in go/types and cmd/compile/internal/types2.
172174
if isStruct && t.NumFields() == 0 && t.Sym() != nil && t.Sym().Name == "align64" && isAtomicStdPkg(t.Sym().Pkg) {
173175
maxalign = 8
174176
}

src/cmd/compile/internal/types2/sizes.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,17 @@ func (s *StdSizes) Alignof(T Type) int64 {
5353
// is the same as unsafe.Alignof(x[0]), but at least 1."
5454
return s.Alignof(t.elem)
5555
case *Struct:
56+
if len(t.fields) == 0 && isSyncAtomicAlign64(T) {
57+
// Special case: sync/atomic.align64 is an
58+
// empty struct we recognize as a signal that
59+
// the struct it contains must be
60+
// 64-bit-aligned.
61+
//
62+
// This logic is equivalent to the logic in
63+
// cmd/compile/internal/types/size.go:calcStructOffset
64+
return 8
65+
}
66+
5667
// spec: "For a variable x of struct type: unsafe.Alignof(x)
5768
// is the largest of the values unsafe.Alignof(x.f) for each
5869
// field f of x, but at least 1."
@@ -93,6 +104,18 @@ func (s *StdSizes) Alignof(T Type) int64 {
93104
return a
94105
}
95106

107+
func isSyncAtomicAlign64(T Type) bool {
108+
named, ok := T.(*Named)
109+
if !ok {
110+
return false
111+
}
112+
obj := named.Obj()
113+
return obj.Name() == "align64" &&
114+
obj.Pkg() != nil &&
115+
(obj.Pkg().Path() == "sync/atomic" ||
116+
obj.Pkg().Path() == "runtime/internal/atomic")
117+
}
118+
96119
func (s *StdSizes) Offsetsof(fields []*Var) []int64 {
97120
offsets := make([]int64, len(fields))
98121
var o int64

src/cmd/compile/internal/types2/sizes_test.go

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,15 @@ import (
1414

1515
// findStructType typechecks src and returns the first struct type encountered.
1616
func findStructType(t *testing.T, src string) *types2.Struct {
17+
return findStructTypeConfig(t, src, &types2.Config{})
18+
}
19+
20+
func findStructTypeConfig(t *testing.T, src string, conf *types2.Config) *types2.Struct {
1721
f, err := parseSrc("x.go", src)
1822
if err != nil {
1923
t.Fatal(err)
2024
}
2125
info := types2.Info{Types: make(map[syntax.Expr]types2.TypeAndValue)}
22-
var conf types2.Config
2326
_, err = conf.Check("x", []*syntax.File{f}, &info)
2427
if err != nil {
2528
t.Fatal(err)
@@ -105,3 +108,39 @@ const _ = unsafe.Offsetof(struct{ x int64 }{}.x)
105108
_ = conf.Sizes.Alignof(tv.Type)
106109
}
107110
}
111+
112+
// Issue #53884.
113+
func TestAtomicAlign(t *testing.T) {
114+
const src = `
115+
package main
116+
117+
import "sync/atomic"
118+
119+
var s struct {
120+
x int32
121+
y atomic.Int64
122+
z int64
123+
}
124+
`
125+
126+
want := []int64{0, 8, 16}
127+
for _, arch := range []string{"386", "amd64"} {
128+
t.Run(arch, func(t *testing.T) {
129+
conf := types2.Config{
130+
Importer: defaultImporter(),
131+
Sizes: types2.SizesFor("gc", arch),
132+
}
133+
ts := findStructTypeConfig(t, src, &conf)
134+
var fields []*types2.Var
135+
// Make a copy manually :(
136+
for i := 0; i < ts.NumFields(); i++ {
137+
fields = append(fields, ts.Field(i))
138+
}
139+
140+
offsets := conf.Sizes.Offsetsof(fields)
141+
if offsets[0] != want[0] || offsets[1] != want[1] || offsets[2] != want[2] {
142+
t.Errorf("OffsetsOf(%v) = %v want %v", ts, offsets, want)
143+
}
144+
})
145+
}
146+
}

src/go/types/sizes.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,17 @@ func (s *StdSizes) Alignof(T Type) int64 {
5353
// is the same as unsafe.Alignof(x[0]), but at least 1."
5454
return s.Alignof(t.elem)
5555
case *Struct:
56+
if len(t.fields) == 0 && isSyncAtomicAlign64(T) {
57+
// Special case: sync/atomic.align64 is an
58+
// empty struct we recognize as a signal that
59+
// the struct it contains must be
60+
// 64-bit-aligned.
61+
//
62+
// This logic is equivalent to the logic in
63+
// cmd/compile/internal/types/size.go:calcStructOffset
64+
return 8
65+
}
66+
5667
// spec: "For a variable x of struct type: unsafe.Alignof(x)
5768
// is the largest of the values unsafe.Alignof(x.f) for each
5869
// field f of x, but at least 1."
@@ -93,6 +104,18 @@ func (s *StdSizes) Alignof(T Type) int64 {
93104
return a
94105
}
95106

107+
func isSyncAtomicAlign64(T Type) bool {
108+
named, ok := T.(*Named)
109+
if !ok {
110+
return false
111+
}
112+
obj := named.Obj()
113+
return obj.Name() == "align64" &&
114+
obj.Pkg() != nil &&
115+
(obj.Pkg().Path() == "sync/atomic" ||
116+
obj.Pkg().Path() == "runtime/internal/atomic")
117+
}
118+
96119
func (s *StdSizes) Offsetsof(fields []*Var) []int64 {
97120
offsets := make([]int64, len(fields))
98121
var o int64

src/go/types/sizes_test.go

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,16 @@ import (
1717

1818
// findStructType typechecks src and returns the first struct type encountered.
1919
func findStructType(t *testing.T, src string) *types.Struct {
20+
return findStructTypeConfig(t, src, &types.Config{})
21+
}
22+
23+
func findStructTypeConfig(t *testing.T, src string, conf *types.Config) *types.Struct {
2024
fset := token.NewFileSet()
2125
f, err := parser.ParseFile(fset, "x.go", src, 0)
2226
if err != nil {
2327
t.Fatal(err)
2428
}
2529
info := types.Info{Types: make(map[ast.Expr]types.TypeAndValue)}
26-
var conf types.Config
2730
_, err = conf.Check("x", fset, []*ast.File{f}, &info)
2831
if err != nil {
2932
t.Fatal(err)
@@ -110,3 +113,39 @@ const _ = unsafe.Offsetof(struct{ x int64 }{}.x)
110113
_ = conf.Sizes.Alignof(tv.Type)
111114
}
112115
}
116+
117+
// Issue #53884.
118+
func TestAtomicAlign(t *testing.T) {
119+
const src = `
120+
package main
121+
122+
import "sync/atomic"
123+
124+
var s struct {
125+
x int32
126+
y atomic.Int64
127+
z int64
128+
}
129+
`
130+
131+
want := []int64{0, 8, 16}
132+
for _, arch := range []string{"386", "amd64"} {
133+
t.Run(arch, func(t *testing.T) {
134+
conf := types.Config{
135+
Importer: importer.Default(),
136+
Sizes: types.SizesFor("gc", arch),
137+
}
138+
ts := findStructTypeConfig(t, src, &conf)
139+
var fields []*types.Var
140+
// Make a copy manually :(
141+
for i := 0; i < ts.NumFields(); i++ {
142+
fields = append(fields, ts.Field(i))
143+
}
144+
145+
offsets := conf.Sizes.Offsetsof(fields)
146+
if offsets[0] != want[0] || offsets[1] != want[1] || offsets[2] != want[2] {
147+
t.Errorf("OffsetsOf(%v) = %v want %v", ts, offsets, want)
148+
}
149+
})
150+
}
151+
}

0 commit comments

Comments
 (0)