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: Conflicting types for generated symbols prevent LTO-enabled compilation #43830

Closed
dbenoit17 opened this issue Jan 21, 2021 · 7 comments
Closed

cmd/cgo: Conflicting types for generated symbols prevent LTO-enabled compilation #43830

dbenoit17 opened this issue Jan 21, 2021 · 7 comments
Labels
FrozenDueToAge help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@dbenoit17
Copy link

dbenoit17 commented Jan 21, 2021 •
edited

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

$ go version
go version go1.15.7 linux/amd64

Does this issue reproduce with the latest release?

Yes.

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

go env Output
$ go env
GOARCH="amd64"
GOHOSTOS="linux"

What did you do?

Enable link time optimization CFLAGS when building a Go program containing an //export directive.

package main

import "fmt"
import "C"

//export add
func add(x int32, y int32) int32 {
  return x + y
}

func main() {
  fmt.Println(add(1,1))
}

The above program builds normally using go build without additional CFLAGS. However, building with CGO_CFLAGS='-flto -ffat-lto-objects' go build produces the following error.

_cgo_export.c:21:13: error: variable ‘_cgoexp_c166a99bca60_add’ redeclared as function
cgo-generated-wrappers:1:5: note: previously declared here
lto1: fatal error: errors during merging of translation units
compilation terminated.
lto-wrapper: fatal error: /usr/bin/gcc returned 1 exit status
compilation terminated.
/usr/bin/ld: error: lto-wrapper failed
collect2: error: ld returned 1 exit status

This is due to the same symbol name being used in separate contexts as both a function and an integer.

What did you expect to see?

Program builds when enabling link time optimization via CFLAGs.

What did you see instead?

Generated intermediate C code for //export is incompatible with link time optimization.

Notes

Additional conflicting symbols may be created if the program binds the value of an exported function's C handle to a Go variable. The following program demonstrates the behavior.

package main

import "fmt"

// extern int add(int, int);
import "C"

//export add
func add(a int32, b int32) int32 {
  return a + b
}

func main() {
  fn := C.add
  fmt.Printf("%v\n", fn)
}

When built with CGO_CFLAGS='-flto -ffat-lto-objects' go build, the linker yields the following error.

cgo-generated-wrappers:1:13: error: function ‘add’ redeclared as variable
_cgo_export.c:24:9: note: previously declared here
_cgo_export.c:21:13: error: variable ‘_cgoexp_2526879f8d2a_add’ redeclared as function
cgo-generated-wrappers:4:5: note: previously declared here
lto1: fatal error: errors during merging of translation units
compilation terminated.
lto-wrapper: fatal error: /usr/bin/gcc returned 1 exit status
compilation terminated.
/usr/bin/ld: error: lto-wrapper failed
collect2: error: ld returned 1 exit status

The interesting aspect to note is the redeclaration of the add symbol in addition to the previously encountered _cgoexp*_add symbol. This comes from introducing the assignment statement of C.add to the variable fn, and causes cgo to emit an additional pair of matching symbols with conflicting types via another code path.
@seankhliao seankhliao added NeedsFix The path to resolution is known, but the work has not been done. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed NeedsFix The path to resolution is known, but the work has not been done. labels Jan 21, 2021
@ianlancetaylor ianlancetaylor added this to the Backlog milestone Jan 21, 2021
@lsm5 lsm5 mentioned this issue Jan 27, 2021
Closed
dbenoit17 added a commit to dbenoit17/go-1 that referenced this issue Feb 17, 2021
@dbenoit17
cmd/cgo: use -fno-lto in cflags when compiling _cgo_export.c
6a347be 
Currently, cgo declares different types for exported symbols between
_cgo_main.c and _cgo_export.c.  This is not compatible with link time
optimization in GCC and causes builds to fail when compiling with
CGO_CFLAGS="-flto -ffat-lto-objects".

This commit appends -fno-lto to the cflags when building _cgo_export.c
to disable link time optimization locally on the built object.

Fixes golang#43830
dbenoit17 added a commit to dbenoit17/go-1 that referenced this issue Feb 17, 2021
@dbenoit17
cmd/cgo: use -fno-lto in cflags when compiling _cgo_export.c
4f6bb06 
Currently, cgo declares different types for exported symbols between
_cgo_main.c and _cgo_export.c.  This is not compatible with link time
optimization in GCC and causes builds to fail when compiling with
CGO_CFLAGS="-flto -ffat-lto-objects".

This commit appends -fno-lto to the cflags when building _cgo_export.c
to disable link time optimization locally on the built object.

Fixes golang#43830
@dbenoit17 dbenoit17 mentioned this issue Feb 17, 2021
Closed
@gopherbot
Copy link

Change https://golang.org/cl/292990 mentions this issue: cmd/cgo: use -fno-lto in cflags when compiling _cgo_export.c

@derekparker derekparker mentioned this issue Feb 17, 2021
Closed
@gopherbot
Copy link

Change https://golang.org/cl/293290 mentions this issue: cmd/cgo: Define exported Go func as function

derekparker added a commit to derekparker/go that referenced this issue Feb 22, 2021
@derekparker
cmd/cgo: Fix CGO related LTO bug
f3b2c46 
This patch fixes another CGO related LTO bug. This seems to come
when the programmer tries to assign a C function pointer to a local Go
variable as seen in the reproducer. It turns out we do not need to emit
_cgo_hack or our own definition of the symbol(s) within the _cgo_main.c
file as was previously done.

Fixes golang#43830
@ianlancetaylor ianlancetaylor mentioned this issue Mar 28, 2021
Closed
@mwhudson
Copy link
Contributor

So I think the linked change is half right, basically.

@gopherbot
Copy link

Change https://golang.org/cl/322614 mentions this issue: cmd/link, cmd/cgo: support -flto in CFLAGS

@gopherbot
Copy link

Change https://golang.org/cl/324009 mentions this issue: cmd/go: add declaration to cgo_lto_issue43830 test

@gopherbot
Copy link

Change https://golang.org/cl/324010 mentions this issue: cmd/go: don't run LTO tests on AIX

gopherbot pushed a commit that referenced this issue Jun 1, 2021
@ianlancetaylor
cmd/go: add declaration to cgo_lto_issue43830 test
567ee86 
This permits the test to work in C99 mode.

For #43830

Change-Id: Ide54bd62239cfe602e2664300f04e472df5daf43
Reviewed-on: https://go-review.googlesource.com/c/go/+/324009
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/324049 mentions this issue: cmd/link: move issue 43830 tests out of TestScript

gopherbot pushed a commit that referenced this issue Jun 2, 2021
@ianlancetaylor
cmd/link: move issue 43830 tests out of TestScript
84c0e5d 
These tests pass or fail depending on the exact compiler version,
which the TestScript tests don't support. Rewrite into Go.

For #43830
For #46295

Change-Id: I91b61dfe329d518e461ee56f186f0e9b42858e77
Reviewed-on: https://go-review.googlesource.com/c/go/+/324049
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
@daurnimator daurnimator mentioned this issue Dec 28, 2021
Closed
@golang golang locked and limited conversation to collaborators Jun 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge 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
Backlog
5 participants
@mwhudson @ianlancetaylor @gopherbot @seankhliao @dbenoit17