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

cmd/go: new cgo protections mix up CFLAGS and LDFLAGS from pkg-config #23737

Closed
jeddenlea opened this issue Feb 8, 2018 · 6 comments
Closed

cmd/go: new cgo protections mix up CFLAGS and LDFLAGS from pkg-config #23737

jeddenlea opened this issue Feb 8, 2018 · 6 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
Go1.10

Comments

@jeddenlea
Copy link
Contributor

jeddenlea commented Feb 8, 2018 •
edited

Please answer these questions before submitting your issue. Thanks!

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

1.9.4

Does this issue reproduce with the latest release?

Yes.

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

Linux

What did you do?

Tried to build with cgo, like

#cgo pkg-config: foo

Within our contrived environment, we'll see,

$ $PKG_CONFIG --cflags foo
-pthread -I... -I... -I... -I... -I...
$ $PKG_CONFIG --libs foo
-L... -L... -L... -lfoo -lbar -Wl,--export-dynamic ...

What did you expect to see?

The linker flag -Wl,--export-dynamic there is rightfully rejected. It ought to be allowed with the CGO_LDFLAGS_ALLOW environment variable.

What did you see instead?

go build returned an error,

go build ...: invalid flag in pkg-config --cflags: -Wl,--export-dynamic

And, it may be built, but using CGO_CFLAGS_ALLOW, not CGO_LDFLAGS_ALLOW

I'll have a fix pushed shortly.
@gopherbot
Copy link

Change https://golang.org/cl/92755 mentions this issue: cmd/go: fix CGO_LDFLAGS_ALLOW check

@dgryski
Copy link
Contributor

dgryski commented Feb 8, 2018

/cc @rsc

@jeddenlea jeddenlea changed the title New cgo protections mix up CFLAGS and LDFLAGS New cgo protections mix up CFLAGS and LDFLAGS from pkg-config Feb 8, 2018
@ianlancetaylor ianlancetaylor added this to the Go1.10 milestone Feb 8, 2018
@ianlancetaylor ianlancetaylor added NeedsFix The path to resolution is known, but the work has not been done. release-blocker labels Feb 8, 2018
@ianlancetaylor ianlancetaylor changed the title New cgo protections mix up CFLAGS and LDFLAGS from pkg-config cmd/go: new cgo protections mix up CFLAGS and LDFLAGS from pkg-config Feb 8, 2018
@raff
Copy link

raff commented Feb 8, 2018

I get a similar error with a package (LuaJIT) that does the following in order to link a *.a file:
#cgo LDFLAGS: ${SRCDIR}/../../../LuaJIT/LuaJIT/src/libluajit.a -lm -ldl

I was able to "fix" it by doing this, but it would be good if it was at least documented (also, I am wondering if there are any other cases if trying to pass non-flag parameters in CFLAGS/LDFLAGS):
#cgo LDFLAGS: -lm -ldl -L${SRCDIR}/../../../LuaJIT/LuaJIT/src -lluajit

@FiloSottile
Copy link
Contributor

@raff yours seems like a separate issue, where this line is missing support for .a files:

go/src/cmd/go/internal/work/security.go

Line 91 in 104445e

re(`[a-zA-Z0-9_].*\.(o|obj|dll|dylib|so)`), // direct linker inputs: x.o or libfoo.so (but not -foo.o or @foo.o)

I submitted a CL for that. https://golang.org/cl/92855

@gopherbot
Copy link

Change https://golang.org/cl/92855 mentions this issue: cmd/go: add .a files to allowed linker inputs

@ianlancetaylor ianlancetaylor mentioned this issue Feb 8, 2018
Closed
@ianlancetaylor
Copy link
Contributor

Closing in favor of a general whitelist issue in #23749.

@golang golang locked and limited conversation to collaborators Feb 8, 2019
@rsc rsc removed their assignment Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Milestone
Go1.10
Development

No branches or pull requests
7 participants
@raff @rsc @dgryski @FiloSottile @jeddenlea @ianlancetaylor @gopherbot