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

Jump to bottom

plugin: Global variable is exported as pointer instead of value #31882

Closed
dwillemv opened this issue May 7, 2019 · 3 comments
Closed

plugin: Global variable is exported as pointer instead of value #31882

dwillemv opened this issue May 7, 2019 · 3 comments
Labels
FrozenDueToAge

Comments

@dwillemv
Copy link

dwillemv commented May 7, 2019 •
edited by randall77

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

$ go version
go version go1.12.5 linux/amd64

Does this issue reproduce with the latest release?

Yes, as well as against tip

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

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPROXY=""
GORACE=""
GOTMPDIR=""
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/dev/shm/go-build021671470=/tmp/go-build -gno-record-gcc-switches"

What did you do?

This example requires 3 packages: a plugin, an imported package and a main package
plugin:

package main

import "../q"

var Gv = q.MagicType{}

func main() {
var a interface{} = Gv
_ = a.(q.MagicType) // correctly does not panic when it is built and run
}

imported package:

package q

type MagicType struct{}

Main package that loads the plugin:

package main

import (
	"../q"
	"plugin"
)

func main() {
	p, err := plugin.Open("../p/p.so")
	if err != nil {
		panic(err)
	}
	sym, err := p.Lookup("Gv")
	if err != nil {
		panic(err)
	}
	// _ = sym.(*q.MagicType) // should panic, because Gv is not a pointer, but assert passes
	_ = sym.(q.MagicType)  // should not panic, but does since go1.8
}

What did you expect to see?

No panics.

What did you see instead?

panic: interface conversion: plugin.Symbol is *q.MagicType, not q.MagicType
@randall77
Copy link
Contributor

This is expected. When using the Lookup function in package plugin on a global variable, you get a pointer to that global variable.
If you instead got the value of that global variable, there would be no way to modify that global variable.

@dwillemv
Copy link
Author

dwillemv commented May 9, 2019

This design choice has implications on the readability of code that makes use of plugins.
Instead of being able to write
mt := sym.(magicType)
this now needs to be put in a less readable form:
var mt MagicType
*mt = *sym.(*MagicType)

To me this does not seem consistent either with the Go design philosophy or within the actual implementation. I can't think of anywhere else in the Go standard library where the type of a variable is changed automatically to satisfy an assumed requirement.
If the variable I'm reading was stored by value I don't expect a pointer on the other side.
Why must this package assume that the end-user would want to modify a global variable? If they want to modify it they can choose to use a pointer, but I think the choice shouldn't be made on their behalf.

@randall77
Copy link
Contributor

It's only the difference between

mt := sym.(magicType)

and

mt := *sym.(*magicType)

It's only 2 characters extra. I agree it's a bit tricky, but plugins are already well into the tricky realm.

I suspect most people don't want to modify the global variable, but some people will. We have to provide some way to modify a global. If Lookup didn't return a pointer, we'd need a separate function like Set or Addr to provide that functionality. (Or force plugin authors to do var F int; var Fptr = &F, and users to read Fptr if they want to modify F.)

In any case, it is too late now. We can't change the behavior of Lookup because of the Go 1 compatibility guarantee. We could add a new method like Read which would have the behavior you want, but that would end up being as much confusing as clarifying for someone new to the API.

Maybe some additional docs in the plugin package would help. The example makes it clear, but the Lookup function itself could mention something about this behavior.

@golang golang locked and limited conversation to collaborators May 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@dwillemv @randall77 @gopherbot