Skip to content

Commit

Permalink
Merge pull request #49 from bytedance/fix_race
Browse files Browse the repository at this point in the history
fix: support -race for generic function mocks
  • Loading branch information
Sychorius authored Jan 17, 2024
2 parents 1df46d9 + 065917c commit a60edcc
Show file tree
Hide file tree
Showing 6 changed files with 183 additions and 79 deletions.
34 changes: 28 additions & 6 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,12 @@ jobs:
unit-benchmark-test:
strategy:
matrix:
go:
[
go: [
"1.13",
"1.14",
"1.15",
"1.16",
"1.17",
# "1.14",
# "1.15",
# "1.16",
# "1.17",
"1.18",
"1.19",
"1.20",
Expand All @@ -26,6 +25,29 @@ jobs:
- os: Windows
arch: ARM64
runs-on: ["${{ matrix.os }}", "${{ matrix.arch }}"]
steps:
- uses: actions/checkout@v3
- name: Set up Go
uses: actions/setup-go@v3
with:
go-version: ${{ matrix.go }}
- name: Unit Test
run: MOCKEY_DEBUG=true go test -gcflags="all=-l -N" -race -covermode=atomic -coverprofile=coverage.out ./...
- name: Benchmark
run: go test -gcflags="all=-l -N" -bench=. -benchmem -run=none ./...

unit-benchmark-test-no-race: # some go version -race has fatal issues
strategy:
matrix:
go: ["1.14", "1.15", "1.16", "1.17"]
os: [linux] # should be [ macOS, linux, windows ], but currently we don't have macOS and windows runners
arch: [X64, ARM64]
exclude:
- os: Linux
arch: ARM64
- os: Windows
arch: ARM64
runs-on: ["${{ matrix.os }}", "${{ matrix.arch }}"]
steps:
- uses: actions/checkout@v3
- name: Set up Go
Expand Down
38 changes: 3 additions & 35 deletions internal/monkey/inst/disasm_amd64.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,6 @@ import (
"golang.org/x/arch/x86/x86asm"
)

//go:linkname duffcopy runtime.duffcopy
func duffcopy()

//go:linkname duffzero runtime.duffzero
func duffzero()

var duffcopyStart, duffcopyEnd, duffzeroStart, duffzeroEnd uintptr

func init() {
duffcopyStart, duffcopyEnd = calcFnAddrRange("duffcopy", duffcopy)
duffzeroStart, duffzeroEnd = calcFnAddrRange("duffzero", duffzero)
}

func calcFnAddrRange(name string, fn func()) (uintptr, uintptr) {
v := reflect.ValueOf(fn)
var start, end uintptr
Expand Down Expand Up @@ -107,28 +94,9 @@ func GetGenericJumpAddr(addr uintptr, maxScan uint64) uintptr {
if inst.Op == x86asm.CALL {
rel := int32(inst.Args[0].(x86asm.Rel))
fnAddr := calcAddr(uintptr(unsafe.Pointer(&code[0]))+uintptr(pos+uint64(inst.Len)), rel)

/*
When function argument size is too big, golang will use duff-copy
to get better performance. Thus we will see more call-instruction
in asm code.
For example, assume struct type like this:
```
type Large15 struct _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _ string}
```
when we use `Large15` as generic function's argument or return types,
the wrapper `function[Large15]` will call `duffcopy` and `duffzero`
before passing arguments and after receiving returns.
Notice that `duff` functions are very special, they are always called
in the middle of function body(not at beginning). So we should check
the `call` instruction's target address with a range.
*/

isDuffCopy := (fnAddr >= duffcopyStart && fnAddr <= duffcopyEnd)
isDuffZero := (fnAddr >= duffzeroStart && fnAddr <= duffzeroEnd)
tool.DebugPrintf("found: CALL, raw is: %x, rel: %v isDuffCopy: %v, isDuffZero: %v fnAddr: %v\n", inst.String(), rel, isDuffCopy, isDuffZero, fnAddr)
if !isDuffCopy && !isDuffZero {
isExtraCall, extraName := isGenericProxyCallExtra(fnAddr)
tool.DebugPrintf("found CALL, raw is: %x, rel: %v, raw is: %x, fnAddr: %v, isExtraCall: %v, extraName: %v\n", inst.String(), rel, fnAddr, isExtraCall, extraName)
if !isExtraCall {
allAddrs = append(allAddrs, fnAddr)
}
}
Expand Down
40 changes: 4 additions & 36 deletions internal/monkey/inst/disasm_arm64.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,6 @@ import (
"golang.org/x/arch/arm64/arm64asm"
)

//go:linkname duffcopy runtime.duffcopy
func duffcopy()

//go:linkname duffzero runtime.duffzero
func duffzero()

var duffcopyStart, duffcopyEnd, duffzeroStart, duffzeroEnd uintptr

func init() {
duffcopyStart, duffcopyEnd = calcFnAddrRange("duffcopy", duffcopy)
duffzeroStart, duffzeroEnd = calcFnAddrRange("duffzero", duffzero)
}

func calcFnAddrRange(name string, fn func()) (uintptr, uintptr) {
v := reflect.ValueOf(fn)
var start, end uintptr
Expand All @@ -57,7 +44,7 @@ func calcFnAddrRange(name string, fn func()) (uintptr, uintptr) {

if inst.Op == arm64asm.RET {
end = start + uintptr(pos)
tool.DebugPrintf("init: duffcopy(%v,%v)\n", name, start, end)
tool.DebugPrintf("init: %v(%v,%v)\n", name, start, end)
return start, end
}

Expand Down Expand Up @@ -94,28 +81,9 @@ func GetGenericJumpAddr(addr uintptr, maxScan uint64) uintptr {

if inst.Op == arm64asm.BL {
fnAddr := calcAddr(uintptr(unsafe.Pointer(&code[0]))+uintptr(pos), inst.Enc)

/*
When function argument size is too big, golang will use duff-copy
to get better performance. Thus we will see more call-instruction
in asm code.
For example, assume struct type like this:
```
type Large15 struct _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _ string}
```
when we use `Large15` as generic function's argument or return types,
the wrapper `function[Large15]` will call `duffcopy` and `duffzero`
before passing arguments and after receiving returns.
Notice that `duff` functions are very special, they are always called
in the middle of function body(not at beginning). So we should check
the `call` instruction's target address with a range.
*/

isDuffCopy := (fnAddr >= duffcopyStart && fnAddr <= duffcopyEnd)
isDuffZero := (fnAddr >= duffzeroStart && fnAddr <= duffzeroEnd)
tool.DebugPrintf("found: BL, raw is: %x, isDuffCopy: %v, isDuffZero: %v fnAddr: %v\n", inst.String(), isDuffCopy, isDuffZero, fnAddr)
if !isDuffCopy && !isDuffZero {
isExtraCall, extraName := isGenericProxyCallExtra(fnAddr)
tool.DebugPrintf("found BL, raw is: %x, fnAddr: %v, isExtraCall: %v, extraName: %v\n", inst.String(), fnAddr, isExtraCall, extraName)
if !isExtraCall {
allAddrs = append(allAddrs, fnAddr)
}
}
Expand Down
75 changes: 75 additions & 0 deletions internal/monkey/inst/generic_extra.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
* Copyright 2022 ByteDance Inc.
*
* 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.
*/

package inst

import (
_ "unsafe"
)

//go:linkname duffcopy runtime.duffcopy
func duffcopy()

//go:linkname duffzero runtime.duffzero
func duffzero()

var duffcopyStart, duffcopyEnd, duffzeroStart, duffzeroEnd uintptr

func init() {
duffcopyStart, duffcopyEnd = calcFnAddrRange("duffcopy", duffcopy)
duffzeroStart, duffzeroEnd = calcFnAddrRange("duffzero", duffzero)
}

// proxyCallRace exclude functions used for race check in unit test
//
// If we use '-race' in test command, golang may use 'racefuncenter' and 'racefuncexit' in generic
// functions's proxy, which is toxic for us to find the original generic function implementation.
//
// So we need to exclude them. We simply exclude most of race functions defined in runtime.
var proxyCallRace = map[uintptr]string{}

// isGenericProxyCallExtra checks if generic function's proxy called an extra internal function
// We check duffcopy/duffzero for passing huge params/returns and race-functions for -race option
func isGenericProxyCallExtra(addr uintptr) (bool, string) {
/*
When function argument size is too big, golang will use duff-copy
to get better performance. Thus we will see more call-instruction
in asm code.
For example, assume struct type like this:
```
type Large15 struct _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _ string}
```
when we use `Large15` as generic function's argument or return types,
the wrapper `function[Large15]` will call `duffcopy` and `duffzero`
before passing arguments and after receiving returns.
Notice that `duff` functions are very special, they are always called
in the middle of function body(not at beginning). So we should check
the `call` instruction's target address with a range.
*/
if addr >= duffcopyStart && addr <= duffcopyEnd {
return true, "diffcopy"
}

if addr >= duffzeroStart && addr <= duffzeroEnd {
return true, "duffzero"
}

if name, ok := proxyCallRace[addr]; ok {
return true, name
}
return false, ""
}
71 changes: 71 additions & 0 deletions internal/monkey/inst/generic_extra_race.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
//go:build race
// +build race

/*
* Copyright 2022 ByteDance Inc.
*
* 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.
*/

package inst

import (
"reflect"
_ "unsafe"
)

//go:linkname racefuncenter runtime.racefuncenter
func racefuncenter(callpc uintptr)

// not implemented & never used
////go:linkname racefuncenterfp runtime.racefuncenterfp
// func racefuncenterfp(fp uintptr)

//go:linkname racefuncexit runtime.racefuncexit
func racefuncexit()

//go:linkname raceread runtime.raceread
func raceread(addr uintptr)

//go:linkname racewrite runtime.racewrite
func racewrite(addr uintptr)

//go:linkname racereadrange runtime.racereadrange
func racereadrange(addr, size uintptr)

//go:linkname racewriterange runtime.racewriterange
func racewriterange(addr, size uintptr)

//go:linkname racereadrangepc1 runtime.racereadrangepc1
func racereadrangepc1(addr, size, pc uintptr)

//go:linkname racewriterangepc1 runtime.racewriterangepc1
func racewriterangepc1(addr, size, pc uintptr)

//go:linkname racecallbackthunk runtime.racecallbackthunk
func racecallbackthunk(uintptr)

func init() {
proxyCallRace = map[uintptr]string{
reflect.ValueOf(racefuncenter).Pointer(): "racefuncenter",
// reflect.ValueOf(racefuncenterfp).Pointer(): "racefuncenterfp", // not implemented & never used
reflect.ValueOf(racefuncexit).Pointer(): "racefuncexit",
reflect.ValueOf(raceread).Pointer(): "raceread",
reflect.ValueOf(racewrite).Pointer(): "racewrite",
reflect.ValueOf(racereadrange).Pointer(): "racereadrange",
reflect.ValueOf(racewriterange).Pointer(): "racewriterange",
reflect.ValueOf(racereadrangepc1).Pointer(): "racereadrangepc1",
reflect.ValueOf(racewriterangepc1).Pointer(): "racewriterangepc1",
reflect.ValueOf(racecallbackthunk).Pointer(): "racecallbackthunk",
}
}
4 changes: 2 additions & 2 deletions mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -421,11 +421,11 @@ func (mocker *Mocker) mock() {
}

func (mocker *Mocker) Times() int {
return int(mocker.times)
return int(atomic.LoadInt64(&mocker.times))
}

func (mocker *Mocker) MockTimes() int {
return int(mocker.mockTimes)
return int(atomic.LoadInt64(&mocker.mockTimes))
}

func (mocker *Mocker) key() uintptr {
Expand Down

0 comments on commit a60edcc

Please sign in to comment.