Skip to content

Commit

Permalink
avoid build ID mismatches when using -debugdir
Browse files Browse the repository at this point in the history
A recent change added the -debugdir value to addGarbleToHash,
which is part of the hash seed for all obfuscation taking place.

In principle, it was okay to add, just like any other garble flag.
In practice, it broke the added test case:

	> garble -debugdir ./debug1 build
	[stderr]
	# test/main
	FBi9xa6e.(*ac0bCOhR).String: relocation target FBi9xa6e.rV55e6H9 not defined
	qmECK6zf.init.0: relocation target qmECK6zf.eUU08z98 not defined
	[...]

This is because -debugdir gets turned into an absolute path,
and not all garble processes ended up using it consistently.

The fix is rather simple; since -debugdir doesn't affect obfuscation,
don't include it in the build hash seeding at all.

Fixes #451.
  • Loading branch information
mvdan authored and lu4p committed Jan 10, 2022
1 parent 34cbd1b commit ff80300
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 7 deletions.
19 changes: 16 additions & 3 deletions hash.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,14 @@ func addGarbleToHash(inputHash []byte) []byte {
if cache.GOGARBLE != "" {
fmt.Fprintf(h, " GOGARBLE=%s", cache.GOGARBLE)
}
appendFlags(h)
appendFlags(h, true)
return h.Sum(nil)[:buildIDComponentLength]
}

// appendFlags writes garble's own flags to w in string form.
// Errors are ignored, as w is always a buffer or hasher.
func appendFlags(w io.Writer) {
// If forBuildHash is set, only the flags affecting a build are written.
func appendFlags(w io.Writer, forBuildHash bool) {
if flagLiterals {
io.WriteString(w, " -literals")
}
Expand All @@ -114,7 +115,19 @@ func appendFlags(w io.Writer) {
if flagDebug {
io.WriteString(w, " -debug")
}
if flagDebugDir != "" {
if flagDebugDir != "" && !forBuildHash {
// -debugdir is a bit special.
//
// When passing down flags via -toolexec,
// we do want the actual flag value to be kept.
//
// For build hashes, we can skip the flag entirely,
// as it doesn't affect obfuscation at all.
//
// TODO: in the future, we could avoid using the -a build flag
// by using "-debugdir=yes" here, and caching the obfuscated source.
// Incremental builds would recover the cached source
// to repopulate the output directory if it was removed.
io.WriteString(w, " -debugdir=")
io.WriteString(w, flagDebugDir)
}
Expand Down
2 changes: 1 addition & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ This command wraps "go %s". Below is its help:
var toolexecFlag strings.Builder
toolexecFlag.WriteString("-toolexec=")
toolexecFlag.WriteString(cache.ExecPath)
appendFlags(&toolexecFlag)
appendFlags(&toolexecFlag, false)

goArgs := []string{
command,
Expand Down
14 changes: 11 additions & 3 deletions testdata/scripts/debugdir.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
env GOGARBLE=test/main
env GOGARBLE=*

# ! garble -debug -debugdir ./debug1 build
# cp stderr /tmp/log
# exit
garble -debugdir ./debug1 build
exists 'debug1/test/main/imported/imported.go' 'debug1/test/main/main.go'
exists 'debug1/test/main/imported/imported.go' 'debug1/test/main/main.go' 'debug1/reflect/type.go'
! grep ImportedFunc $WORK/debug1/test/main/imported/imported.go
! grep ImportedFunc $WORK/debug1/test/main/main.go
! grep 'some comment' $WORK/debug1/test/main/main.go
Expand All @@ -22,7 +25,11 @@ go 1.17
-- main.go --
package main

import "test/main/imported" // some comment
import (
"reflect"

"test/main/imported" // some comment
)

type someType int // some comment
var someVar = 0
Expand All @@ -33,6 +40,7 @@ type someStruct struct {

func main() {
imported.ImportedFunc()
reflect.TypeOf(123)
}

-- imported/imported.go --
Expand Down

0 comments on commit ff80300

Please sign in to comment.