Code style and refactoring changes (#7)

* Code style and refactoring changes * fmt changes * add go1.10 in the test suite * adding go1.9 in the test matrix * adding go1.8 in the test matrix * test matrix changes * adding go1.7 in the test matrix * try to add macos to the test matrix * go1.6 in test matrix * remove go1.6 from the test matrix due an error in go get for macos
undefinedlabs · Jun 12, 2020 · 608c12d · 608c12d
1 parent bfd2e1d
commit 608c12d
Show file tree

Hide file tree

Showing 8 changed files with 66 additions and 37 deletions.
diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml
@@ -5,8 +5,8 @@ jobs:
     name: Test
     strategy:
       matrix:
-        go_version: [1.11, 1.12, 1.13, 1.14]
-        os: [ubuntu-latest, windows-latest]
+        go_version: [1.7, 1.8, 1.9, "1.10", 1.11, 1.12, 1.13, 1.14, 1.15]
+        os: [ubuntu-latest, windows-latest, macos-latest]
     runs-on: ${{ matrix.os }}
     steps:
       - name: Set up Go ${{ matrix.go_version }}

diff --git a/patcher.go b/patcher.go
@@ -34,13 +34,11 @@ var (
 func PatchMethod(target, redirection interface{}) (*Patch, error) {
 	tValue := getValueFrom(target)
 	rValue := getValueFrom(redirection)
-	err := isPatchable(&tValue, &rValue)
-	if err != nil {
+	if err := isPatchable(&tValue, &rValue); err != nil {
 		return nil, err
 	}
 	patch := &Patch{target: &tValue, redirection: &rValue}
-	err = applyPatch(patch)
-	if err != nil {
+	if err := applyPatch(patch); err != nil {
 		return nil, err
 	}
 	return patch, nil
@@ -54,37 +52,32 @@ func PatchInstanceMethodByName(target reflect.Type, methodName string, redirecti
 	if !ok {
 		return nil, errors.New(fmt.Sprintf("Method '%v' not found", methodName))
 	}
-	return PatchMethodByReflect(method, redirection)
+	return PatchMethodByReflect(method.Func, redirection)
 }
-func PatchMethodByReflect(target reflect.Method, redirection interface{}) (*Patch, error) {
-	tValue := &target.Func
+func PatchMethodByReflect(target reflect.Value, redirection interface{}) (*Patch, error) {
+	tValue := &target
 	rValue := getValueFrom(redirection)
-	err := isPatchable(tValue, &rValue)
-	if err != nil {
+	if err := isPatchable(tValue, &rValue); err != nil {
 		return nil, err
 	}
 	patch := &Patch{target: tValue, redirection: &rValue}
-	err = applyPatch(patch)
-	if err != nil {
+	if err := applyPatch(patch); err != nil {
 		return nil, err
 	}
 	return patch, nil
 }
-func PatchMethodWithMakeFunc(target reflect.Method, fn func(args []reflect.Value) (results []reflect.Value)) (*Patch, error) {
-	rValue := reflect.MakeFunc(target.Type, fn)
-	return PatchMethodByReflect(target, rValue)
+func PatchMethodWithMakeFunc(target reflect.Value, fn func(args []reflect.Value) (results []reflect.Value)) (*Patch, error) {
+	return PatchMethodByReflect(target, reflect.MakeFunc(target.Type(), fn))
 }
 
 func (p *Patch) Patch() error {
 	if p == nil {
 		return errors.New("patch is nil")
 	}
-	err := isPatchable(p.target, p.redirection)
-	if err != nil {
+	if err := isPatchable(p.target, p.redirection); err != nil {
 		return err
 	}
-	err = applyPatch(p)
-	if err != nil {
+	if err := applyPatch(p); err != nil {
 		return err
 	}
 	return nil
@@ -103,7 +96,7 @@ func isPatchable(target, redirection *reflect.Value) error {
 	if target.Type() != redirection.Type() {
 		return errors.New(fmt.Sprintf("the target and/or redirection doesn't have the same type: %s != %s", target.Type(), redirection.Type()))
 	}
-	if _, ok := patches[getSafePointer(target)]; ok {
+	if _, ok := patches[getSafeCodePointer(target)]; ok {
 		return errors.New("the target is already patched")
 	}
 	return nil
@@ -112,7 +105,7 @@ func isPatchable(target, redirection *reflect.Value) error {
 func applyPatch(patch *Patch) error {
 	patchLock.Lock()
 	defer patchLock.Unlock()
-	tPointer := getSafePointer(patch.target)
+	tPointer := getSafeCodePointer(patch.target)
 	rPointer := getInternalPtrFromValue(patch.redirection)
 	rPointerJumpBytes, err := getJumpFuncBytes(rPointer)
 	if err != nil {
@@ -121,8 +114,7 @@ func applyPatch(patch *Patch) error {
 	tPointerBytes := getMemorySliceFromPointer(tPointer, len(rPointerJumpBytes))
 	targetBytes := make([]byte, len(tPointerBytes))
 	copy(targetBytes, tPointerBytes)
-	err = copyDataToPtr(tPointer, rPointerJumpBytes)
-	if err != nil {
+	if err := copyDataToPtr(tPointer, rPointerJumpBytes); err != nil {
 		return err
 	}
 	patch.targetBytes = targetBytes
@@ -136,7 +128,7 @@ func applyUnpatch(patch *Patch) error {
 	if patch.targetBytes == nil || len(patch.targetBytes) == 0 {
 		return errors.New("the target is not patched")
 	}
-	tPointer := getSafePointer(patch.target)
+	tPointer := getSafeCodePointer(patch.target)
 	if _, ok := patches[tPointer]; !ok {
 		return errors.New("the target is not patched")
 	}
@@ -164,7 +156,7 @@ func getMemorySliceFromPointer(p unsafe.Pointer, length int) []byte {
 	}))
 }
 
-func getSafePointer(value *reflect.Value) unsafe.Pointer {
+func getSafeCodePointer(value *reflect.Value) unsafe.Pointer {
 	p := getInternalPtrFromValue(value)
 	if p != nil {
 		p = *(*unsafe.Pointer)(p)

diff --git a/patcher_test.go b/patcher_test.go
@@ -44,6 +44,44 @@ func TestPatcher(t *testing.T) {
 	}
 }
 
+func TestPatcherUsingReflect(t *testing.T) {
+	patch, err := PatchMethodByReflect(reflect.ValueOf(methodA), methodB)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if methodA() != 2 {
+		t.Fatal("The patch did not work")
+	}
+
+	err = patch.Unpatch()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if methodA() != 1 {
+		t.Fatal("The unpatch did not work")
+	}
+}
+
+func TestPatcherUsingMakeFunc(t *testing.T) {
+	patch, err := PatchMethodWithMakeFunc(reflect.ValueOf(methodA), func(args []reflect.Value) (results []reflect.Value) {
+		return []reflect.Value{reflect.ValueOf(42)}
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if methodA() != 42 {
+		t.Fatal("The patch did not work")
+	}
+
+	err = patch.Unpatch()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if methodA() != 1 {
+		t.Fatal("The unpatch did not work")
+	}
+}
+
 func TestInstancePatcher(t *testing.T) {
 	mStruct := myStruct{}
 

diff --git a/patcher_unix.go b/patcher_unix.go
@@ -24,8 +24,7 @@ func getMemorySliceFromUintptr(p uintptr, length int) []byte {
 func callMProtect(addr unsafe.Pointer, length int, prot int) error {
 	for p := uintptr(addr) & ^(uintptr(pageSize - 1)); p < uintptr(addr)+uintptr(length); p += uintptr(pageSize) {
 		page := getMemorySliceFromUintptr(p, pageSize)
-		err := syscall.Mprotect(page, prot)
-		if err != nil {
+		if err := syscall.Mprotect(page, prot); err != nil {
 			return err
 		}
 	}
@@ -35,13 +34,11 @@ func callMProtect(addr unsafe.Pointer, length int, prot int) error {
 func copyDataToPtr(ptr unsafe.Pointer, data []byte) error {
 	dataLength := len(data)
 	ptrByteSlice := getMemorySliceFromPointer(ptr, len(data))
-	err := callMProtect(ptr, dataLength, writeAccess)
-	if err != nil {
+	if err := callMProtect(ptr, dataLength, writeAccess); err != nil {
 		return err
 	}
 	copy(ptrByteSlice, data[:])
-	err = callMProtect(ptr, dataLength, readAccess)
-	if err != nil {
+	if err := callMProtect(ptr, dataLength, readAccess); err != nil {
 		return err
 	}
 	return nil

diff --git a/patcher_unsupported.go b/patcher_unsupported.go
@@ -13,5 +13,5 @@ import (
 // Gets the jump function rewrite bytes
 //go:nosplit
 func getJumpFuncBytes(to unsafe.Pointer) ([]byte, error) {
-	return nil, errors.New(fmt.Sprintf("Unsupported architecture: %s", runtime.GOARCH))
+	return nil, errors.New(fmt.Sprintf("unsupported architecture: %s", runtime.GOARCH))
 }
diff --git a/patcher_windows.go b/patcher_windows.go
@@ -23,13 +23,11 @@ func copyDataToPtr(ptr unsafe.Pointer, data []byte) error {
 	var oldPerms, tmp uint32
 	dataLength := len(data)
 	ptrByteSlice := getMemorySliceFromPointer(ptr, len(data))
-	err := callVirtualProtect(ptr, dataLength, pageExecuteReadAndWrite, unsafe.Pointer(&oldPerms))
-	if err != nil {
+	if err := callVirtualProtect(ptr, dataLength, pageExecuteReadAndWrite, unsafe.Pointer(&oldPerms)); err != nil {
 		return err
 	}
 	copy(ptrByteSlice, data[:])
-	err = callVirtualProtect(ptr, dataLength, oldPerms, unsafe.Pointer(&tmp))
-	if err != nil {
+	if err := callVirtualProtect(ptr, dataLength, oldPerms, unsafe.Pointer(&tmp)); err != nil {
 		return err
 	}
 	return nil

diff --git a/patcher_x32.go b/patcher_x32.go
@@ -4,6 +4,8 @@ package mpatch
 
 import "unsafe"
 
+const jumpLength = 7
+
 // Gets the jump function rewrite bytes
 //go:nosplit
 func getJumpFuncBytes(to unsafe.Pointer) ([]byte, error) {

diff --git a/patcher_x64.go b/patcher_x64.go
@@ -4,6 +4,8 @@ package mpatch
 
 import "unsafe"
 
+const jumpLength = 12
+
 // Gets the jump function rewrite bytes
 //go:nosplit
 func getJumpFuncBytes(to unsafe.Pointer) ([]byte, error) {