Skip to content

Commit

Permalink
avoid breaking github.com/davecgh/go-spew
Browse files Browse the repository at this point in the history
It assumes that reflect.Value has a field named "flag",
which wasn't the case with obfuscated builds as we obfuscated it.

We already treated the reflect package as special,
for instance when not obfuscating Method or MethodByName.
In a similar fashion, mark reflect's rtype and Value types to not be
obfuscated alongside their field names. Note that rtype is the
implementation behind the reflect.Type interface.

This fix is fairly manual and repetitive.
transformCompile, transformLinkname, and transformAsm should all
use the same mechanism to tell if names should be obfuscated.
However, they do not do that right now, and that refactor feels too
risky for a bugfix release. We add more TODOs instead.

We're not adding go-spew to scripts/check-third-party.sh since the
project is largely abandoned. It's not even a Go module yet.
The only broken bit from it is what we've added to our tests.

Fixes #676.
  • Loading branch information
mvdan committed Feb 11, 2023
1 parent b0f8dfb commit 5effe20
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 8 deletions.
27 changes: 19 additions & 8 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -1118,7 +1118,10 @@ func (tf *transformer) transformLinkname(localName, newName string) (string, str

var newForeignName string
if receiver, name, ok := strings.Cut(foreignName, "."); ok {
if strings.HasPrefix(receiver, "(*") {
if lpkg.ImportPath == "reflect" && (receiver == "(*rtype)" || receiver == "Value") {
// These receivers are not obfuscated.
// See the TODO below.
} else if strings.HasPrefix(receiver, "(*") {
// pkg/path.(*Receiver).method
receiver = strings.TrimPrefix(receiver, "(*")
receiver = strings.TrimSuffix(receiver, ")")
Expand All @@ -1129,8 +1132,8 @@ func (tf *transformer) transformLinkname(localName, newName string) (string, str
}
// Exported methods are never obfuscated.
//
// TODO: we're duplicating the logic behind these decisions.
// How can we more easily reuse the same logic?
// TODO(mvdan): We're duplicating the logic behind these decisions.
// Reuse the logic with transformCompile.
if !token.IsExported(name) {
name = hashWithPackage(lpkg, name)
}
Expand Down Expand Up @@ -1908,19 +1911,29 @@ func (tf *transformer) transformGoFile(file *ast.File) *ast.File {
return true // universe scope
}

// The Go toolchain needs to detect symbols from these packages,
// so we are not obfuscating their package paths or declared names.
// TODO: We match by object name here, which is actually imprecise.
// For example, in package embed we match the type FS, but we would also
// match any field or method named FS.
path := pkg.Path()
switch path {
case "embed":
// FS is detected by the compiler for //go:embed.
// TODO: We probably want a conditional, otherwise we're not
// obfuscating the embed package at all.
return name == "FS"
case "reflect":
switch name {
// Per the linker's deadcode.go docs,
// the Method and MethodByName methods are what drive the logic.
switch name {
case "Method", "MethodByName":
return true
// Some packages reach into reflect internals, like go-spew.
// It's not particularly right of them to do that,
// and it's entirely unsupported, but try to accomodate for now.
// At least it's enough to leave the rtype and Value types intact.
case "rtype", "Value":
tf.recursivelyRecordAsNotObfuscated(obj.Type())
return true
}
}

Expand Down Expand Up @@ -2059,8 +2072,6 @@ func (tf *transformer) recursivelyRecordAsNotObfuscated(t types.Type) {
obj := t.Obj()
if pkg := obj.Pkg(); pkg == nil || pkg != tf.pkg {
return // not from the specified package
} else if pkg.Path() == "reflect" {
return // reflect's own types can always be obfuscated
}
if recordedAsNotObfuscated(obj) {
return // prevent endless recursion
Expand Down
2 changes: 2 additions & 0 deletions testdata/script/linker.txtar
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# Past garble versions might not properly patch cmd/link with "git apply"
# when running inside a git repository. Skip the extra check with -short.
[!short] [exec:git] exec git init -q
[!short] [exec:git] env GARBLE_CACHE_DIR=$WORK/linker-cache

Expand Down
10 changes: 10 additions & 0 deletions testdata/script/reflect.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,16 @@ type FmtType struct {
FmtTypeField int
}

// copied from github.com/davecgh/go-spew, which reaches into reflect's internals
var _ = func() uintptr {
field, ok := reflect.TypeOf(reflect.Value{}).FieldByName("flag")
if !ok {
panic("reflect.Value has no flag field")
}
return field.Offset
}()


-- importedpkg/imported.go --
package importedpkg

Expand Down

0 comments on commit 5effe20

Please sign in to comment.