}
}
+ // Scrutinize CFLAGS and related for flags that might cause
+ // problems if we are using internal linking (for example, use of
+ // plugins, LTO, etc) by calling a helper routine that builds on
+ // the existing CGO flags allow-lists. If we see anything
+ // suspicious, emit a special token file "preferlinkext" (known to
+ // the linker) in the object file to signal the that it should not
+ // try to link internally and should revert to external linking.
+ // The token we pass is a suggestion, not a mandate; if a user is
+ // explicitly asking for a specific linkmode via the "-linkmode"
+ // flag, the token will be ignored. NB: in theory we could ditch
+ // the token approach and just pass a flag to the linker when we
+ // eventually invoke it, and the linker flag could then be
+ // documented (although coming up with a simple explanation of the
+ // flag might be challenging). For more context see issues #58619,
+ // #58620, and #58848.
+ flagSources := []string{"CGO_CFLAGS", "CGO_CXXFLAGS", "CGO_FFLAGS"}
+ flagLists := [][]string{cgoCFLAGS, cgoCXXFLAGS, cgoFFLAGS}
+ if flagsNotCompatibleWithInternalLinking(flagSources, flagLists) {
+ tokenFile := objdir + "preferlinkext"
+ if cfg.BuildN || cfg.BuildX {
+ b.Showcmd("", "echo > %s", tokenFile)
+ }
+ if !cfg.BuildN {
+ if err := os.WriteFile(tokenFile, nil, 0666); err != nil {
+ return nil, nil, err
+ }
+ }
+ outObj = append(outObj, tokenFile)
+ }
+
if cfg.BuildMSan {
cgoCFLAGS = append([]string{"-fsanitize=memory"}, cgoCFLAGS...)
cgoLDFLAGS = append([]string{"-fsanitize=memory"}, cgoLDFLAGS...)
return outGo, outObj, nil
}
+// flagsNotCompatibleWithInternalLinking scans the list of cgo
+// compiler flags (C/C++/Fortran) looking for flags that might cause
+// problems if the build in question uses internal linking. The
+// primary culprits are use of plugins or use of LTO, but we err on
+// the side of caution, supporting only those flags that are on the
+// allow-list for safe flags from security perspective. Return is TRUE
+// if a sensitive flag is found, FALSE otherwise.
+func flagsNotCompatibleWithInternalLinking(sourceList []string, flagListList [][]string) bool {
+ for i := range sourceList {
+ sn := sourceList[i]
+ fll := flagListList[i]
+ if err := checkCompilerFlagsForInternalLink(sn, sn, fll); err != nil {
+ return true
+ }
+ }
+ return false
+}
+
// dynimport creates a Go source file named importGo containing
// //go:cgo_import_dynamic directives for each symbol or library
// dynamically imported by the object files outObj.
}
func checkCompilerFlags(name, source string, list []string) error {
- return checkFlags(name, source, list, validCompilerFlags, validCompilerFlagsWithNextArg)
+ checkOverrides := true
+ return checkFlags(name, source, list, validCompilerFlags, validCompilerFlagsWithNextArg, checkOverrides)
}
func checkLinkerFlags(name, source string, list []string) error {
- return checkFlags(name, source, list, validLinkerFlags, validLinkerFlagsWithNextArg)
+ checkOverrides := true
+ return checkFlags(name, source, list, validLinkerFlags, validLinkerFlagsWithNextArg, checkOverrides)
}
-func checkFlags(name, source string, list []string, valid []*lazyregexp.Regexp, validNext []string) error {
+// checkCompilerFlagsForInternalLink returns an error if 'list'
+// contains a flag or flags that may not be fully supported by
+// internal linking (meaning that we should punt the link to the
+// external linker).
+func checkCompilerFlagsForInternalLink(name, source string, list []string) error {
+ checkOverrides := false
+ if err := checkFlags(name, source, list, validCompilerFlags, validCompilerFlagsWithNextArg, checkOverrides); err != nil {
+ return err
+ }
+ // Currently the only flag on the allow list that causes problems
+ // for the linker is "-flto"; check for it manually here.
+ for _, fl := range list {
+ if strings.HasPrefix(fl, "-flto") {
+ return fmt.Errorf("flag %q triggers external linking", fl)
+ }
+ }
+ return nil
+}
+
+func checkFlags(name, source string, list []string, valid []*lazyregexp.Regexp, validNext []string, checkOverrides bool) error {
// Let users override rules with $CGO_CFLAGS_ALLOW, $CGO_CFLAGS_DISALLOW, etc.
var (
allow *regexp.Regexp
disallow *regexp.Regexp
)
- if env := cfg.Getenv("CGO_" + name + "_ALLOW"); env != "" {
- r, err := regexp.Compile(env)
- if err != nil {
- return fmt.Errorf("parsing $CGO_%s_ALLOW: %v", name, err)
+ if checkOverrides {
+ if env := cfg.Getenv("CGO_" + name + "_ALLOW"); env != "" {
+ r, err := regexp.Compile(env)
+ if err != nil {
+ return fmt.Errorf("parsing $CGO_%s_ALLOW: %v", name, err)
+ }
+ allow = r
}
- allow = r
- }
- if env := cfg.Getenv("CGO_" + name + "_DISALLOW"); env != "" {
- r, err := regexp.Compile(env)
- if err != nil {
- return fmt.Errorf("parsing $CGO_%s_DISALLOW: %v", name, err)
+ if env := cfg.Getenv("CGO_" + name + "_DISALLOW"); env != "" {
+ r, err := regexp.Compile(env)
+ if err != nil {
+ return fmt.Errorf("parsing $CGO_%s_DISALLOW: %v", name, err)
+ }
+ disallow = r
}
- disallow = r
}
Args:
import (
"os"
+ "strings"
"testing"
)
{"-Wp,-Ufoo"},
{"-Wp,-Dfoo1"},
{"-Wp,-Ufoo1"},
+ {"-flto"},
{"-fobjc-arc"},
{"-fno-objc-arc"},
{"-fomit-frame-pointer"},
t.Fatalf("missing error for -fplugin=lint.so: %v", err)
}
}
+
+func TestCheckCompilerFlagsForInternalLink(t *testing.T) {
+ // Any "bad" compiler flag should trigger external linking.
+ for _, f := range badCompilerFlags {
+ if err := checkCompilerFlagsForInternalLink("test", "test", f); err == nil {
+ t.Errorf("missing error for %q", f)
+ }
+ }
+
+ // All "good" compiler flags should not trigger external linking,
+ // except for anything that begins with "-flto".
+ for _, f := range goodCompilerFlags {
+ foundLTO := false
+ for _, s := range f {
+ if strings.Contains(s, "-flto") {
+ foundLTO = true
+ }
+ }
+ if err := checkCompilerFlagsForInternalLink("test", "test", f); err != nil {
+ // expect error for -flto
+ if !foundLTO {
+ t.Errorf("unexpected error for %q: %v", f, err)
+ }
+ } else {
+ // expect no error for everything else
+ if foundLTO {
+ t.Errorf("missing error for %q: %v", f, err)
+ }
+ }
+ }
+}
add("link", lazyBool("testenv.HasLink()", testenv.HasLink))
add("mismatched-goroot", script.Condition("test's GOROOT_FINAL does not match the real GOROOT", isMismatchedGoroot))
add("msan", sysCondition("-msan", platform.MSanSupported, true))
+ add("cgolinkext", script.BoolCondition("platform requires external linking for cgo", platform.MustLinkExternal(cfg.Goos, cfg.Goarch, true)))
add("net", lazyBool("testenv.HasExternalNetwork()", testenv.HasExternalNetwork))
add("race", sysCondition("-race", platform.RaceDetectorSupported, true))
add("symlink", lazyBool("testenv.HasSymlink()", testenv.HasSymlink))
$WORK filesystem is case-sensitive
[cgo]
host CGO_ENABLED
+[cgolinkext]
+ platform requires external linking for cgo
[compiler:*]
runtime.Compiler == <suffix>
[cross]
--- /dev/null
+# Test case to verify that when we have a package that uses CGO in
+# combination with selected "unusual" flags (involving plugins, LTO)
+# that we force external linking. See related
+# issues 58619, 58620, and 58848.
+
+[compiler:gccgo] skip # only external linking for gccgo
+
+# This test requires external linking, so use cgo as a proxy
+[!cgo] skip
+
+# Here we build three program: one with explicit CGO use, one with no
+# CGO use, and one that uses a stdlib package ("runtime/cgo") that has
+# CGO in it. It used to be that only the explicit use of CGO would
+# trigger external linking, and that the program that only used
+# "runtime/cgo" would always be handled with internal linking. This caused
+# issues when users included odd/unusual flags (ex: -fplugin, -flto)
+# in CGO_CFLAGS, causing the Go linker to have to read and interpret
+# non-standard host objects.
+#
+# As of 1.21 we continue to use internal linking for programs whose
+# CGO use comes ony from stdlib packages in the absence of any flag
+# funny business, however if the Go command sees flags that may be suspicious,
+# it signals the Go linker to invoke the external linker.
+
+# The next few tests run builds passing "-n" to the Go command, then
+# checking the output to see if the Go command is trying to pass a
+# "preferlinkext" token to the linker to request external linking.
+
+#-----------------------
+
+# Use a fresh GOCACHE for these next steps, so as to have the real
+# actions for the runtime/cgo package appear in the "-n -x" output.
+env GOCACHE=$WORK/gocache
+mkdir $GOCACHE
+
+# First build: there is no CGO in use, so no token should be present regardless
+# of weird CGO flags.
+go build -x -n -o dummy.exe ./noUseOfCgo
+! stderr preferlinkext
+env CGO_CFLAGS=-flto
+go build -x -n -o dummy.exe ./noUseOfCgo
+! stderr preferlinkext
+env CGO_CFLAGS=
+
+# Second build uses CGO, so we expect to see the token present in the
+# -n output only when strange flags are used.
+go build -x -n -o dummy.exe ./usesInternalCgo
+! stderr preferlinkext
+env CGO_CFLAGS=-flto
+go build -x -n -o dummy.exe ./usesInternalCgo
+stderr preferlinkext
+env CGO_CFLAGS=-fplugin
+go build -x -n -o dummy.exe ./usesInternalCgo
+stderr preferlinkext
+env CGO_CFLAGS=-fprofile-instr-generate
+go build -x -n -o dummy.exe ./usesInternalCgo
+stderr preferlinkext
+env CGO_CFLAGS=
+
+[short] skip
+
+# In the remaining tests below we do actual builds (without -n) to
+# verify that the Go linker is going the right thing in addition to the
+# Go command. Here the idea is to pass "-tmpdir" to the linker, then
+# check after the link is done for the presence of the file
+# <tmpdir>/go.o, which the Go linker creates prior to kicking off the
+# external linker.
+
+mkdir tmp1
+mkdir tmp2
+mkdir tmp3
+mkdir tmp4
+mkdir tmp5
+
+# First build: no external linking expected
+go build -ldflags=-tmpdir=tmp1 -o $devnull ./noUseOfCgo &
+
+# Second build: using only "runtime/cgo", expect internal linking.
+go build -ldflags=-tmpdir=tmp2 -o $devnull ./usesInternalCgo &
+
+# Third build: program uses only "runtime/cgo", so we would normally
+# expect internal linking, except that cflags contain suspicious entries
+# (in this case, a flag that does not appear on the allow list).
+env CGO_CFLAGS=-fmerge-all-constants
+env CGO_LDFLAGS=-fmerge-all-constants
+go build -ldflags=-tmpdir=tmp3 -o $devnull ./usesInternalCgo &
+env CGO_CFLAGS=
+env CGO_LDFLAGS=
+
+# Fourth build: explicit CGO, expect external linking.
+go build -ldflags=-tmpdir=tmp4 -o $devnull ./usesExplicitCgo &
+
+# Fifth build: explicit CGO, but we specifically asked for internal linking
+# via a flag, so using internal linking it is.
+[cgolinkext] go list ./usesInternalCgo
+[!cgolinkext] go build '-ldflags=-tmpdir=tmp5 -linkmode=internal' -o $devnull ./usesInternalCgo &
+
+wait
+
+# Check first build: no external linking expected
+! exists tmp1/go.o
+
+# Check second build: using only "runtime/cgo", expect internal linking.
+[!cgolinkext] ! exists tmp2/go.o
+[cgolinkext] exists tmp2/go.o
+
+# Check third build: has suspicious flag.
+exists tmp3/go.o
+
+# Fourth build: explicit CGO, expect external linking.
+exists tmp4/go.o
+
+# Fifth build: explicit CGO, -linkmode=internal.
+! exists tmp5/go.o
+
+-- go.mod --
+
+module cgo.example
+
+go 1.20
+
+-- noUseOfCgo/main.go --
+
+package main
+
+func main() {
+ println("clean as a whistle")
+}
+
+-- usesInternalCgo/main.go --
+
+package main
+
+import (
+ "runtime/cgo"
+)
+
+func main() {
+ q := "hello"
+ h := cgo.NewHandle(q)
+ h.Delete()
+}
+
+-- usesExplicitCgo/main.go --
+
+package main
+
+/*
+int meaningOfLife() { return 42; }
+*/
+import "C"
+
+func main() {
+ println(C.meaningOfLife())
+}
ctxt.LinkMode = LinkExternal
via = "via GO_EXTLINK_ENABLED "
default:
- if extNeeded || (iscgo && externalobj) {
+ preferExternal := len(preferlinkext) != 0
+ if preferExternal && ctxt.Debugvlog > 0 {
+ ctxt.Logf("external linking prefer list is %v\n", preferlinkext)
+ }
+ if extNeeded || (iscgo && (externalobj || preferExternal)) {
ctxt.LinkMode = LinkExternal
} else {
ctxt.LinkMode = LinkInternal
// any of these objects, we must link externally. Issue 52863.
dynimportfail []string
+ // preferlinkext is a list of packages for which the Go command
+ // noticed use of peculiar C flags. If we see any of these,
+ // default to linking externally unless overridden by the
+ // user. See issues #58619, #58620, and #58848.
+ preferlinkext []string
+
// unknownObjFormat is set to true if we see an object whose
// format we don't recognize.
unknownObjFormat = false
if arhdr.name == "dynimportfail" {
dynimportfail = append(dynimportfail, lib.Pkg)
}
+ if arhdr.name == "preferlinkext" {
+ // Ignore this directive if -linkmode has been
+ // set explicitly.
+ if ctxt.LinkMode == LinkAuto {
+ preferlinkext = append(preferlinkext, lib.Pkg)
+ }
+ }
// Skip other special (non-object-file) sections that
// build tools may have added. Such sections must have