random magic value using modified cmd/link alternative

README.md

@@ -140,7 +140,7 @@ to document the current shortcomings of this tool.
    ```
 
 * Go plugins are not currently supported; see [#87](https://github.com/burrowers/garble/issues/87).
-* Currently, Garble depends on `git` for applying non-strict patches. After adding support for non-strict patches to go-gitdiff (see [#30](https://github.com/bluekeyes/go-gitdiff/issues/30)) or alternative libraries, `git` dependency will be removed. 
+* Garble requires `git` to patch the linker. That can be avoided once go-gitdiff supports [non-strict patches](https://github.com/bluekeyes/go-gitdiff/issues/30). 
 
 ### Contributing
 

internal/linker/linker.go

@@ -10,15 +10,16 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
-	"github.com/bluekeyes/go-gitdiff/gitdiff"
-	"github.com/rogpeppe/go-internal/lockedfile"
 	"io"
 	"io/fs"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"runtime"
 	"strings"
+
+	"github.com/bluekeyes/go-gitdiff/gitdiff"
+	"github.com/rogpeppe/go-internal/lockedfile"
 )
 
 const (
@@ -76,8 +77,8 @@ func loadLinkerPatches() (string, map[string]string, error) {
 
 // TODO(pagran): Remove git dependency in future
 // more information in README.md
-func applyPatch(workingDirectory, patch string) error {
-	cmd := exec.Command("git", "-C", workingDirectory, "apply")
+func applyPatch(workingDir, patch string) error {
+	cmd := exec.Command("git", "-C", workingDir, "apply")
 	cmd.Stdin = strings.NewReader(patch)
 	return cmd.Run()
 }
@@ -110,18 +111,18 @@ func fileExists(path string) bool {
 	return !stat.IsDir()
 }
 
-func applyPatches(srcDirectory, workingDirectory string, patches map[string]string) (map[string]string, error) {
+func applyPatches(srcDir, workingDir string, patches map[string]string) (map[string]string, error) {
 	mod := make(map[string]string)
 	for fileName, patch := range patches {
-		oldPath := filepath.Join(srcDirectory, fileName)
-		newPath := filepath.Join(workingDirectory, fileName)
+		oldPath := filepath.Join(srcDir, fileName)
+		newPath := filepath.Join(workingDir, fileName)
 		mod[oldPath] = newPath
 
 		if err := copyFile(oldPath, newPath); err != nil {
 			return nil, err
 		}
 
-		if err := applyPatch(workingDirectory, patch); err != nil {
+		if err := applyPatch(workingDir, patch); err != nil {
 			return nil, fmt.Errorf("apply patch for %s failed: %v", fileName, err)
 		}
 	}
@@ -175,12 +176,12 @@ func writeVersion(linkerPath, goVersion, patchesVer string) error {
 	return os.WriteFile(versionPath, []byte(getCurrentVersion(goVersion, patchesVer)), 0o777)
 }
 
-func compileLinker(workingDirectory string, overlay map[string]string, outputLinkPath string) error {
+func buildLinker(workingDir string, overlay map[string]string, outputLinkPath string) error {
 	file, err := json.Marshal(&struct{ Replace map[string]string }{overlay})
 	if err != nil {
 		return err
 	}
-	overlayPath := filepath.Join(workingDirectory, "overlay.json")
+	overlayPath := filepath.Join(workingDir, "overlay.json")
 	if err := os.WriteFile(overlayPath, file, 0o777); err != nil {
 		return err
 	}
@@ -197,44 +198,53 @@ func compileLinker(workingDirectory string, overlay map[string]string, outputLin
 	return nil
 }
 
-func PatchLinker(goRoot, goVersion, goExe, tempDirectory string) (string, error) {
+func PatchLinker(goRoot, goVersion, goExe, tempDir string) (string, func(), error) {
 	patchesVer, patches, err := loadLinkerPatches()
 	if err != nil {
 		panic(fmt.Errorf("cannot retrieve linker patches: %v", err))
 	}
 
 	outputLinkPath, err := cachePath(goExe)
 	if err != nil {
-		return "", err
+		return "", nil, err
 	}
 
 	mutex := lockedfile.MutexAt(outputLinkPath + ".lock")
 	unlock, err := mutex.Lock()
 	if err != nil {
-		return "", err
+		return "", nil, err
 	}
-	defer unlock()
+
+	// If build is successful, mutex unlocking must be on the caller's side
+	successBuild := false
+	defer func() {
+		if !successBuild {
+			unlock()
+		}
+	}()
 
 	isCorrectVer, err := checkVersion(outputLinkPath, goVersion, patchesVer)
 	if err != nil {
-		return "", err
+		return "", nil, err
 	}
 	if isCorrectVer && fileExists(outputLinkPath) {
-		return outputLinkPath, nil
+		successBuild = true
+		return outputLinkPath, unlock, nil
 	}
 
 	srcDir := filepath.Join(goRoot, baseSrcSubdir)
-	workingDirectory := filepath.Join(tempDirectory, "linker-src")
+	workingDir := filepath.Join(tempDir, "linker-src")
 
-	overlay, err := applyPatches(srcDir, workingDirectory, patches)
+	overlay, err := applyPatches(srcDir, workingDir, patches)
 	if err != nil {
-		return "", err
+		return "", nil, err
 	}
-	if err := compileLinker(workingDirectory, overlay, outputLinkPath); err != nil {
-		return "", err
+	if err := buildLinker(workingDir, overlay, outputLinkPath); err != nil {
+		return "", nil, err
 	}
 	if err := writeVersion(outputLinkPath, goVersion, patchesVer); err != nil {
-		return "", err
+		return "", nil, err
 	}
-	return outputLinkPath, nil
+	successBuild = true
+	return outputLinkPath, unlock, nil
 }

main.go

@@ -438,12 +438,21 @@ func mainErr(args []string) error {
 			log.Printf("skipping transform on %s with args: %s", tool, strings.Join(transformed, " "))
 		}
 
+		var linkUnlock func()
+		defer func() {
+			if linkUnlock != nil {
+				linkUnlock()
+			}
+		}()
+
 		executablePath := args[0]
 		if tool == "link" {
-			modifiedLinkPath, err := linker.PatchLinker(cache.GoEnv.GOROOT, cache.GoEnv.GOVERSION, cache.GoEnv.GOEXE, sharedTempDir)
+			modifiedLinkPath, unlock, err := linker.PatchLinker(cache.GoEnv.GOROOT, cache.GoEnv.GOVERSION, cache.GoEnv.GOEXE, sharedTempDir)
 			if err != nil {
 				return fmt.Errorf("cannot get modified linker: %v", err)
 			}
+
+			linkUnlock = unlock
 			executablePath = modifiedLinkPath
 			os.Setenv(linker.MagicValueEnv, strconv.FormatUint(uint64(magicValue()), 10))
 

runtime_patch.go

@@ -49,11 +49,10 @@ func updateMagicValue(file *ast.File, magicValue uint32) {
 
 	for _, decl := range file.Decls {
 		funcDecl, ok := decl.(*ast.FuncDecl)
-		if !ok || funcDecl.Name.Name != "moduledataverify1" {
-			continue
+		if ok && funcDecl.Name.Name == "moduledataverify1" {
+			ast.Inspect(funcDecl, updateMagic)
+			break
 		}
-		ast.Inspect(funcDecl, updateMagic)
-		break
 	}
 
 	if !magicUpdated {