keep importmap entries in the right direction

For packages that we alter, we parse and modify the importcfg file.
Parsing is necessary so we can locate obfuscated object files,
which we use to remember what identifiers were obfuscated.
Modifying the files is necessary when we obfuscate import paths,
and those import paths have entries in an importcfg file.

However, we made one crucial mistake when writing the code.
When handling importmap entries such as:

	importmap golang.org/x/net/idna=vendor/golang.org/x/net/idna

we would name the two sides beforePath and afterPath, respectively.
They were added to importMap with afterPath as the key,
but when we iterated over the map to write a modified importcfg file,
we would then assume the key is beforepath.
All in all, we would end up writing the opposite direction:

	importmap vendor/golang.org/x/net/idna=golang.org/x/net/idna

This would ultimately result in the importmap never being useful,
and some rather confusing error messages such as:

	cannot find package golang.org/x/net/idna (using -importcfg)

Add a test case that reproduces this error,
and fix the code so it always uses beforePath as the key.
Note that we were also updating importCfgEntries with such entries.
I could not reproduce any failure when just removing that code,
nor could I explain why it was added there in the first place.
As such, remove that bit of code as well.

Finally, a reasonable question might be why we never noticed the bug.
In practice, such "importmap"s, represented as ImportMap by "go list",
only currently appear for packages vendored into the standard library.
Until very recently, we didn't support obfuscating most of std,
so we would usually not alter the affected importcfg files.
Now that we do parse and modify them, the bug surfaced.

Fixes #408.
pull/422/head
Daniel Martí 3 years ago committed by Andrew LeFevre
parent ea2e0bdf71
commit d5d1131b75

@ -194,7 +194,7 @@ func obfuscatedTypesPackage(path string) *types.Package {
entry = &importCfgEntry{
packagefile: pkg.Export,
}
// Adding it to buildInfo.imports allows us to reuse the
// Adding it to importCfgEntries allows us to reuse the
// "if" branch below. Plus, if this edge case triggers
// multiple times in a single package compile, we can
// call "go list" once and cache its result.
@ -351,6 +351,7 @@ func mainErr(args []string) error {
return err
}
}
// log.Println(tool, transformed)
cmd := exec.Command(args[0], transformed...)
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
@ -867,6 +868,7 @@ func processImportCfg(flags []string) (newImportCfg string, _ error) {
return "", err
}
// TODO: use slices rather than maps to generate a deterministic importcfg.
importCfgEntries = make(map[string]*importCfgEntry)
importMap := make(map[string]string)
@ -888,7 +890,7 @@ func processImportCfg(flags []string) (newImportCfg string, _ error) {
continue
}
beforePath, afterPath := args[:j], args[j+1:]
importMap[afterPath] = beforePath
importMap[beforePath] = afterPath
case "packagefile":
args := strings.TrimSpace(line[i+1:])
j := strings.Index(args, "=")
@ -899,10 +901,6 @@ func processImportCfg(flags []string) (newImportCfg string, _ error) {
impPkg := &importCfgEntry{packagefile: objectPath}
importCfgEntries[importPath] = impPkg
if otherPath, ok := importMap[importPath]; ok {
importCfgEntries[otherPath] = impPkg
}
}
}
// log.Printf("%#v", buildInfo)

@ -5,6 +5,12 @@ stderr '^GOPRIVATE="match-absolutely/nothing" does not match any packages to be
env GOPRIVATE=test/main/imported
garble build ./importer
# Obfuscated packages which import non-obfuscated std packages.
# Some of the imported std packages use "import maps" due to vendoring,
# and a past bug made this case fail for "garble build".
env GOPRIVATE=test/main
garble build -o=out ./stdimporter
[short] stop # rebuilding std is slow
env GOPRIVATE='*'

Loading…
Cancel
Save