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: scoping of directives changed in 1.19 #58438

Closed
rittneje opened this issue Feb 9, 2023 · 1 comment
Closed

cmd/cgo: scoping of directives changed in 1.19 #58438

rittneje opened this issue Feb 9, 2023 · 1 comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge

Comments

@rittneje
Copy link

rittneje commented Feb 9, 2023

Consider the following test module.

cgotest/go.mod

module cgotest

go 1.18

cgotest/main.go

package main

import _ "cgotest/subpackage"

/*
#include <sqlite3.h>
*/
import "C"

func main() {
	C.sqlite3_open_v2(nil, nil, 0, nil)
}

cgotest/subpackage/c.go

package subpackage

/*
#cgo LDFLAGS: -lsqlite3
*/
import "C"

In Go 1.18, this fails at link time.

$ go version
go version go1.18.10 darwin/amd64
$ go build main.go 
# command-line-arguments
Undefined symbols for architecture x86_64:
  "_sqlite3_open_v2", referenced from:
      __cgo_6884829902c1_Cfunc_sqlite3_open_v2 in _x002.o
     (maybe you meant: __cgo_6884829902c1_Cfunc_sqlite3_open_v2)
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

In Go 1.19 and 1.20, it compiles successfully. That implies that Go 1.19 changed the scoping of #cgo directives from package-scope to module-scope (or something like that).

However, this was not mentioned in the 1.19 or 1.20 release notes. Furthermore, the cgo docs only mention packages, not modules or sub-packages.

If this change was intentional, then this is a request to update the documentation.
If this change was not intentional, then this is a bug report.
@ianlancetaylor ianlancetaylor changed the title cgo: scoping of directives changed in 1.19 cmd/cgo: scoping of directives changed in 1.19 Feb 9, 2023
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Feb 9, 2023
@ianlancetaylor
Copy link
Contributor

The scope of the directives didn't change. A #cgo LDFLAGS directive has always applied to the final link step that links all the imported packages together. Nothing else makes sense for LDFLAGS.

The failure in 1.18 is due to a different problem: trying to compute the dynamic imports, which are required for internal linking, failed because for that case we didn't consider the LDFLAGS for imported packages, only the LDFLAGS for the current package. That is technically wrong but usually worked, because most packages don't rely on picking up LDFLAGS from an imported package.

However, since that was only required for internal linking, which is not the default for packages that use cgo, we changed things so that if we can't compute the dynamic imports we ignore the failure and simply require external linking. That was #52863.

This didn't make it into the release notes because we don't normally mention simple bug fixes in the release notes.

Closing this issue because there is nothing to do.

@ianlancetaylor ianlancetaylor closed this as not planned Won't fix, can't repro, duplicate, stale Feb 9, 2023
@dr2chase dr2chase added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Feb 10, 2023
@emersion emersion mentioned this issue Feb 17, 2023
Open
emersion added a commit to emersion/soju that referenced this issue Feb 22, 2023
@emersion
Bump minimum Go version to 1.19
522f8a8 
Two reasons:

- Because of [1], when mattn/go-sqlite3 is used without libsqlite3,
  we need Go 1.19.
- atomic.Int64 has been introduced in Go 1.19.

[1]: golang/go#58438
@golang golang locked and limited conversation to collaborators Feb 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@dr2chase @ianlancetaylor @gopherbot @rittneje