Skip to content

Commit 873401d

Browse files
committed
cmd/compile: ensure equal functions don't do unaligned loads
On architectures which don't support unaligned loads, make sure we don't generate code that requires them. Generated hash functions also matter in this respect, but they all look ok. Update #37716 Fixes #46283 Change-Id: I6197fdfe04da4428092c99bd871d93738789e16b Reviewed-on: https://go-review.googlesource.com/c/go/+/322151 Trust: Keith Randall <[email protected]> Trust: Josh Bleecher Snyder <[email protected]> Run-TryBot: Keith Randall <[email protected]> Reviewed-by: Cherry Mui <[email protected]> Reviewed-by: Josh Bleecher Snyder <[email protected]> Reviewed-by: eric fang <[email protected]> TryBot-Result: Go Bot <[email protected]>
1 parent b836106 commit 873401d

File tree

3 files changed

+135
-0
lines changed

3 files changed

+135
-0
lines changed

Diff for: src/cmd/compile/internal/reflectdata/alg.go

+20
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package reflectdata
66

77
import (
88
"fmt"
9+
"math/bits"
910
"sort"
1011

1112
"cmd/compile/internal/base"
@@ -47,6 +48,11 @@ func eqCanPanic(t *types.Type) bool {
4748
func AlgType(t *types.Type) types.AlgKind {
4849
a, _ := types.AlgType(t)
4950
if a == types.AMEM {
51+
if t.Alignment() < int64(base.Ctxt.Arch.Alignment) && t.Alignment() < t.Width {
52+
// For example, we can't treat [2]int16 as an int32 if int32s require
53+
// 4-byte alignment. See issue 46283.
54+
return a
55+
}
5056
switch t.Width {
5157
case 0:
5258
return types.AMEM0
@@ -769,6 +775,20 @@ func memrun(t *types.Type, start int) (size int64, next int) {
769775
if f := t.Field(next); f.Sym.IsBlank() || !isRegularMemory(f.Type) {
770776
break
771777
}
778+
// For issue 46283, don't combine fields if the resulting load would
779+
// require a larger alignment than the component fields.
780+
if base.Ctxt.Arch.Alignment > 1 {
781+
align := t.Alignment()
782+
if off := t.Field(start).Offset; off&(align-1) != 0 {
783+
// Offset is less aligned than the containing type.
784+
// Use offset to determine alignment.
785+
align = 1 << uint(bits.TrailingZeros64(uint64(off)))
786+
}
787+
size := t.Field(next).End() - t.Field(start).Offset
788+
if size > align {
789+
break
790+
}
791+
}
772792
}
773793
return t.Field(next-1).End() - t.Field(start).Offset, next
774794
}

Diff for: src/cmd/compile/internal/test/align_test.go

+96
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
// Copyright 2021 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
// Test to make sure that equality functions (and hash
6+
// functions) don't do unaligned reads on architectures
7+
// that can't do unaligned reads. See issue 46283.
8+
9+
package test
10+
11+
import "testing"
12+
13+
type T1 struct {
14+
x float32
15+
a, b, c, d int16 // memequal64
16+
}
17+
type T2 struct {
18+
x float32
19+
a, b, c, d int32 // memequal128
20+
}
21+
22+
type A2 [2]byte // eq uses a 2-byte load
23+
type A4 [4]byte // eq uses a 4-byte load
24+
type A8 [8]byte // eq uses an 8-byte load
25+
26+
//go:noinline
27+
func cmpT1(p, q *T1) {
28+
if *p != *q {
29+
panic("comparison test wrong")
30+
}
31+
}
32+
33+
//go:noinline
34+
func cmpT2(p, q *T2) {
35+
if *p != *q {
36+
panic("comparison test wrong")
37+
}
38+
}
39+
40+
//go:noinline
41+
func cmpA2(p, q *A2) {
42+
if *p != *q {
43+
panic("comparison test wrong")
44+
}
45+
}
46+
47+
//go:noinline
48+
func cmpA4(p, q *A4) {
49+
if *p != *q {
50+
panic("comparison test wrong")
51+
}
52+
}
53+
54+
//go:noinline
55+
func cmpA8(p, q *A8) {
56+
if *p != *q {
57+
panic("comparison test wrong")
58+
}
59+
}
60+
61+
func TestAlignEqual(t *testing.T) {
62+
cmpT1(&T1{}, &T1{})
63+
cmpT2(&T2{}, &T2{})
64+
65+
m1 := map[T1]bool{}
66+
m1[T1{}] = true
67+
m1[T1{}] = false
68+
if len(m1) != 1 {
69+
t.Fatalf("len(m1)=%d, want 1", len(m1))
70+
}
71+
m2 := map[T2]bool{}
72+
m2[T2{}] = true
73+
m2[T2{}] = false
74+
if len(m2) != 1 {
75+
t.Fatalf("len(m2)=%d, want 1", len(m2))
76+
}
77+
78+
type X2 struct {
79+
y byte
80+
z A2
81+
}
82+
var x2 X2
83+
cmpA2(&x2.z, &A2{})
84+
type X4 struct {
85+
y byte
86+
z A4
87+
}
88+
var x4 X4
89+
cmpA4(&x4.z, &A4{})
90+
type X8 struct {
91+
y byte
92+
z A8
93+
}
94+
var x8 X8
95+
cmpA8(&x8.z, &A8{})
96+
}

Diff for: src/cmd/internal/sys/arch.go

+19
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,12 @@ type Arch struct {
4040

4141
// MinLC is the minimum length of an instruction code.
4242
MinLC int
43+
44+
// Alignment is maximum alignment required by the architecture
45+
// for any (compiler-generated) load or store instruction.
46+
// Loads or stores smaller than Alignment must be naturally aligned.
47+
// Loads or stores larger than Alignment need only be Alignment-aligned.
48+
Alignment int8
4349
}
4450

4551
// InFamily reports whether a is a member of any of the specified
@@ -60,6 +66,7 @@ var Arch386 = &Arch{
6066
PtrSize: 4,
6167
RegSize: 4,
6268
MinLC: 1,
69+
Alignment: 1,
6370
}
6471

6572
var ArchAMD64 = &Arch{
@@ -69,6 +76,7 @@ var ArchAMD64 = &Arch{
6976
PtrSize: 8,
7077
RegSize: 8,
7178
MinLC: 1,
79+
Alignment: 1,
7280
}
7381

7482
var ArchARM = &Arch{
@@ -78,6 +86,7 @@ var ArchARM = &Arch{
7886
PtrSize: 4,
7987
RegSize: 4,
8088
MinLC: 4,
89+
Alignment: 4, // TODO: just for arm5?
8190
}
8291

8392
var ArchARM64 = &Arch{
@@ -87,6 +96,7 @@ var ArchARM64 = &Arch{
8796
PtrSize: 8,
8897
RegSize: 8,
8998
MinLC: 4,
99+
Alignment: 1,
90100
}
91101

92102
var ArchMIPS = &Arch{
@@ -96,6 +106,7 @@ var ArchMIPS = &Arch{
96106
PtrSize: 4,
97107
RegSize: 4,
98108
MinLC: 4,
109+
Alignment: 4,
99110
}
100111

101112
var ArchMIPSLE = &Arch{
@@ -105,6 +116,7 @@ var ArchMIPSLE = &Arch{
105116
PtrSize: 4,
106117
RegSize: 4,
107118
MinLC: 4,
119+
Alignment: 4,
108120
}
109121

110122
var ArchMIPS64 = &Arch{
@@ -114,6 +126,7 @@ var ArchMIPS64 = &Arch{
114126
PtrSize: 8,
115127
RegSize: 8,
116128
MinLC: 4,
129+
Alignment: 8,
117130
}
118131

119132
var ArchMIPS64LE = &Arch{
@@ -123,6 +136,7 @@ var ArchMIPS64LE = &Arch{
123136
PtrSize: 8,
124137
RegSize: 8,
125138
MinLC: 4,
139+
Alignment: 8,
126140
}
127141

128142
var ArchPPC64 = &Arch{
@@ -132,6 +146,7 @@ var ArchPPC64 = &Arch{
132146
PtrSize: 8,
133147
RegSize: 8,
134148
MinLC: 4,
149+
Alignment: 1,
135150
}
136151

137152
var ArchPPC64LE = &Arch{
@@ -141,6 +156,7 @@ var ArchPPC64LE = &Arch{
141156
PtrSize: 8,
142157
RegSize: 8,
143158
MinLC: 4,
159+
Alignment: 1,
144160
}
145161

146162
var ArchRISCV64 = &Arch{
@@ -150,6 +166,7 @@ var ArchRISCV64 = &Arch{
150166
PtrSize: 8,
151167
RegSize: 8,
152168
MinLC: 4,
169+
Alignment: 8, // riscv unaligned loads work, but are really slow (trap + simulated by OS)
153170
}
154171

155172
var ArchS390X = &Arch{
@@ -159,6 +176,7 @@ var ArchS390X = &Arch{
159176
PtrSize: 8,
160177
RegSize: 8,
161178
MinLC: 2,
179+
Alignment: 1,
162180
}
163181

164182
var ArchWasm = &Arch{
@@ -168,6 +186,7 @@ var ArchWasm = &Arch{
168186
PtrSize: 8,
169187
RegSize: 8,
170188
MinLC: 1,
189+
Alignment: 1,
171190
}
172191

173192
var Archs = [...]*Arch{

0 commit comments

Comments
 (0)