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.

Sign up for GitHub

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

cmd/cgo: mark cgo generated wrappers hidden #28340

Open
eliasnaur opened this issue Oct 23, 2018 · 7 comments
Open

cmd/cgo: mark cgo generated wrappers hidden #28340

eliasnaur opened this issue Oct 23, 2018 · 7 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@eliasnaur
Copy link
Contributor

Please answer these questions before submitting your issue. Thanks!

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

go version devel +c2a8d5972f Sat Sep 22 10:58:54 2018 +0200 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/elias/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/elias/dev/go"
GOPROXY=""
GORACE=""
GOROOT="/home/elias/dev/go-release"
GOTMPDIR=""
GOTOOLDIR="/home/elias/dev/go-release/pkg/tool/linux_amd64"
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=/tmp/go-build186016377=/tmp/go-build -gno-record-gcc-switches"

What did you do?

$ cat test.go
package main

/*
__attribute__ ((visibility ("hidden"))) void some_c_func() {
}
*/
import "C"

func main() {
	C.some_c_func()
}
$ go build test.go
$ objdump -t test|grep some_c_func

What did you expect to see?

All cgo wrappers for some_c_func marked local.

What did you see instead?

00000000006bd160 l     O .data	0000000000000008              main._cgo_1622f853b35c_Cfunc_some_c_func
000000000044fb20 l     F .text	0000000000000052              main._Cfunc_some_c_func
000000000044fc10 l     F .text	0000000000000001              some_c_func
000000000044fc20 g     F .text	0000000000000001              _cgo_1622f853b35c_Cfunc_some_c_func

The _cgo_1622f853b35c_Cfunc_some_c_func function is global, not local as I would expect. I would expect it to be local even if some_c_func itself is not.
@ianlancetaylor ianlancetaylor changed the title cgo: mark cgo generated wrappers hidden cmd/cgo: mark cgo generated wrappers hidden Oct 23, 2018
@ianlancetaylor ianlancetaylor added help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Oct 23, 2018
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Oct 23, 2018
@steeve
Copy link
Contributor

steeve commented Feb 23, 2019

I have the exact same problem with //exported functions.

@steeve
Copy link
Contributor

steeve commented Feb 23, 2019

I think this is because the generated C function isn't marked with the same visibility attribute:

CGO_NO_SANITIZE_THREAD
void
_cgo_ce009a93dc37_Cfunc_some_c_func(void *v)
{
	struct {
		char unused;
	} __attribute__((__packed__)) *_cgo_a = v;
	_cgo_tsan_acquire();
	some_c_func();
	_cgo_tsan_release();
}

Looking at https://github.com/golang/go/blob/master/src/cmd/cgo/out.go#L618, it seems the CGo wrappers are not generated with hidden attributes. Is there a reason to that ?

@steeve
Copy link
Contributor

steeve commented Feb 23, 2019

Reading https://github.com/bazelbuild/rules_go/blob/master/go/private/rules/cgo.bzl#L672-L683, it seems this is because the symbols are needed to generate _cgo_import.go, although I'm not sure they need to be exported.

@steeve
Copy link
Contributor

steeve commented Feb 23, 2019

Adding static to the generated wrapper makes toolchain bootstrapping fail with:

# cmd/trace
crypto/x509._cgo_cd891bf309c8_Cfunc_CFDataGetBytePtr: relocation target _cgo_cd891bf309c8_Cfunc_CFDataGetBytePtr not defined
crypto/x509._cgo_cd891bf309c8_Cfunc_CFDataGetLength: relocation target _cgo_cd891bf309c8_Cfunc_CFDataGetLength not defined
crypto/x509._cgo_cd891bf309c8_Cfunc_CFRelease: relocation target _cgo_cd891bf309c8_Cfunc_CFRelease not defined
crypto/x509._cgo_cd891bf309c8_Cfunc_FetchPEMRoots: relocation target _cgo_cd891bf309c8_Cfunc_FetchPEMRoots not defined
net._cgo_0b21510eea8f_C2func_getaddrinfo: relocation target _cgo_0b21510eea8f_C2func_getaddrinfo not defined
net._cgo_0b21510eea8f_C2func_getnameinfo: relocation target _cgo_0b21510eea8f_C2func_getnameinfo not defined
net._cgo_0b21510eea8f_Cfunc_freeaddrinfo: relocation target _cgo_0b21510eea8f_Cfunc_freeaddrinfo not defined
net._cgo_0b21510eea8f_Cfunc_gai_strerror: relocation target _cgo_0b21510eea8f_Cfunc_gai_strerror not defined
...

Marking them with __attribute__ ((visibility ("hidden"))) makes it fail with:

# os/signal/internal/pty
cgo-gcc-prolog:32:40: error: expected identifier or '('
cgo-gcc-prolog:56:40: error: expected identifier or '('
cgo-gcc-prolog:80:40: error: expected identifier or '('
...

So I'm guessing it's may not be an easy fix.

Finally, reading go tool cgo we find the -dynimport flags which is used to generate _cgo_import.go, I think the wrappers being exported is actually a needed requirement.

@steeve
Copy link
Contributor

steeve commented Feb 23, 2019 •
edited

It seems we could get away with patching https://github.com/golang/go/blob/master/src/cmd/cgo/out.go#L283 to use elf.Symbols() and some string matching (something like strings.HasPrefix(s, "_cgo_")) but .Symbols() doesn't exist for macho and pe.

Even if they existed I'm not even sure it would work.

@steeve
Copy link
Contributor

steeve commented Feb 23, 2019 •
edited

I actually managed to make the wrapper hidden by leveraging #pragma GCC visibility:

/*
#pragma GCC visibility push(hidden)

static void some_c_func()
{
}
*/
import "C"
$ nm export | grep some
0000000004050380 t __cgo_de03a8cc6e60_Cfunc_some_c_func
0000000004050290 t main._Cfunc_some_c_func
00000000040bce28 d main._cgo_de03a8cc6e60_Cfunc_some_c_func

and without the #pragma:

$ nm export | grep some
0000000004050380 T __cgo_76ff56afc72d_Cfunc_some_c_func
0000000004050290 t main._Cfunc_some_c_func
00000000040bce28 d main._cgo_76ff56afc72d_Cfunc_some_c_func

Not sure how safe that is, though.

@steeve
Copy link
Contributor

steeve commented Feb 23, 2019 •
edited

Actually, it's better to put it right before the end of the comment, or else inludes have it defined and errors such as ld: error: hidden symbol 'free' is not defined locally will pop up:

/*
static void some_c_func()
{
}
#pragma GCC visibility push(hidden)
*/
import "C"

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
4 participants
@steeve @eliasnaur @ianlancetaylor @gopherbot