Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reflect: Value.Call crashes due to dead methods elimination #42579

Closed
shawndx opened this issue Nov 13, 2020 · 5 comments
Closed

reflect: Value.Call crashes due to dead methods elimination #42579

shawndx opened this issue Nov 13, 2020 · 5 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Milestone

Comments

@shawndx
Copy link
Contributor

shawndx commented Nov 13, 2020

What version of Go are you using (go version)?

$ go version
go version devel +30ba798093 Thu Nov 12 22:50:40 2020 +0000 linux/amd64

Does this issue reproduce with the latest release?

Not reproducible with go1.15.2, looks like it was introduced by a recent commit 95848fc.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env

What did you do?

Simplified Test Case:

plugin
|-- build.sh
|-- go.mod
|-- go.sum
|-- plugin
| -- plugin.go |-- test | -- main.go
-- type -- type.go

3 directories, 6 files

  1. build.sh
#!/bin/bash

pwd=`pwd`

cd $pwd/plugin
go build -buildmode=plugin -o plugin.so

cd $pwd/test
go build
./test
  1. go.mod
module plugin

go 1.16
  1. plugin/plugin.go
package main

import (
	typ "plugin/type"
)

var V *typ.T = &typ.T{"hello plugin", "xd"}
  1. type/type.go
package greeting_type

type T struct {
        Msg    string
        Sender string
}

func (t *T) Greeting() string {
        r := t.Msg + " from " + t.Sender
	return r
}

func (t *T) GreetingWithArg(a string) string {
        r := t.Msg + " from " + t.Sender + " with args:" + a
	return r
}
  1. test/main.go
package main

import (
	"fmt"
	"plugin"
	"reflect"
)

func lookup_symbol(p *plugin.Plugin, sn string) plugin.Symbol {
	f, err := p.Lookup(sn)
	if err != nil {
		fmt.Println(err)
		return nil
	}

	// just diagnostic
	switch v := f.(type) {
	default:
		fmt.Printf("Found symbol %s of type %T\n", sn, v)
	}
	return f
}

func call_method(p *plugin.Plugin, sn, fn string, args []reflect.Value) {
	sym := lookup_symbol(p, sn)
	if sym == nil {
		fmt.Printf("No symbol named %s found\n", sn)
		return
	}

	f := reflect.ValueOf(sym).Elem().MethodByName(fn)
	if !f.IsValid() || f.IsZero() || f.IsNil() {	// anything more ??
		fmt.Printf("No valid method %s for receiver %s found\n", fn, sn)
		return
	}

	// to check validity (number, type) of arguments, etc.

	rv := f.Call(args)

	// need an anonymous way to handle results?
	if len(rv) > 0 {
		r := rv[0]
		fmt.Printf("First return value: %#v\n", r)
	}
}

func test_plugin(path, so string) {
	p, err := plugin.Open(path + "/" + so)
	if err != nil {
		fmt.Println(err)
		return
	}

	call_method(p, "V", "Greeting", []reflect.Value{})
	call_method(p, "V", "GreetingWithArg", []reflect.Value{reflect.ValueOf("aa")})

	// illegal
	call_method(p, "V", "GreetingWithArgs", []reflect.Value{reflect.ValueOf("aa"), reflect.ValueOf("bb")})
}

func main() {
	test_plugin("../plugin", "plugin.so")
}

What did you expect to see?

Testcase passed, as with go1.15.2 and previous versions.

What did you see instead?

A crash in reflect.Value.Call

Found symbol V of type **greeting_type.T
unexpected fault address 0xffffffffffffffff
fatal error: fault
[signal SIGSEGV: segmentation violation code=0x1 addr=0xffffffffffffffff pc=0xffffffffffffffff]

goroutine 1 [running]:
runtime.throw(0x546d71, 0x5)
	/home/jixiangdong/util/go.v/src/runtime/panic.go:1112 +0x72 fp=0xc00004fa80 sp=0xc00004fa50 pc=0x4827b2
runtime.sigpanic()
	/home/jixiangdong/util/go.v/src/runtime/signal_unix.go:737 +0x268 fp=0xc00004fab8 sp=0xc00004fa80 pc=0x4975e8
runtime.call32(0xc00007c300, 0xc000011448, 0xc00000e0a8, 0x800000018)
	/home/jixiangdong/util/go.v/src/runtime/asm_amd64.s:547 +0x3e fp=0xc00004fae8 sp=0xc00004fab8 pc=0x4b3efe
reflect.Value.call(0x7f745430c4a0, 0x7f745434e048, 0x293, 0x546b98, 0x4, 0xc00004fe98, 0x0, 0x0, 0x56e5a0, 0xc000066180, ...)
	/home/jixiangdong/util/go.v/src/reflect/value.go:476 +0x8e7 fp=0xc00004fcf0 sp=0xc00004fae8 pc=0x4d6247
reflect.Value.Call(0x7f745430c4a0, 0x7f745434e048, 0x293, 0xc00004fe98, 0x0, 0x0, 0x7f745434e048, 0x293, 0x9)
	/home/jixiangdong/util/go.v/src/reflect/value.go:337 +0xb9 fp=0xc00004fd70 sp=0xc00004fcf0 pc=0x4d5719
main.call_method(0xc00007c1e0, 0x546a12, 0x1, 0x547307, 0x8, 0xc00004fe98, 0x0, 0x0)
	/home/jixiangdong/data/work/example.com/plugin/test/main.go:39 +0x2c5 fp=0xc00004fe50 sp=0xc00004fd70 pc=0x51aba5
@shawndx
Copy link
Contributor Author

shawndx commented Nov 13, 2020

@cherrymui Looks like the case starts crashing with the following change.

commit 95848fc
Author: Cherry Zhang cherryyz@google.com
Date: Mon Jun 8 18:38:59 2020 -0400

[dev.link] cmd/compile, cmd/link: remove dead methods if type is not used in interface

@ianlancetaylor ianlancetaylor added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker labels Nov 13, 2020
@ianlancetaylor ianlancetaylor added this to the Go1.16 milestone Nov 13, 2020
@martisch
Copy link
Contributor

I would vote for a minimal deadcode elimination mode in the linker/compiler as soon as plugins are involved. Maybe the should be a general flag as I would assume plugins will be able to work around assumptions of many future interprocedural optimizations that rely on understanding global program dependencies/call flows.

@cherrymui
Copy link
Member

Thanks. Yeah, I think that's right, as reflect.Method can be used on the other side of DSO boundary. I think it may be possible to make a similar case even without that CL.

@cherrymui
Copy link
Member

Maybe the should be a general flag as I would assume plugins will be able to work around assumptions of many future interprocedural optimizations that rely on understanding global program dependencies/call flows.

In the linker there is buildmode=plugin and CanUsePlugin (i.e. plugin.Open symbol exists). It's hard to do in the compiler as when compiling one package we don't know if another package uses plugin.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/269819 mentions this issue: cmd/compile: mark plugin-exported types as used in interface

@golang golang locked and limited conversation to collaborators Nov 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Projects
None yet
Development

No branches or pull requests

5 participants