cmd/compile: fix nilcheck for AIX
This commit adapts compile tool to create correct nilchecks for AIX.
AIX allows to load a nil pointer. Therefore, the default nilcheck
which issues a load must be replaced by a CMP instruction followed by a
store at 0x0 if the value is nil. The store will trigger a SIGSEGV as on
others OS.
The nilcheck algorithm must be adapted to do not remove nilcheck if it's
only a read. Stores are detected with v.Type.IsMemory().
Tests related to nilptr must be adapted to the previous changements.
nilptr.go cannot be used as it's because the AIX address space starts at
1<<32.
Change-Id: I9f5aaf0b7e185d736a9b119c0ed2fe4e5bd1e7af
Reviewed-on: https://go-review.googlesource.com/c/144538
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
diff --git a/src/cmd/compile/internal/gc/ssa.go b/src/cmd/compile/internal/gc/ssa.go
index 27af607..e0b4b40 100644
--- a/src/cmd/compile/internal/gc/ssa.go
+++ b/src/cmd/compile/internal/gc/ssa.go
@@ -3651,7 +3651,8 @@
break
}
closure = s.expr(fn)
- if thearch.LinkArch.Family == sys.Wasm {
+ if thearch.LinkArch.Family == sys.Wasm || objabi.GOOS == "aix" && k != callGo {
+ // On AIX, the closure needs to be verified as fn can be nil, except if it's a call go. This needs to be handled by the runtime to have the "go of nil func value" error.
// TODO(neelance): On other architectures this should be eliminated by the optimization steps
s.nilCheck(closure)
}
diff --git a/src/cmd/compile/internal/ppc64/ssa.go b/src/cmd/compile/internal/ppc64/ssa.go
index a6dd8ca..3b37c79 100644
--- a/src/cmd/compile/internal/ppc64/ssa.go
+++ b/src/cmd/compile/internal/ppc64/ssa.go
@@ -10,6 +10,7 @@
"cmd/compile/internal/types"
"cmd/internal/obj"
"cmd/internal/obj/ppc64"
+ "cmd/internal/objabi"
"math"
"strings"
)
@@ -1183,13 +1184,44 @@
p.To.Sym = v.Aux.(*obj.LSym)
case ssa.OpPPC64LoweredNilCheck:
- // Issue a load which will fault if arg is nil.
- p := s.Prog(ppc64.AMOVBZ)
- p.From.Type = obj.TYPE_MEM
- p.From.Reg = v.Args[0].Reg()
- gc.AddAux(&p.From, v)
- p.To.Type = obj.TYPE_REG
- p.To.Reg = ppc64.REGTMP
+ if objabi.GOOS == "aix" {
+ // CMP Rarg0, R0
+ // BNE 2(PC)
+ // STW R0, 0(R0)
+ // NOP (so the BNE has somewhere to land)
+
+ // CMP Rarg0, R0
+ p := s.Prog(ppc64.ACMP)
+ p.From.Type = obj.TYPE_REG
+ p.From.Reg = v.Args[0].Reg()
+ p.To.Type = obj.TYPE_REG
+ p.To.Reg = ppc64.REG_R0
+
+ // BNE 2(PC)
+ p2 := s.Prog(ppc64.ABNE)
+ p2.To.Type = obj.TYPE_BRANCH
+
+ // STW R0, 0(R0)
+ // Write at 0 is forbidden and will trigger a SIGSEGV
+ p = s.Prog(ppc64.AMOVW)
+ p.From.Type = obj.TYPE_REG
+ p.From.Reg = ppc64.REG_R0
+ p.To.Type = obj.TYPE_MEM
+ p.To.Reg = ppc64.REG_R0
+
+ // NOP (so the BNE has somewhere to land)
+ nop := s.Prog(obj.ANOP)
+ gc.Patch(p2, nop)
+
+ } else {
+ // Issue a load which will fault if arg is nil.
+ p := s.Prog(ppc64.AMOVBZ)
+ p.From.Type = obj.TYPE_MEM
+ p.From.Reg = v.Args[0].Reg()
+ gc.AddAux(&p.From, v)
+ p.To.Type = obj.TYPE_REG
+ p.To.Reg = ppc64.REGTMP
+ }
if gc.Debug_checknil != 0 && v.Pos.Line() > 1 { // v.Pos.Line()==1 in generated wrappers
gc.Warnl(v.Pos, "generated nil check")
}
diff --git a/src/cmd/compile/internal/ssa/nilcheck.go b/src/cmd/compile/internal/ssa/nilcheck.go
index fca4f0b..e0669cf 100644
--- a/src/cmd/compile/internal/ssa/nilcheck.go
+++ b/src/cmd/compile/internal/ssa/nilcheck.go
@@ -5,6 +5,7 @@
package ssa
import (
+ "cmd/internal/objabi"
"cmd/internal/src"
)
@@ -183,6 +184,9 @@
// This should agree with minLegalPointer in the runtime.
const minZeroPage = 4096
+// faultOnLoad is true if a load to an address below minZeroPage will trigger a SIGSEGV.
+var faultOnLoad = objabi.GOOS != "aix"
+
// nilcheckelim2 eliminates unnecessary nil checks.
// Runs after lowering and scheduling.
func nilcheckelim2(f *Func) {
@@ -225,12 +229,16 @@
// Find any pointers that this op is guaranteed to fault on if nil.
var ptrstore [2]*Value
ptrs := ptrstore[:0]
- if opcodeTable[v.Op].faultOnNilArg0 {
+ if opcodeTable[v.Op].faultOnNilArg0 && (faultOnLoad || v.Type.IsMemory()) {
+ // On AIX, only writing will fault.
ptrs = append(ptrs, v.Args[0])
}
- if opcodeTable[v.Op].faultOnNilArg1 {
+ if opcodeTable[v.Op].faultOnNilArg1 && (faultOnLoad || (v.Type.IsMemory() && v.Op != OpPPC64LoweredMove)) {
+ // On AIX, only writing will fault.
+ // LoweredMove is a special case because it's considered as a "mem" as it stores on arg0 but arg1 is accessed as a load and should be checked.
ptrs = append(ptrs, v.Args[1])
}
+
for _, ptr := range ptrs {
// Check to make sure the offset is small.
switch opcodeTable[v.Op].auxType {
diff --git a/test/nilptr.go b/test/nilptr.go
index 8d674a7..90f57c5 100644
--- a/test/nilptr.go
+++ b/test/nilptr.go
@@ -7,6 +7,9 @@
// Test that the implementation catches nil ptr indirection
// in a large address space.
+// +build !aix
+// Address space starts at 1<<32 on AIX, so dummy is too far.
+
package main
import "unsafe"
diff --git a/test/nilptr3.go b/test/nilptr3.go
index 6aa718e..e0f2ed9 100644
--- a/test/nilptr3.go
+++ b/test/nilptr3.go
@@ -1,6 +1,7 @@
// errorcheck -0 -d=nil
// +build !wasm
+// +build !aix
// Copyright 2013 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
@@ -192,21 +193,6 @@
_ = &x[9] // ERROR "removed[a-z ]* nil check"
}
-func f5(p *float32, q *float64, r *float32, s *float64) float64 {
- x := float64(*p) // ERROR "removed nil check"
- y := *q // ERROR "removed nil check"
- *r = 7 // ERROR "removed nil check"
- *s = 9 // ERROR "removed nil check"
- return x + y
-}
-
-type T [29]byte
-
-func f6(p, q *T) {
- x := *p // ERROR "removed nil check"
- *q = x // ERROR "removed nil check"
-}
-
func m1(m map[int][80]byte) byte {
v := m[3] // ERROR "removed nil check"
return v[5]
@@ -257,11 +243,6 @@
return t, *p // ERROR "removed nil check"
}
-// make sure to remove nil check for memory move (issue #18003)
-func f8(t *[8]int) [8]int {
- return *t // ERROR "removed nil check"
-}
-
func f9() []int {
x := new([1]int)
x[0] = 1 // ERROR "removed nil check"
diff --git a/test/nilptr3_wasm.go b/test/nilptr3_wasm.go
deleted file mode 100644
index df29cdc..0000000
--- a/test/nilptr3_wasm.go
+++ /dev/null
@@ -1,270 +0,0 @@
-// errorcheck -0 -d=nil
-
-// +build wasm
-
-// Copyright 2013 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// Test that nil checks are removed.
-// Optimization is enabled.
-
-package p
-
-type Struct struct {
- X int
- Y float64
-}
-
-type BigStruct struct {
- X int
- Y float64
- A [1 << 20]int
- Z string
-}
-
-type Empty struct {
-}
-
-type Empty1 struct {
- Empty
-}
-
-var (
- intp *int
- arrayp *[10]int
- array0p *[0]int
- bigarrayp *[1 << 26]int
- structp *Struct
- bigstructp *BigStruct
- emptyp *Empty
- empty1p *Empty1
-)
-
-func f1() {
- _ = *intp // ERROR "generated nil check"
-
- // This one should be removed but the block copy needs
- // to be turned into its own pseudo-op in order to see
- // the indirect.
- _ = *arrayp // ERROR "generated nil check"
-
- // 0-byte indirect doesn't suffice.
- // we don't registerize globals, so there are no removed.* nil checks.
- _ = *array0p // ERROR "generated nil check"
- _ = *array0p // ERROR "removed nil check"
-
- _ = *intp // ERROR "removed nil check"
- _ = *arrayp // ERROR "removed nil check"
- _ = *structp // ERROR "generated nil check"
- _ = *emptyp // ERROR "generated nil check"
- _ = *arrayp // ERROR "removed nil check"
-}
-
-func f2() {
- var (
- intp *int
- arrayp *[10]int
- array0p *[0]int
- bigarrayp *[1 << 20]int
- structp *Struct
- bigstructp *BigStruct
- emptyp *Empty
- empty1p *Empty1
- )
-
- _ = *intp // ERROR "generated nil check"
- _ = *arrayp // ERROR "generated nil check"
- _ = *array0p // ERROR "generated nil check"
- _ = *array0p // ERROR "removed.* nil check"
- _ = *intp // ERROR "removed.* nil check"
- _ = *arrayp // ERROR "removed.* nil check"
- _ = *structp // ERROR "generated nil check"
- _ = *emptyp // ERROR "generated nil check"
- _ = *arrayp // ERROR "removed.* nil check"
- _ = *bigarrayp // ERROR "generated nil check" ARM removed nil check before indirect!!
- _ = *bigstructp // ERROR "generated nil check"
- _ = *empty1p // ERROR "generated nil check"
-}
-
-func fx10k() *[10000]int
-
-var b bool
-
-func f3(x *[10000]int) {
- // Using a huge type and huge offsets so the compiler
- // does not expect the memory hardware to fault.
- _ = x[9999] // ERROR "generated nil check"
-
- for {
- if x[9999] != 0 { // ERROR "removed nil check"
- break
- }
- }
-
- x = fx10k()
- _ = x[9999] // ERROR "generated nil check"
- if b {
- _ = x[9999] // ERROR "removed.* nil check"
- } else {
- _ = x[9999] // ERROR "removed.* nil check"
- }
- _ = x[9999] // ERROR "removed nil check"
-
- x = fx10k()
- if b {
- _ = x[9999] // ERROR "generated nil check"
- } else {
- _ = x[9999] // ERROR "generated nil check"
- }
- _ = x[9999] // ERROR "generated nil check"
-
- fx10k()
- // This one is a bit redundant, if we figured out that
- // x wasn't going to change across the function call.
- // But it's a little complex to do and in practice doesn't
- // matter enough.
- _ = x[9999] // ERROR "removed nil check"
-}
-
-func f3a() {
- x := fx10k()
- y := fx10k()
- z := fx10k()
- _ = &x[9] // ERROR "generated nil check"
- y = z
- _ = &x[9] // ERROR "removed.* nil check"
- x = y
- _ = &x[9] // ERROR "generated nil check"
-}
-
-func f3b() {
- x := fx10k()
- y := fx10k()
- _ = &x[9] // ERROR "generated nil check"
- y = x
- _ = &x[9] // ERROR "removed.* nil check"
- x = y
- _ = &x[9] // ERROR "removed.* nil check"
-}
-
-func fx10() *[10]int
-
-func f4(x *[10]int) {
- // Most of these have no checks because a real memory reference follows,
- // and the offset is small enough that if x is nil, the address will still be
- // in the first unmapped page of memory.
-
- _ = x[9] // ERROR "generated nil check" // bug: would like to remove this check (but nilcheck and load are in different blocks)
-
- for {
- if x[9] != 0 { // ERROR "removed nil check"
- break
- }
- }
-
- x = fx10()
- _ = x[9] // ERROR "generated nil check" // bug would like to remove before indirect
- if b {
- _ = x[9] // ERROR "removed nil check"
- } else {
- _ = x[9] // ERROR "removed nil check"
- }
- _ = x[9] // ERROR "removed nil check"
-
- x = fx10()
- if b {
- _ = x[9] // ERROR "generated nil check" // bug would like to remove before indirect
- } else {
- _ = &x[9] // ERROR "generated nil check"
- }
- _ = x[9] // ERROR "generated nil check" // bug would like to remove before indirect
-
- fx10()
- _ = x[9] // ERROR "removed nil check"
-
- x = fx10()
- y := fx10()
- _ = &x[9] // ERROR "generated nil check"
- y = x
- _ = &x[9] // ERROR "removed[a-z ]* nil check"
- x = y
- _ = &x[9] // ERROR "removed[a-z ]* nil check"
-}
-
-func f5(p *float32, q *float64, r *float32, s *float64) float64 {
- x := float64(*p) // ERROR "generated nil check"
- y := *q // ERROR "generated nil check"
- *r = 7 // ERROR "generated nil check"
- *s = 9 // ERROR "generated nil check"
- return x + y
-}
-
-type T [29]byte
-
-func f6(p, q *T) {
- x := *p // ERROR "generated nil check"
- *q = x // ERROR "generated nil check"
-}
-
-func m1(m map[int][80]byte) byte {
- v := m[3] // ERROR "removed nil check"
- return v[5]
-}
-func m2(m map[int][800]byte) byte {
- v := m[3] // ERROR "removed nil check"
- return v[5]
-}
-func m3(m map[int][80]byte) (byte, bool) {
- v, ok := m[3] // ERROR "removed nil check"
- return v[5], ok
-}
-func m4(m map[int][800]byte) (byte, bool) {
- v, ok := m[3] // ERROR "removed nil check"
- return v[5], ok
-}
-func p1() byte {
- p := new([100]byte)
- return p[5] // ERROR "removed nil check"
-}
-
-// make sure not to do nil check for access of PAUTOHEAP
-//go:noinline
-func (p *Struct) m() {}
-func c1() {
- var x Struct
- func() { x.m() }() // ERROR "removed nil check"
-}
-
-type SS struct {
- x byte
-}
-
-type TT struct {
- SS
-}
-
-func f(t *TT) *byte {
- // See issue 17242.
- s := &t.SS // ERROR "generated nil check"
- return &s.x // ERROR "removed nil check"
-}
-
-// make sure not to do nil check for newobject
-func f7() (*Struct, float64) {
- t := new(Struct)
- p := &t.Y // ERROR "removed nil check"
- return t, *p // ERROR "removed nil check"
-}
-
-// make sure to remove nil check for memory move (issue #18003)
-func f8(t *[8]int) [8]int {
- return *t // ERROR "generated nil check"
-}
-
-func f9() []int {
- x := new([1]int)
- x[0] = 1 // ERROR "removed nil check"
- y := x[:] // ERROR "removed nil check"
- return y
-}
diff --git a/test/nilptr5.go b/test/nilptr5.go
new file mode 100644
index 0000000..2c48c0b
--- /dev/null
+++ b/test/nilptr5.go
@@ -0,0 +1,33 @@
+// errorcheck -0 -d=nil
+
+// +build !wasm
+// +build !aix
+
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Test that nil checks are removed.
+// Optimization is enabled.
+
+package p
+
+func f5(p *float32, q *float64, r *float32, s *float64) float64 {
+ x := float64(*p) // ERROR "removed nil check"
+ y := *q // ERROR "removed nil check"
+ *r = 7 // ERROR "removed nil check"
+ *s = 9 // ERROR "removed nil check"
+ return x + y
+}
+
+type T [29]byte
+
+func f6(p, q *T) {
+ x := *p // ERROR "removed nil check"
+ *q = x // ERROR "removed nil check"
+}
+
+// make sure to remove nil check for memory move (issue #18003)
+func f8(t *[8]int) [8]int {
+ return *t // ERROR "removed nil check"
+}
diff --git a/test/nilptr5_aix.go b/test/nilptr5_aix.go
new file mode 100644
index 0000000..ff69005
--- /dev/null
+++ b/test/nilptr5_aix.go
@@ -0,0 +1,32 @@
+// errorcheck -0 -d=nil
+
+// +build aix
+
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Test that nil checks are removed.
+// Optimization is enabled.
+
+package p
+
+func f5(p *float32, q *float64, r *float32, s *float64) float64 {
+ x := float64(*p) // ERROR "generated nil check"
+ y := *q // ERROR "generated nil check"
+ *r = 7 // ERROR "removed nil check"
+ *s = 9 // ERROR "removed nil check"
+ return x + y
+}
+
+type T [29]byte
+
+func f6(p, q *T) {
+ x := *p // ERROR "generated nil check"
+ *q = x // ERROR "generated nil check"
+}
+
+// make sure to remove nil check for memory move (issue #18003)
+func f8(t *[8]int) [8]int {
+ return *t // ERROR "generated nil check"
+}
diff --git a/test/nilptr5_wasm.go b/test/nilptr5_wasm.go
new file mode 100644
index 0000000..6ef8a02
--- /dev/null
+++ b/test/nilptr5_wasm.go
@@ -0,0 +1,32 @@
+// errorcheck -0 -d=nil
+
+// +build wasm
+
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Test that nil checks are removed.
+// Optimization is enabled.
+
+package p
+
+func f5(p *float32, q *float64, r *float32, s *float64) float64 {
+ x := float64(*p) // ERROR "generated nil check"
+ y := *q // ERROR "generated nil check"
+ *r = 7 // ERROR "generated nil check"
+ *s = 9 // ERROR "generated nil check"
+ return x + y
+}
+
+type T [29]byte
+
+func f6(p, q *T) {
+ x := *p // ERROR "generated nil check"
+ *q = x // ERROR "generated nil check"
+}
+
+// make sure to remove nil check for memory move (issue #18003)
+func f8(t *[8]int) [8]int {
+ return *t // ERROR "generated nil check"
+}
diff --git a/test/nilptr_aix.go b/test/nilptr_aix.go
new file mode 100644
index 0000000..ea5fcc3
--- /dev/null
+++ b/test/nilptr_aix.go
@@ -0,0 +1,185 @@
+// run
+
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Test that the implementation catches nil ptr indirection
+// in a large address space.
+
+// +build aix
+
+package main
+
+import "unsafe"
+
+// Having a big address space means that indexing
+// at a 1G + 256 MB offset from a nil pointer might not
+// cause a memory access fault. This test checks
+// that Go is doing the correct explicit checks to catch
+// these nil pointer accesses, not just relying on the hardware.
+// The reason of the 1G offset is because AIX addresses start after 1G.
+var dummy [256 << 20]byte // give us a big address space
+
+func main() {
+ // the test only tests what we intend to test
+ // if dummy starts in the first 256 MB of memory.
+ // otherwise there might not be anything mapped
+ // at the address that might be accidentally
+ // dereferenced below.
+ if uintptr(unsafe.Pointer(&dummy)) < 1<<32 {
+ panic("dummy not far enough")
+ }
+
+ shouldPanic(p1)
+ shouldPanic(p2)
+ shouldPanic(p3)
+ shouldPanic(p4)
+ shouldPanic(p5)
+ shouldPanic(p6)
+ shouldPanic(p7)
+ shouldPanic(p8)
+ shouldPanic(p9)
+ shouldPanic(p10)
+ shouldPanic(p11)
+ shouldPanic(p12)
+ shouldPanic(p13)
+ shouldPanic(p14)
+ shouldPanic(p15)
+ shouldPanic(p16)
+}
+
+func shouldPanic(f func()) {
+ defer func() {
+ if recover() == nil {
+ panic("memory reference did not panic")
+ }
+ }()
+ f()
+}
+
+func p1() {
+ // Array index.
+ var p *[1 << 33]byte = nil
+ println(p[1<<32+256<<20]) // very likely to be inside dummy, but should panic
+}
+
+var xb byte
+
+func p2() {
+ var p *[1 << 33]byte = nil
+ xb = 123
+
+ // Array index.
+ println(p[uintptr(unsafe.Pointer(&xb))]) // should panic
+}
+
+func p3() {
+ // Array to slice.
+ var p *[1 << 33]byte = nil
+ var x []byte = p[0:] // should panic
+ _ = x
+}
+
+var q *[1 << 33]byte
+
+func p4() {
+ // Array to slice.
+ var x []byte
+ var y = &x
+ *y = q[0:] // should crash (uses arraytoslice runtime routine)
+}
+
+func fb([]byte) {
+ panic("unreachable")
+}
+
+func p5() {
+ // Array to slice.
+ var p *[1 << 33]byte = nil
+ fb(p[0:]) // should crash
+}
+
+func p6() {
+ // Array to slice.
+ var p *[1 << 33]byte = nil
+ var _ []byte = p[10 : len(p)-10] // should crash
+}
+
+type T struct {
+ x [1<<32 + 256<<20]byte
+ i int
+}
+
+func f() *T {
+ return nil
+}
+
+var y *T
+var x = &y
+
+func p7() {
+ // Struct field access with large offset.
+ println(f().i) // should crash
+}
+
+func p8() {
+ // Struct field access with large offset.
+ println((*x).i) // should crash
+}
+
+func p9() {
+ // Struct field access with large offset.
+ var t *T
+ println(&t.i) // should crash
+}
+
+func p10() {
+ // Struct field access with large offset.
+ var t *T
+ println(t.i) // should crash
+}
+
+type T1 struct {
+ T
+}
+
+type T2 struct {
+ *T1
+}
+
+func p11() {
+ t := &T2{}
+ p := &t.i
+ println(*p)
+}
+
+// ADDR(DOT(IND(p))) needs a check also
+func p12() {
+ var p *T = nil
+ println(*(&((*p).i)))
+}
+
+// Tests suggested in golang.org/issue/6080.
+
+func p13() {
+ var x *[10]int
+ y := x[:]
+ _ = y
+}
+
+func p14() {
+ println((*[1]int)(nil)[:])
+}
+
+func p15() {
+ for i := range (*[1]int)(nil)[:] {
+ _ = i
+ }
+}
+
+func p16() {
+ for i, v := range (*[1]int)(nil)[:] {
+ _ = i + v
+ }
+}
