Skip to content

Commit

Permalink
Filter out -lstdc++ for targets with no C++ code (#1686)
Browse files Browse the repository at this point in the history
This avoids an unnecessary run-time dependency on libstdc++.so, which
may not be present in container environments.

go build only adds this dependency if C++ code is included in the
build. We now filter out this dependency from the cc toolchain flags
unless we have C++/ObjC source or we have a dependency on a cc_library
or objc_library.

Fixes #1681
Related bazelbuild/bazel#2954
  • Loading branch information
jayconrod authored and Jay Conrod committed Sep 5, 2018
1 parent 3a5a0ff commit 4c45fcd
Show file tree
Hide file tree
Showing 9 changed files with 277 additions and 2 deletions.
6 changes: 5 additions & 1 deletion go/private/actions/link.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,11 @@ def emit_link(
config_strip = len(go._ctx.configuration.bin_dir.path) + 1
pkg_depth = executable.dirname[config_strip:].count("/") + 1

extldflags = list(go.cgo_tools.linker_options)
# Exclude -lstdc++ from link options. We don't want to link against it
# unless we actually have some C++ code. _cgo_codegen will include it
# in archives via CGO_LDFLAGS if it's needed.
extldflags = [f for f in go.cgo_tools.linker_options if f not in ("-lstdc++", "-lc++")]

if go.coverage_enabled:
extldflags.append("--coverage")
gc_linkopts, extldflags = _extract_extldflags(gc_linkopts, extldflags)
Expand Down
6 changes: 6 additions & 0 deletions go/private/rules/cgo.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,12 @@ def _cgo_codegen_impl(ctx):
objc_outs.append(gen_file)
builder_args.add_all(["-src", gen_file.path + "=" + src.path])

# Filter out -lstdc++ in CGO_LDFLAGS if we don't have any C++ code. This
# also gets filtered out in link.bzl.
have_cc = len(source.cxx) + len(source.objc) + len(ctx.attr.deps) > 0
if not have_cc:
linkopts = [o for o in linkopts if o not in ("-lstdc++", "-lc++")]

tool_args.add_all(["-objdir", out_dir])

inputs = sets.union(ctx.files.srcs, go.crosstool, go.sdk.tools, go.stdlib.libs)
Expand Down
58 changes: 57 additions & 1 deletion tests/core/cgo/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library", "go_test")

go_test(
name = "opts_test",
Expand Down Expand Up @@ -68,6 +68,62 @@ genrule(
tags = ["manual"],
)

go_test(
name = "cc_libs_test",
srcs = [
"cc_libs_darwin_test.go",
"cc_libs_linux_test.go",
],
data = [
":c_srcs",
":cc_deps",
":cc_srcs",
":pure",
],
deps = ["//go/tools/bazel:go_default_library"],
)

go_binary(
name = "pure",
srcs = ["main.go"],
out = "pure_bin",
cgo = True,
pure = "on",
)

go_binary(
name = "c_srcs",
srcs = [
"foo.c",
"main.go",
],
out = "c_srcs_bin",
cgo = True,
)

go_binary(
name = "cc_srcs",
srcs = [
"bar.cc",
"main.go",
],
out = "cc_srcs_bin",
cgo = True,
)

go_binary(
name = "cc_deps",
srcs = ["main.go"],
out = "cc_deps_bin",
cdeps = [":bar_dep"],
cgo = True,
)

cc_library(
name = "bar_dep",
srcs = ["bar.cc"],
)

go_test(
name = "race_test",
srcs = [
Expand Down
12 changes: 12 additions & 0 deletions tests/core/cgo/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,18 @@ Checks that Go binaries can link against dynamic C libraries. Some libraries
(especially those provided with ``cc_import``) may only have dynamic versions,
and we should be able to link against them and find them at run-time.

cc_libs_test
------------

Checks that Go binaries that include cgo code may or may not link against
libstdc++, depending on how they're linked. This tests several binaries:

* ``pure_bin`` - built in ``"pure"`` mode, should not depend on libstdc++.
* ``c_srcs`` - has no C++ code in sources, should not depend on libstdc++.
* ``cc_srcs`` - has some C++ code in sources, should depend on libstdc++.
* ``cc_deps`` - depends on a ``cc_library``, should depend on libstdc++
because we don't know what's in it.

race_test
---------

Expand Down
5 changes: 5 additions & 0 deletions tests/core/cgo/bar.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#include <iostream>

void bar() {
std::cout << "bar" << std::endl;
}
91 changes: 91 additions & 0 deletions tests/core/cgo/cc_libs_darwin_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// Copyright 2018 The Bazel Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package cc_libs_test

import (
"debug/macho"
"path"
"strings"
"testing"

"github.com/bazelbuild/rules_go/go/tools/bazel"
)

func TestBinaries(t *testing.T) {
for _, test := range []struct {
shortPath string
wantLibs map[string]bool
}{
{
shortPath: "tests/core/cgo/pure_bin",
wantLibs: map[string]bool{"libSystem": false, "libc++": false},
}, {
shortPath: "tests/core/cgo/c_srcs_bin",
wantLibs: map[string]bool{"libSystem": true, "libc++": false},
}, {
shortPath: "tests/core/cgo/cc_srcs_bin",
wantLibs: map[string]bool{"libSystem": true, "libc++": true},
}, {
shortPath: "tests/core/cgo/cc_deps_bin",
wantLibs: map[string]bool{"libSystem": true, "libc++": true},
},
} {
t.Run(path.Base(test.shortPath), func(t *testing.T) {
libs, err := listLibs(test.shortPath)
if err != nil {
t.Fatal(err)
}
haveLibs := make(map[string]bool)
for _, lib := range libs {
haveLibs[lib] = true
}
for haveLib := range haveLibs {
if wantLib, ok := test.wantLibs[haveLib]; ok && !wantLib {
t.Errorf("unexpected dependency on library %q", haveLib)
}
}
for wantLib, want := range test.wantLibs {
if want && !haveLibs[wantLib] {
t.Errorf("wanted dependency on library %q", wantLib)
}
}
})
}
}

func listLibs(shortPath string) ([]string, error) {
binPath, err := bazel.Runfile(shortPath)
if err != nil {
return nil, err
}
f, err := macho.Open(binPath)
if err != nil {
return nil, err
}
defer f.Close()
libs, err := f.ImportedLibraries()
if err != nil {
return nil, err
}
for i := range libs {
if pos := strings.LastIndexByte(libs[i], '/'); pos >= 0 {
libs[i] = libs[i][pos+1:]
}
if pos := strings.IndexByte(libs[i], '.'); pos >= 0 {
libs[i] = libs[i][:pos]
}
}
return libs, nil
}
91 changes: 91 additions & 0 deletions tests/core/cgo/cc_libs_linux_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// Copyright 2018 The Bazel Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package cc_libs_test

import (
"debug/elf"
"path"
"strings"
"testing"

"github.com/bazelbuild/rules_go/go/tools/bazel"
)

func TestBinaries(t *testing.T) {
for _, test := range []struct {
shortPath string
wantLibs map[string]bool
}{
{
shortPath: "tests/core/cgo/pure_bin",
wantLibs: map[string]bool{"libc": false, "libstdc++": false},
}, {
shortPath: "tests/core/cgo/c_srcs_bin",
wantLibs: map[string]bool{"libc": true, "libstdc++": false},
}, {
shortPath: "tests/core/cgo/cc_srcs_bin",
wantLibs: map[string]bool{"libc": true, "libstdc++": true},
}, {
shortPath: "tests/core/cgo/cc_deps_bin",
wantLibs: map[string]bool{"libc": true, "libstdc++": true},
},
} {
t.Run(path.Base(test.shortPath), func(t *testing.T) {
libs, err := listLibs(test.shortPath)
if err != nil {
t.Fatal(err)
}
haveLibs := make(map[string]bool)
for _, lib := range libs {
haveLibs[lib] = true
}
for haveLib := range haveLibs {
if wantLib, ok := test.wantLibs[haveLib]; ok && !wantLib {
t.Errorf("unexpected dependency on library %q", haveLib)
}
}
for wantLib, want := range test.wantLibs {
if want && !haveLibs[wantLib] {
t.Errorf("wanted dependency on library %q", wantLib)
}
}
})
}
}

func listLibs(shortPath string) ([]string, error) {
binPath, err := bazel.Runfile(shortPath)
if err != nil {
return nil, err
}
f, err := elf.Open(binPath)
if err != nil {
return nil, err
}
defer f.Close()
libs, err := f.ImportedLibraries()
if err != nil {
return nil, err
}
for i := range libs {
if pos := strings.LastIndexByte(libs[i], '/'); pos >= 0 {
libs[i] = libs[i][pos+1:]
}
if pos := strings.IndexByte(libs[i], '.'); pos >= 0 {
libs[i] = libs[i][:pos]
}
}
return libs, nil
}
7 changes: 7 additions & 0 deletions tests/core/cgo/foo.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#include <stdio.h>

void foo() {
printf("foo\n");
}


3 changes: 3 additions & 0 deletions tests/core/cgo/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package main

func main() {}

0 comments on commit 4c45fcd

Please sign in to comment.