Skip to content

Commit

Permalink
avoid reflect method call panics with GOGARBLE=*
Browse files Browse the repository at this point in the history
We were obfuscating reflect's package path and its declared names,
but the toolchain wants to detect the presence of method reflection
to turn down the aggressiveness of dead code elimination.

Given that the obfuscation broke the detection,
we could easily end up in crashes when making reflect calls:

	fatal error: unreachable method called. linker bug?

	goroutine 1 [running]:
	runtime.throw({0x50c9b3?, 0x2?})
		runtime/panic.go:1047 +0x5d fp=0xc000063660 sp=0xc000063630 pc=0x43245d
	runtime.unreachableMethod()
		runtime/iface.go:532 +0x25 fp=0xc000063680 sp=0xc000063660 pc=0x40a845
	runtime.call16(0xc00010a360, 0xc00000e0a8, 0x0, 0x0, 0x0, 0x8, 0xc000063bb0)
		runtime/wcS9OpRFL:728 +0x49 fp=0xc0000636a0 sp=0xc000063680 pc=0x45eae9
	runtime.reflectcall(0xc00001c120?, 0x1?, 0x1?, 0x18110?, 0xc0?, 0x1?, 0x1?)
		<autogenerated>:1 +0x3c fp=0xc0000636e0 sp=0xc0000636a0 pc=0x462e9c

Avoid obfuscating the three names which cause problems: "reflect",
"Method", and "MethodByName".

While here, we also teach obfuscatedImportPath to skip "runtime",
as I also saw that the toolchain detects it for many reasons.
That wasn't a problem yet, as we do not obfuscate the runtime,
but it was likely going to become a problem in the future.
  • Loading branch information
mvdan committed Dec 13, 2022
1 parent 481e3a1 commit 9c0cdc6
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 13 deletions.
19 changes: 13 additions & 6 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -1741,12 +1741,19 @@ func (tf *transformer) transformGo(file *ast.File) *ast.File {
return true // universe scope
}

if pkg.Path() == "embed" {
// The Go compiler needs to detect types such as embed.FS.
// That will fail if we change the import path or type name.
// Leave it as is.
// Luckily, the embed package just declares the FS type.
return true
// The Go toolchain needs to detect symbols from these packages,
// so we are not obfuscating their package paths or declared names.
switch pkg.Path() {
case "embed":
// FS is detected by the compiler for //go:embed.
return name == "FS"
case "reflect":
// Per the linker's deadcode.go docs,
// the Method and MethodByName methods are what drive the logic.
switch name {
case "Method", "MethodByName":
return true
}
}

// The package that declared this object did not obfuscate it.
Expand Down
14 changes: 11 additions & 3 deletions shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,17 @@ type listedPackage struct {
}

func (p *listedPackage) obfuscatedImportPath() string {
// We can't obfuscate the embed package's import path,
// as the toolchain expects to recognize the package by it.
if p.ImportPath == "embed" || !p.ToObfuscate {
// We can't obfuscate these standard library import paths,
// as the toolchain expects to recognize the packages by them:
//
// * runtime: it is special in many ways
// * reflect: its presence turns down dead code elimination
// * embed: its presence enables using //go:embed
switch p.ImportPath {
case "runtime", "reflect", "embed":
return p.ImportPath
}
if !p.ToObfuscate {
return p.ImportPath
}
newPath := hashWithPackage(p, p.ImportPath)
Expand Down
4 changes: 0 additions & 4 deletions testdata/script/reflect.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,9 @@ func main() {
printfWithoutPackage("%T\n", importedpkg.ReflectTypeOf(2))
printfWithoutPackage("%T\n", importedpkg.ReflectTypeOfIndirect(4))


// More complex use of reflect.
v := importedpkg.ReflectValueOfVar
printfWithoutPackage("%#v\n", v)
// Keep the method from being unreachable, otherwise Call below may panic.
// TODO(mvdan): This only started being necessary with GOGARBLE=*. Why?
Sink = v.ExportedMethodName
method := reflect.ValueOf(&v).MethodByName("ExportedMethodName")
if method.IsValid() {
fmt.Println(method.Call(nil))
Expand Down

0 comments on commit 9c0cdc6

Please sign in to comment.