rely on go/types alias tracking

Allows us to remove EmbeddedAliasFields, recordedObjectString,
and all the logic around using them.

Resolves an issue where a user was running into a panic in
our logic to record embedded aliases.

Note that this means we require Go 1.23.5 or later now,
which also meant some changes to goversion.txtar to keep it green.

Fixes #827.
pull/908/head
Daniel Martí 3 months ago
parent b50710696a
commit 42cc022611
No known key found for this signature in database

@ -1,6 +1,8 @@
module mvdan.cc/garble
go 1.23
// Before the .5 bugfix release, alias tracking via go/types
// was broken; see https://go.dev/issue/70517.
go 1.23.5
require (
github.com/bluekeyes/go-gitdiff v0.8.0

@ -292,8 +292,8 @@ func (e errJustExit) Error() string { return fmt.Sprintf("exit: %d", e) }
func goVersionOK() bool {
const (
minGoVersion = "go1.23" // the first major version we support
maxGoVersion = "go1.24" // the first major version we don't support
minGoVersion = "go1.23.5" // the minimum Go version we support; could be a bugfix release if needed
unsupportedGo = "go1.24" // the first major version we don't support
)
// rxVersion looks for a version like "go1.2" or "go1.2.3" in `go env GOVERSION`.
@ -311,8 +311,8 @@ func goVersionOK() bool {
fmt.Fprintf(os.Stderr, "Go version %q is too old; please upgrade to %s or newer\n", toolchainVersionFull, minGoVersion)
return false
}
if version.Compare(sharedCache.GoVersion, maxGoVersion) >= 0 {
fmt.Fprintf(os.Stderr, "Go version %q is too new; Go linker patches aren't available for %s or later yet\n", toolchainVersionFull, maxGoVersion)
if version.Compare(sharedCache.GoVersion, unsupportedGo) >= 0 {
fmt.Fprintf(os.Stderr, "Go version %q is too new; Go linker patches aren't available for %s or later yet\n", toolchainVersionFull, unsupportedGo)
return false
}
@ -337,11 +337,6 @@ garble was built with %q and can't be used with the newer %q; rebuild it with a
}
func mainErr(args []string) error {
// TODO(mvdan): until we can rely on alias tracking to work reliably,
// we must turn it off so that we don't get inconsistent types.
// See: https://go.dev/issue/70394
os.Setenv("GODEBUG", "gotypesalias=0")
command, args := args[0], args[1:]
// Catch users reaching for `go build -toolexec=garble`.
@ -1384,11 +1379,6 @@ func (tf *transformer) processImportCfg(flags []string, requiredPkgs []string) (
type (
funcFullName = string // as per go/types.Func.FullName
objectString = string // as per recordedObjectString
typeName struct {
PkgPath string // empty if builtin
Name string
}
)
// pkgCache contains information about a package that will be stored in fsCache.
@ -1405,20 +1395,11 @@ type pkgCache struct {
// ReflectObjectNames maps obfuscated names which are reflected to their "real"
// non-obfuscated names.
ReflectObjectNames map[objectString]string
// EmbeddedAliasFields records which embedded fields use a type alias.
// They are the only instance where a type alias matters for obfuscation,
// because the embedded field name is derived from the type alias itself,
// and not the type that the alias points to.
// In that way, the type alias is obfuscated as a form of named type,
// bearing in mind that it may be owned by a different package.
EmbeddedAliasFields map[objectString]typeName
}
func (c *pkgCache) CopyFrom(c2 pkgCache) {
maps.Copy(c.ReflectAPIs, c2.ReflectAPIs)
maps.Copy(c.ReflectObjectNames, c2.ReflectObjectNames)
maps.Copy(c.EmbeddedAliasFields, c2.EmbeddedAliasFields)
}
func ssaBuildPkg(pkg *types.Package, files []*ast.File, info *types.Info) *ssa.Package {
@ -1489,8 +1470,7 @@ func computePkgCache(fsCache *cache.Cache, lpkg *listedPackage, pkg *types.Packa
"reflect.TypeOf": {0: true},
"reflect.ValueOf": {0: true},
},
ReflectObjectNames: map[objectString]string{},
EmbeddedAliasFields: map[objectString]typeName{},
ReflectObjectNames: map[objectString]string{},
}
for _, imp := range lpkg.Imports {
if imp == "C" {
@ -1542,29 +1522,6 @@ func computePkgCache(fsCache *cache.Cache, lpkg *listedPackage, pkg *types.Packa
}
}
// Fill EmbeddedAliasFields from the type info.
for name, obj := range info.Uses {
obj, ok := obj.(*types.TypeName)
if !ok || !obj.IsAlias() {
continue
}
vr, _ := info.Defs[name].(*types.Var)
if vr == nil || !vr.Embedded() {
continue
}
vrStr := recordedObjectString(vr)
if vrStr == "" {
continue
}
aliasTypeName := typeName{
Name: obj.Name(),
}
if pkg := obj.Pkg(); pkg != nil {
aliasTypeName.PkgPath = pkg.Path()
}
computed.EmbeddedAliasFields[vrStr] = aliasTypeName
}
// Fill the reflect info from SSA, which builds on top of the syntax tree and type info.
inspector := reflectInspector{
lpkg: lpkg,
@ -1919,38 +1876,11 @@ func (tf *transformer) transformGoFile(file *ast.File) *ast.File {
//
// Alternatively, if we don't have an alias, we still want to
// use the embedded type, not the field.
vrStr := recordedObjectString(vr)
aliasTypeName, ok := tf.curPkgCache.EmbeddedAliasFields[vrStr]
if ok {
aliasScope := tf.pkg.Scope()
if path := aliasTypeName.PkgPath; path == "" {
aliasScope = types.Universe
} else if path != tf.pkg.Path() {
// If the package is a dependency, import it.
// We can't grab the package via tf.pkg.Imports,
// because some of the packages under there are incomplete.
// ImportFrom will cache complete imports, anyway.
pkg2, err := tf.origImporter.ImportFrom(path, parentWorkDir, 0)
if err != nil {
panic(err)
}
aliasScope = pkg2.Scope()
}
tname, ok := aliasScope.Lookup(aliasTypeName.Name).(*types.TypeName)
if !ok {
panic(fmt.Sprintf("EmbeddedAliasFields pointed %q to a missing type %q", vrStr, aliasTypeName))
}
if !tname.IsAlias() {
panic(fmt.Sprintf("EmbeddedAliasFields pointed %q to a non-alias type %q", vrStr, aliasTypeName))
}
obj = tname
} else {
tname := namedType(obj.Type())
if tname == nil {
return true // unnamed type (probably a basic type, e.g. int)
}
obj = tname
tname := namedType(obj.Type())
if tname == nil {
return true // unnamed type (probably a basic type, e.g. int)
}
obj = tname
pkg = obj.Pkg()
}
if pkg == nil {

@ -1,10 +1,8 @@
package main
import (
"fmt"
"go/types"
"maps"
"path/filepath"
"slices"
"golang.org/x/tools/go/ssa"
@ -427,44 +425,6 @@ func (ri *reflectInspector) recursivelyRecordUsedForReflect(t types.Type, parent
}
}
// TODO: remove once alias tracking is properly implemented
func recordedObjectString(obj types.Object) objectString {
if obj == nil {
return ""
}
pkg := obj.Pkg()
if pkg == nil {
return ""
}
// Names which are not at the package level still need to avoid obfuscation in some cases:
//
// 1. Field names on global types, which can be reached via reflection.
// 2. Field names on anonymous types can also be reached via reflection.
// 3. Local named types can be embedded in a local struct, becoming a field name as well.
//
// For now, a hack: assume that packages don't declare the same field
// more than once in the same line. This works in practice, but one
// could craft Go code to break this assumption.
// Also note that the compiler's object files include filenames and line
// numbers, but not column numbers nor byte offsets.
if pkg.Scope() != obj.Parent() {
switch obj := obj.(type) {
case *types.Var: // struct fields; cases 1 and 2 above
if !obj.IsField() {
return "" // local variables don't need to be recorded
}
case *types.TypeName: // local named types; case 3 above
default:
return "" // other objects (labels, consts, etc) don't need to be recorded
}
pos := fset.Position(obj.Pos())
return fmt.Sprintf("%s.%s - %s:%d", pkg.Path(), obj.Name(),
filepath.Base(pos.Filename), pos.Line)
}
// For top-level names, "pkgpath.Name" is unique.
return pkg.Path() + "." + obj.Name()
}
// obfuscatedObjectName returns the obfucated name of a types.Object,
// parent is needed to correctly get the obfucated name of struct fields
func (ri *reflectInspector) obfuscatedObjectName(obj types.Object, parent *types.Struct) string {

@ -7,13 +7,13 @@ env PATH=${WORK}/.bin${:}${PATH}
# An empty go version.
env TOOLCHAIN_GOVERSION=''
! exec garble build
stderr 'Go version is too old; please upgrade to go1\.23 or newer'
stderr 'Go version is too old; please upgrade to go1\.23\.5 or newer'
# We should error on a devel version that's too old.
# Note that they lacked the "goN.M-" prefix.
env TOOLCHAIN_GOVERSION='devel +afb5fca Sun Aug 07 00:00:00 2020 +0000'
! exec garble build
stderr 'Go version is too old; please upgrade to go1\.23 or newer'
stderr 'Go version is too old; please upgrade to go1\.23\.5 or newer'
# Another form of old version; with an old "goN.M-" prefix.
env TOOLCHAIN_GOVERSION='devel go1.15-afb5fca Sun Aug 07 00:00:00 2020 +0000'
@ -22,15 +22,16 @@ stderr 'Go version "devel go1\.15-.*2020.*" is too old; please upgrade to go1\.2
# A current devel version should be fine.
# Note that we don't look at devel version timestamps.
env GARBLE_TEST_GOVERSION='go1.23.0'
env TOOLCHAIN_GOVERSION='devel go1.23-ad97d204f0 Sun Sep 12 16:46:58 2023 +0000'
! exec garble build
stderr 'mocking the real build'
env GARBLE_TEST_GOVERSION='go1.23.5'
# TODO: temporarily disabled while we do not support tip.
# env TOOLCHAIN_GOVERSION='devel go1.23-ad97d204f0 Sun Sep 12 16:46:58 2023 +0000'
# ! exec garble build
# stderr 'mocking the real build'
# We should error on a stable version that's too old.
env TOOLCHAIN_GOVERSION='go1.14'
! exec garble build
stderr 'Go version "go1\.14" is too old; please upgrade to go1\.23 or newer'
stderr 'Go version "go1\.14" is too old; please upgrade to go1\.23\.5 or newer'
# We should reject a future stable version, as we don't have linker patches yet.
# Note that we need to bump the version of Go that supposedly built it, too.
@ -40,27 +41,27 @@ env TOOLCHAIN_GOVERSION='go1.28.2'
stderr 'Go version "go1\.28\.2" is too new; Go linker patches aren''t available for go1\.24 or later yet'
# We should accept custom devel strings.
env TOOLCHAIN_GOVERSION='devel go1.23-somecustomversion'
env TOOLCHAIN_GOVERSION='devel go1.23.5-somecustomversion'
! exec garble build
stderr 'mocking the real build'
# The current toolchain may be older than the one that built garble.
env GARBLE_TEST_GOVERSION='go1.23.2'
env TOOLCHAIN_GOVERSION='go1.23.1'
env GARBLE_TEST_GOVERSION='go1.23.22'
env TOOLCHAIN_GOVERSION='go1.23.21'
! exec garble build
stderr 'mocking the real build'
# The current toolchain may be equal to the one that built garble.
env GARBLE_TEST_GOVERSION='devel go1.23-6673d5d701 Sun Mar 20 16:05:03 2023 +0000'
env TOOLCHAIN_GOVERSION='devel go1.23-6673d5d701 Sun Mar 20 16:05:03 2023 +0000'
env GARBLE_TEST_GOVERSION='go1.23.25'
env TOOLCHAIN_GOVERSION='go1.23.25'
! exec garble build
stderr 'mocking the real build'
# The current toolchain must not be newer than the one that built garble.
env GARBLE_TEST_GOVERSION='go1.18'
env TOOLCHAIN_GOVERSION='go1.23.1'
env TOOLCHAIN_GOVERSION='go1.23.25'
! exec garble build
stderr 'garble was built with "go1\.18" and can''t be used with the newer "go1\.23\.1"; rebuild '
stderr 'garble was built with "go1\.18" and can''t be used with the newer "go1\.23\.25"; rebuild '
# We'll error even if the difference is a minor (bugfix) level.
# In practice it probably wouldn't matter, but in theory it could still lead to tricky bugs.
@ -72,7 +73,7 @@ stderr 'garble was built with "go1\.23\.11" and can''t be used with the newer "g
# If garble builds itself and is then used, it won't know what version built it.
# As a fallback, we drop the comparison against the toolchain's version.
env GARBLE_TEST_GOVERSION='bogus version'
env TOOLCHAIN_GOVERSION='go1.23.3'
env TOOLCHAIN_GOVERSION='go1.23.25'
! exec garble build
stderr 'mocking the real build'
-- go.mod --

Loading…
Cancel
Save