From 4c9340fcbf0cb10c230925e24f082478f7c458a0 Mon Sep 17 00:00:00 2001 From: Kevin Krakauer Date: Thu, 18 Mar 2021 15:38:13 -0700 Subject: Fix alignment issue with 64-bit atomics on 32 bit machines --- pkg/tcpip/BUILD | 2 ++ 1 file changed, 2 insertions(+) (limited to 'pkg/tcpip/BUILD') diff --git a/pkg/tcpip/BUILD b/pkg/tcpip/BUILD index e96ba50ae..5237eba76 100644 --- a/pkg/tcpip/BUILD +++ b/pkg/tcpip/BUILD @@ -19,6 +19,8 @@ go_template_instance( go_library( name = "tcpip", srcs = [ + "aligned.go", + "aligned_unsafe.go", "errors.go", "sock_err_list.go", "socketops.go", -- cgit v1.2.3 From 1e2ba2666135e7f28cb3c4185add5a257ad53637 Mon Sep 17 00:00:00 2001 From: Kevin Krakauer Date: Fri, 19 Mar 2021 13:44:26 -0700 Subject: Moved to atomicbitops and renamed files --- pkg/atomicbitops/BUILD | 2 + pkg/atomicbitops/aligned_32bit_unsafe.go | 80 ++++++++++++++++++++++++++++++++ pkg/atomicbitops/aligned_64bit.go | 61 ++++++++++++++++++++++++ pkg/tcpip/BUILD | 3 +- pkg/tcpip/aligned.go | 61 ------------------------ pkg/tcpip/aligned_unsafe.go | 80 -------------------------------- pkg/tcpip/socketops.go | 3 +- pkg/tcpip/stack/BUILD | 1 + pkg/tcpip/stack/stack.go | 5 +- pkg/tcpip/tcpip.go | 3 +- 10 files changed, 152 insertions(+), 147 deletions(-) create mode 100644 pkg/atomicbitops/aligned_32bit_unsafe.go create mode 100644 pkg/atomicbitops/aligned_64bit.go delete mode 100644 pkg/tcpip/aligned.go delete mode 100644 pkg/tcpip/aligned_unsafe.go (limited to 'pkg/tcpip/BUILD') diff --git a/pkg/atomicbitops/BUILD b/pkg/atomicbitops/BUILD index 1a30f6967..11072d4de 100644 --- a/pkg/atomicbitops/BUILD +++ b/pkg/atomicbitops/BUILD @@ -5,6 +5,8 @@ package(licenses = ["notice"]) go_library( name = "atomicbitops", srcs = [ + "aligned_32bit_unsafe.go", + "aligned_64bit.go", "atomicbitops.go", "atomicbitops_amd64.s", "atomicbitops_arm64.s", diff --git a/pkg/atomicbitops/aligned_32bit_unsafe.go b/pkg/atomicbitops/aligned_32bit_unsafe.go new file mode 100644 index 000000000..1d71a0641 --- /dev/null +++ b/pkg/atomicbitops/aligned_32bit_unsafe.go @@ -0,0 +1,80 @@ +// Copyright 2021 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// +build arm 386 + +package atomicbitops + +import ( + "sync/atomic" + "unsafe" +) + +// AlignedAtomicInt64 is an atomic int64 that is guaranteed to be 64-bit +// aligned, even on 32-bit systems. +// +// Per https://golang.org/pkg/sync/atomic/#pkg-note-BUG: +// +// "On ARM, 386, and 32-bit MIPS, it is the caller's responsibility to arrange +// for 64-bit alignment of 64-bit words accessed atomically. The first word in +// a variable or in an allocated struct, array, or slice can be relied upon to +// be 64-bit aligned." +type AlignedAtomicInt64 struct { + value [15]byte +} + +func (aa *AlignedAtomicInt64) ptr() *int64 { + return (*int64)(unsafe.Pointer((uintptr(unsafe.Pointer(&aa.value)) + 7) &^ 7)) +} + +func (aa *AlignedAtomicInt64) Load() int64 { + return atomic.LoadInt64(aa.ptr()) +} + +func (aa *AlignedAtomicInt64) Store(v int64) { + atomic.StoreInt64(aa.ptr(), v) +} + +func (aa *AlignedAtomicInt64) Add(v int64) int64 { + return atomic.AddInt64(aa.ptr(), v) +} + +// AlignedAtomicUint64 is an atomic uint64 that is guaranteed to be 64-bit +// aligned, even on 32-bit systems. +// +// Per https://golang.org/pkg/sync/atomic/#pkg-note-BUG: +// +// "On ARM, 386, and 32-bit MIPS, it is the caller's responsibility to arrange +// for 64-bit alignment of 64-bit words accessed atomically. The first word in +// a variable or in an allocated struct, array, or slice can be relied upon to +// be 64-bit aligned." +type AlignedAtomicUint64 struct { + value [15]byte +} + +func (aa *AlignedAtomicUint64) ptr() *uint64 { + return (*uint64)(unsafe.Pointer((uintptr(unsafe.Pointer(&aa.value)) + 7) &^ 7)) +} + +func (aa *AlignedAtomicUint64) Load() uint64 { + return atomic.LoadUint64(aa.ptr()) +} + +func (aa *AlignedAtomicUint64) Store(v uint64) { + atomic.StoreUint64(aa.ptr(), v) +} + +func (aa *AlignedAtomicUint64) Add(v uint64) uint64 { + return atomic.AddUint64(aa.ptr(), v) +} diff --git a/pkg/atomicbitops/aligned_64bit.go b/pkg/atomicbitops/aligned_64bit.go new file mode 100644 index 000000000..21043c53d --- /dev/null +++ b/pkg/atomicbitops/aligned_64bit.go @@ -0,0 +1,61 @@ +// Copyright 2021 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// +build !arm,!386 + +package atomicbitops + +import "sync/atomic" + +// AlignedAtomicInt64 is an atomic int64 that is guaranteed to be 64-bit +// aligned, even on 32-bit systems. On 64-bit machines, it's just a regular +// int64. +// +// See aligned_unsafe.go in this directory for justification. +type AlignedAtomicInt64 struct { + value int64 +} + +func (aa *AlignedAtomicInt64) Load() int64 { + return atomic.LoadInt64(&aa.value) +} + +func (aa *AlignedAtomicInt64) Store(v int64) { + atomic.StoreInt64(&aa.value, v) +} + +func (aa *AlignedAtomicInt64) Add(v int64) int64 { + return atomic.AddInt64(&aa.value, v) +} + +// AlignedAtomicUint64 is an atomic uint64 that is guaranteed to be 64-bit +// aligned, even on 32-bit systems. On 64-bit machines, it's just a regular +// uint64. +// +// See aligned_unsafe.go in this directory for justification. +type AlignedAtomicUint64 struct { + value uint64 +} + +func (aa *AlignedAtomicUint64) Load() uint64 { + return atomic.LoadUint64(&aa.value) +} + +func (aa *AlignedAtomicUint64) Store(v uint64) { + atomic.StoreUint64(&aa.value, v) +} + +func (aa *AlignedAtomicUint64) Add(v uint64) uint64 { + return atomic.AddUint64(&aa.value, v) +} diff --git a/pkg/tcpip/BUILD b/pkg/tcpip/BUILD index 5237eba76..ea46c30da 100644 --- a/pkg/tcpip/BUILD +++ b/pkg/tcpip/BUILD @@ -19,8 +19,6 @@ go_template_instance( go_library( name = "tcpip", srcs = [ - "aligned.go", - "aligned_unsafe.go", "errors.go", "sock_err_list.go", "socketops.go", @@ -31,6 +29,7 @@ go_library( ], visibility = ["//visibility:public"], deps = [ + "//pkg/atomicbitops", "//pkg/sync", "//pkg/tcpip/buffer", "//pkg/waiter", diff --git a/pkg/tcpip/aligned.go b/pkg/tcpip/aligned.go deleted file mode 100644 index ea22c101f..000000000 --- a/pkg/tcpip/aligned.go +++ /dev/null @@ -1,61 +0,0 @@ -// Copyright 2021 The gVisor Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// +build !arm,!386 - -package tcpip - -import "sync/atomic" - -// AlignedAtomicInt64 is an atomic int64 that is guaranteed to be 64-bit -// aligned, even on 32-bit systems. On 64-bit machines, it's just a regular -// int64. -// -// See aligned_unsafe.go in this directory for justification. -type AlignedAtomicInt64 struct { - value int64 -} - -func (aa *AlignedAtomicInt64) Load() int64 { - return atomic.LoadInt64(&aa.value) -} - -func (aa *AlignedAtomicInt64) Store(v int64) { - atomic.StoreInt64(&aa.value, v) -} - -func (aa *AlignedAtomicInt64) Add(v int64) int64 { - return atomic.AddInt64(&aa.value, v) -} - -// AlignedAtomicUint64 is an atomic uint64 that is guaranteed to be 64-bit -// aligned, even on 32-bit systems. On 64-bit machines, it's just a regular -// uint64. -// -// See aligned_unsafe.go in this directory for justification. -type AlignedAtomicUint64 struct { - value uint64 -} - -func (aa *AlignedAtomicUint64) Load() uint64 { - return atomic.LoadUint64(&aa.value) -} - -func (aa *AlignedAtomicUint64) Store(v uint64) { - atomic.StoreUint64(&aa.value, v) -} - -func (aa *AlignedAtomicUint64) Add(v uint64) uint64 { - return atomic.AddUint64(&aa.value, v) -} diff --git a/pkg/tcpip/aligned_unsafe.go b/pkg/tcpip/aligned_unsafe.go deleted file mode 100644 index 35e869453..000000000 --- a/pkg/tcpip/aligned_unsafe.go +++ /dev/null @@ -1,80 +0,0 @@ -// Copyright 2021 The gVisor Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// +build arm 386 - -package tcpip - -import ( - "sync/atomic" - "unsafe" -) - -// AlignedAtomicInt64 is an atomic int64 that is guaranteed to be 64-bit -// aligned, even on 32-bit systems. -// -// Per https://golang.org/pkg/sync/atomic/#pkg-note-BUG: -// -// "On ARM, 386, and 32-bit MIPS, it is the caller's responsibility to arrange -// for 64-bit alignment of 64-bit words accessed atomically. The first word in -// a variable or in an allocated struct, array, or slice can be relied upon to -// be 64-bit aligned." -type AlignedAtomicInt64 struct { - value [15]byte -} - -func (aa *AlignedAtomicInt64) ptr() *int64 { - return (*int64)(unsafe.Pointer((uintptr(unsafe.Pointer(&aa.value)) + 7) &^ 7)) -} - -func (aa *AlignedAtomicInt64) Load() int64 { - return atomic.LoadInt64(aa.ptr()) -} - -func (aa *AlignedAtomicInt64) Store(v int64) { - atomic.StoreInt64(aa.ptr(), v) -} - -func (aa *AlignedAtomicInt64) Add(v int64) int64 { - return atomic.AddInt64(aa.ptr(), v) -} - -// AlignedAtomicUint64 is an atomic uint64 that is guaranteed to be 64-bit -// aligned, even on 32-bit systems. -// -// Per https://golang.org/pkg/sync/atomic/#pkg-note-BUG: -// -// "On ARM, 386, and 32-bit MIPS, it is the caller's responsibility to arrange -// for 64-bit alignment of 64-bit words accessed atomically. The first word in -// a variable or in an allocated struct, array, or slice can be relied upon to -// be 64-bit aligned." -type AlignedAtomicUint64 struct { - value [15]byte -} - -func (aa *AlignedAtomicUint64) ptr() *uint64 { - return (*uint64)(unsafe.Pointer((uintptr(unsafe.Pointer(&aa.value)) + 7) &^ 7)) -} - -func (aa *AlignedAtomicUint64) Load() uint64 { - return atomic.LoadUint64(aa.ptr()) -} - -func (aa *AlignedAtomicUint64) Store(v uint64) { - atomic.StoreUint64(aa.ptr(), v) -} - -func (aa *AlignedAtomicUint64) Add(v uint64) uint64 { - return atomic.AddUint64(aa.ptr(), v) -} diff --git a/pkg/tcpip/socketops.go b/pkg/tcpip/socketops.go index 8158e238d..b26936b7f 100644 --- a/pkg/tcpip/socketops.go +++ b/pkg/tcpip/socketops.go @@ -18,6 +18,7 @@ import ( "math" "sync/atomic" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/sync" ) @@ -213,7 +214,7 @@ type SocketOptions struct { getSendBufferLimits GetSendBufferLimits `state:"manual"` // sendBufferSize determines the send buffer size for this socket. - sendBufferSize AlignedAtomicInt64 + sendBufferSize atomicbitops.AlignedAtomicInt64 // getReceiveBufferLimits provides the handler to get the min, default and // max size for receive buffer. It is initialized at the creation time and diff --git a/pkg/tcpip/stack/BUILD b/pkg/tcpip/stack/BUILD index 2bd6a67f5..63ab31083 100644 --- a/pkg/tcpip/stack/BUILD +++ b/pkg/tcpip/stack/BUILD @@ -73,6 +73,7 @@ go_library( ], visibility = ["//visibility:public"], deps = [ + "//pkg/atomicbitops", "//pkg/ilist", "//pkg/log", "//pkg/rand", diff --git a/pkg/tcpip/stack/stack.go b/pkg/tcpip/stack/stack.go index 436392f23..3d9e1e286 100644 --- a/pkg/tcpip/stack/stack.go +++ b/pkg/tcpip/stack/stack.go @@ -29,6 +29,7 @@ import ( "time" "golang.org/x/time/rate" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/rand" "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/pkg/tcpip" @@ -65,10 +66,10 @@ type ResumableEndpoint interface { } // uniqueIDGenerator is a default unique ID generator. -type uniqueIDGenerator uint64 +type uniqueIDGenerator atomicbitops.AlignedAtomicUint64 func (u *uniqueIDGenerator) UniqueID() uint64 { - return atomic.AddUint64((*uint64)(u), 1) + return ((*atomicbitops.AlignedAtomicUint64)(u)).Add(1) } // Stack is a networking stack, with all supported protocols, NICs, and route diff --git a/pkg/tcpip/tcpip.go b/pkg/tcpip/tcpip.go index 1bc13ef68..b39af42c4 100644 --- a/pkg/tcpip/tcpip.go +++ b/pkg/tcpip/tcpip.go @@ -40,6 +40,7 @@ import ( "sync/atomic" "time" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/pkg/waiter" ) @@ -1220,7 +1221,7 @@ type NetworkProtocolNumber uint32 // A StatCounter keeps track of a statistic. type StatCounter struct { - count AlignedAtomicUint64 + count atomicbitops.AlignedAtomicUint64 } // Increment adds one to the counter. -- cgit v1.2.3