]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/link/internal/ld: dedup shared libraries on openbsd
authorJoel Sing <joel@sing.id.au>
Wed, 11 Nov 2020 17:30:15 +0000 (04:30 +1100)
committerJoel Sing <joel@sing.id.au>
Mon, 16 Nov 2020 18:39:54 +0000 (18:39 +0000)
When linking internally on OpenBSD, dedup libraries treating versioned
and unversioned libraries as equivalents. Versioned libraries are preferred
and are retained over unversioned libraries.

This avoids the situation where the use of cgo results in a DT_NEEDED for a
versioned library (for example, libc.so.96.1), while a dynamic import
specifies an unversioned library (for example, libc.so). Without deduplication
this would result in two DT_NEEDED entries, causing a failure when ld.so
attempts to load the Go binrary.

Updates #36435
Fixes #39257

Change-Id: I4a4942f259dece01d97bb51df9e13d67c9f94d34
Reviewed-on: https://go-review.googlesource.com/c/go/+/249978
Trust: Joel Sing <joel@sing.id.au>
Run-TryBot: Joel Sing <joel@sing.id.au>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
src/cmd/link/internal/ld/elf_test.go
src/cmd/link/internal/ld/go.go
src/cmd/link/internal/ld/go_test.go [new file with mode: 0644]
src/cmd/link/internal/ld/testdata/issue39256/x.go

index 37f0e77336b2f5047b1bd9ca2c9277055a0e828f..776fc1b4f9793245f7704474e641efe32ad18e6b 100644 (file)
@@ -14,6 +14,7 @@ import (
        "os/exec"
        "path/filepath"
        "runtime"
+       "strings"
        "testing"
 )
 
@@ -123,7 +124,7 @@ func TestNoDuplicateNeededEntries(t *testing.T) {
 
        var count int
        for _, lib := range libs {
-               if lib == "libc.so" {
+               if lib == "libc.so" || strings.HasPrefix(lib, "libc.so.") {
                        count++
                }
        }
index a6cd4c05413af1d5b288c27ed714dafa260a0ba0..fbc7a78d0ed299b98811419fb77c32366e3c96c0 100644 (file)
@@ -18,6 +18,8 @@ import (
        "fmt"
        "io"
        "os"
+       "sort"
+       "strconv"
        "strings"
 )
 
@@ -289,6 +291,62 @@ func setCgoAttr(ctxt *Link, lookup func(string, int) loader.Sym, file string, pk
        return
 }
 
+// openbsdTrimLibVersion indicates whether a shared library is
+// versioned and if it is, returns the unversioned name. The
+// OpenBSD library naming scheme is lib<name>.so.<major>.<minor>
+func openbsdTrimLibVersion(lib string) (string, bool) {
+       parts := strings.Split(lib, ".")
+       if len(parts) != 4 {
+               return "", false
+       }
+       if parts[1] != "so" {
+               return "", false
+       }
+       if _, err := strconv.Atoi(parts[2]); err != nil {
+               return "", false
+       }
+       if _, err := strconv.Atoi(parts[3]); err != nil {
+               return "", false
+       }
+       return fmt.Sprintf("%s.%s", parts[0], parts[1]), true
+}
+
+// dedupLibrariesOpenBSD dedups a list of shared libraries, treating versioned
+// and unversioned libraries as equivalents. Versioned libraries are preferred
+// and retained over unversioned libraries. This avoids the situation where
+// the use of cgo results in a DT_NEEDED for a versioned library (for example,
+// libc.so.96.1), while a dynamic import specifies an unversioned library (for
+// example, libc.so) - this would otherwise result in two DT_NEEDED entries
+// for the same library, resulting in a failure when ld.so attempts to load
+// the Go binary.
+func dedupLibrariesOpenBSD(ctxt *Link, libs []string) []string {
+       libraries := make(map[string]string)
+       for _, lib := range libs {
+               if name, ok := openbsdTrimLibVersion(lib); ok {
+                       // Record unversioned name as seen.
+                       seenlib[name] = true
+                       libraries[name] = lib
+               } else if _, ok := libraries[lib]; !ok {
+                       libraries[lib] = lib
+               }
+       }
+
+       libs = nil
+       for _, lib := range libraries {
+               libs = append(libs, lib)
+       }
+       sort.Strings(libs)
+
+       return libs
+}
+
+func dedupLibraries(ctxt *Link, libs []string) []string {
+       if ctxt.Target.IsOpenbsd() {
+               return dedupLibrariesOpenBSD(ctxt, libs)
+       }
+       return libs
+}
+
 var seenlib = make(map[string]bool)
 
 func adddynlib(ctxt *Link, lib string) {
@@ -385,7 +443,7 @@ func (ctxt *Link) addexport() {
        for _, exp := range ctxt.dynexp {
                Adddynsym(ctxt.loader, &ctxt.Target, &ctxt.ArchSyms, exp)
        }
-       for _, lib := range dynlib {
+       for _, lib := range dedupLibraries(ctxt, dynlib) {
                adddynlib(ctxt, lib)
        }
 }
diff --git a/src/cmd/link/internal/ld/go_test.go b/src/cmd/link/internal/ld/go_test.go
new file mode 100644 (file)
index 0000000..0197196
--- /dev/null
@@ -0,0 +1,121 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package ld
+
+import (
+       "cmd/internal/objabi"
+       "internal/testenv"
+       "io/ioutil"
+       "os"
+       "os/exec"
+       "path/filepath"
+       "reflect"
+       "runtime"
+       "testing"
+)
+
+func TestDedupLibraries(t *testing.T) {
+       ctxt := &Link{}
+       ctxt.Target.HeadType = objabi.Hlinux
+
+       libs := []string{"libc.so", "libc.so.6"}
+
+       got := dedupLibraries(ctxt, libs)
+       if !reflect.DeepEqual(got, libs) {
+               t.Errorf("dedupLibraries(%v) = %v, want %v", libs, got, libs)
+       }
+}
+
+func TestDedupLibrariesOpenBSD(t *testing.T) {
+       ctxt := &Link{}
+       ctxt.Target.HeadType = objabi.Hopenbsd
+
+       tests := []struct {
+               libs []string
+               want []string
+       }{
+               {
+                       libs: []string{"libc.so"},
+                       want: []string{"libc.so"},
+               },
+               {
+                       libs: []string{"libc.so", "libc.so.96.1"},
+                       want: []string{"libc.so.96.1"},
+               },
+               {
+                       libs: []string{"libc.so.96.1", "libc.so"},
+                       want: []string{"libc.so.96.1"},
+               },
+               {
+                       libs: []string{"libc.a", "libc.so.96.1"},
+                       want: []string{"libc.a", "libc.so.96.1"},
+               },
+               {
+                       libs: []string{"libpthread.so", "libc.so"},
+                       want: []string{"libc.so", "libpthread.so"},
+               },
+               {
+                       libs: []string{"libpthread.so.26.1", "libpthread.so", "libc.so.96.1", "libc.so"},
+                       want: []string{"libc.so.96.1", "libpthread.so.26.1"},
+               },
+               {
+                       libs: []string{"libpthread.so.26.1", "libpthread.so", "libc.so.96.1", "libc.so", "libfoo.so"},
+                       want: []string{"libc.so.96.1", "libfoo.so", "libpthread.so.26.1"},
+               },
+       }
+
+       for _, test := range tests {
+               t.Run("dedup", func(t *testing.T) {
+                       got := dedupLibraries(ctxt, test.libs)
+                       if !reflect.DeepEqual(got, test.want) {
+                               t.Errorf("dedupLibraries(%v) = %v, want %v", test.libs, got, test.want)
+                       }
+               })
+       }
+}
+
+func TestDedupLibrariesOpenBSDLink(t *testing.T) {
+       // The behavior we're checking for is of interest only on OpenBSD.
+       if runtime.GOOS != "openbsd" {
+               t.Skip("test only useful on openbsd")
+       }
+
+       testenv.MustHaveGoBuild(t)
+       testenv.MustHaveCGO(t)
+       t.Parallel()
+
+       dir, err := ioutil.TempDir("", "dedup-build")
+       if err != nil {
+               t.Fatal(err)
+       }
+       defer os.RemoveAll(dir)
+
+       // cgo_import_dynamic both the unversioned libraries and pull in the
+       // net package to get a cgo package with a versioned library.
+       srcFile := filepath.Join(dir, "x.go")
+       src := `package main
+
+import (
+       _ "net"
+)
+
+//go:cgo_import_dynamic _ _ "libc.so"
+
+func main() {}`
+       if err := ioutil.WriteFile(srcFile, []byte(src), 0644); err != nil {
+               t.Fatal(err)
+       }
+
+       exe := filepath.Join(dir, "deduped.exe")
+       out, err := exec.Command(testenv.GoToolPath(t), "build", "-o", exe, srcFile).CombinedOutput()
+       if err != nil {
+               t.Fatalf("build failure: %s\n%s\n", err, string(out))
+       }
+
+       // Result should be runnable.
+       if _, err = exec.Command(exe).CombinedOutput(); err != nil {
+               t.Fatal(err)
+       }
+}
index d8562ad1720d1e17a126dfdc1a4f48f259adba8d..97bc1cc4078565c371a9ebb68faf08740f21dead 100644 (file)
@@ -13,6 +13,8 @@ import (
 //go:cgo_import_dynamic libc_close close "libc.so"
 //go:cgo_import_dynamic libc_open open "libc.so"
 
+//go:cgo_import_dynamic _ _ "libc.so"
+
 func trampoline()
 
 func main() {