diff --git a/BUILD.bazel b/BUILD.bazel index 97380f9704..60bf72eb8e 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -130,7 +130,11 @@ go_config( "//conditions:default": False, }), static = "//go/config:static", - strip = "//go/config:strip", + strip = select({ + "//go/private:is_strip_always": True, + "//go/private:is_strip_sometimes_fastbuild": True, + "//conditions:default": False, + }), visibility = ["//visibility:public"], ) diff --git a/go/config/BUILD.bazel b/go/config/BUILD.bazel index 12075e37dc..a292fc9dcf 100644 --- a/go/config/BUILD.bazel +++ b/go/config/BUILD.bazel @@ -1,7 +1,6 @@ load( "@bazel_skylib//rules:common_settings.bzl", "bool_flag", - "bool_setting", "string_flag", "string_list_flag", ) @@ -34,12 +33,6 @@ bool_flag( visibility = ["//visibility:public"], ) -bool_setting( - name = "strip", - build_setting_default = False, - visibility = ["//visibility:public"], -) - bool_flag( name = "debug", build_setting_default = False, diff --git a/go/modes.rst b/go/modes.rst index 0c8b4c1f6d..431049d53d 100644 --- a/go/modes.rst +++ b/go/modes.rst @@ -69,12 +69,6 @@ or using `Bazel configuration transitions`_. | ``CGO_ENABLED=0``). Packages that contain cgo code may still be built, but | | the cgo code will be filtered out, and the ``cgo`` build tag will be false. | +-------------------+---------------------+------------------------------------+ -| :param:`strip` | :type:`bool` | :value:`false` | -+-------------------+---------------------+------------------------------------+ -| Strips symbols from compiled packages and linked binaries (using the ``-w`` | -| flag). May also be set with the ``--strip`` command line option, which | -| affects C/C++ targets, too. | -+-------------------+---------------------+------------------------------------+ | :param:`debug` | :type:`bool` | :value:`false` | +-------------------+---------------------+------------------------------------+ | Includes debugging information in compiled packages (using the ``-N`` and | diff --git a/go/private/BUILD.bazel b/go/private/BUILD.bazel index 2a83778e9c..16907b537a 100644 --- a/go/private/BUILD.bazel +++ b/go/private/BUILD.bazel @@ -30,6 +30,21 @@ config_setting( visibility = ["//:__pkg__"], ) +config_setting( + name = "is_strip_always", + values = {"strip": "always"}, + visibility = ["//:__pkg__"], +) + +config_setting( + name = "is_strip_sometimes_fastbuild", + values = { + "strip": "sometimes", + "compilation_mode": "fastbuild", + }, + visibility = ["//:__pkg__"], +) + bzl_library( name = "context", srcs = ["context.bzl"], diff --git a/go/private/actions/link.bzl b/go/private/actions/link.bzl index b0c1a1081c..a13d4de910 100644 --- a/go/private/actions/link.bzl +++ b/go/private/actions/link.bzl @@ -181,7 +181,7 @@ def emit_link( # Do not remove, somehow this is needed when building for darwin/arm only. tool_args.add("-buildid=redacted") if go.mode.strip: - tool_args.add("-w") + tool_args.add("-s", "-w") tool_args.add_joined("-extldflags", extldflags, join_with = " ") conflict_err = _check_conflicts(arcs) diff --git a/go/private/context.bzl b/go/private/context.bzl index 65f73b5767..3ee11e3522 100644 --- a/go/private/context.bzl +++ b/go/private/context.bzl @@ -830,7 +830,7 @@ def _go_config_impl(ctx): race = ctx.attr.race[BuildSettingInfo].value, msan = ctx.attr.msan[BuildSettingInfo].value, pure = ctx.attr.pure[BuildSettingInfo].value, - strip = ctx.attr.strip[BuildSettingInfo].value, + strip = ctx.attr.strip, debug = ctx.attr.debug[BuildSettingInfo].value, linkmode = ctx.attr.linkmode[BuildSettingInfo].value, gc_linkopts = ctx.attr.gc_linkopts[BuildSettingInfo].value, @@ -860,10 +860,7 @@ go_config = rule( mandatory = True, providers = [BuildSettingInfo], ), - "strip": attr.label( - mandatory = True, - providers = [BuildSettingInfo], - ), + "strip": attr.bool(mandatory = True), "debug": attr.label( mandatory = True, providers = [BuildSettingInfo], diff --git a/go/private/rules/transition.bzl b/go/private/rules/transition.bzl index 8ee26ae792..4e87e30efe 100644 --- a/go/private/rules/transition.bzl +++ b/go/private/rules/transition.bzl @@ -174,7 +174,6 @@ _common_reset_transition_dict = dict({ "//go/config:msan": False, "//go/config:race": False, "//go/config:pure": False, - "//go/config:strip": False, "//go/config:debug": False, "//go/config:linkmode": LINKMODE_NORMAL, "//go/config:tags": [], diff --git a/tests/core/strip/BUILD.bazel b/tests/core/strip/BUILD.bazel new file mode 100644 index 0000000000..bc2f6bd818 --- /dev/null +++ b/tests/core/strip/BUILD.bazel @@ -0,0 +1,6 @@ +load("@io_bazel_rules_go//go/tools/bazel_testing:def.bzl", "go_bazel_test") + +go_bazel_test( + name = "strip_test", + srcs = ["strip_test.go"], +) diff --git a/tests/core/strip/README.rst b/tests/core/strip/README.rst new file mode 100644 index 0000000000..1b5c1f963e --- /dev/null +++ b/tests/core/strip/README.rst @@ -0,0 +1,13 @@ +symbol stripping +==================== + +strip_test +--------- + +Tests that the global Bazel configuration for stripping are applied to go_binary +targets. +In particular, it tests that stripping is performed iff the bazel flag ``--strip`` +is set to ``always`` or ``--strip`` is set to ``sometimes`` and ``--compilation_mode`` +is ``fastbuild``. +Additionally, it tests that stack traces still contain the same information when stripping +is enabled. diff --git a/tests/core/strip/strip_test.go b/tests/core/strip/strip_test.go new file mode 100644 index 0000000000..b71830334a --- /dev/null +++ b/tests/core/strip/strip_test.go @@ -0,0 +1,270 @@ +// Copyright 2019 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 strip_test + +import ( + "bytes" + "errors" + "fmt" + "os/exec" + "strings" + "testing" + + "github.com/bazelbuild/rules_go/go/tools/bazel_testing" +) + +func TestMain(m *testing.M) { + bazel_testing.TestMain(m, bazel_testing.Args{ + Main: ` +-- BUILD.bazel -- +load("@io_bazel_rules_go//go:def.bzl", "go_binary") + +go_binary( + name = "strip", + srcs = ["strip.go"], +) +-- strip.go -- +package main + +import ( + "debug/elf" + "debug/macho" + "debug/pe" + "errors" + "flag" + "fmt" + "io" + "os" + "regexp" + "runtime" + "runtime/debug" + "strings" +) + +var wantStrip = flag.Bool("wantstrip", false, "") + +func main() { + flag.Parse() + stackTrace, err := panicAndRecover() + if err != nil { + panic(err) + } + gotStackTrace := strings.Split(stackTrace, "\n") + if len(gotStackTrace) != len(wantStackTrace) { + panic(fmt.Sprintf("got %d lines of stack trace, want %d", len(gotStackTrace), len(wantStackTrace))) + } + for i := range gotStackTrace { + expectedLine := regexp.MustCompile(wantStackTrace[i]) + if !expectedLine.MatchString(gotStackTrace[i]) { + panic(fmt.Sprintf("got unexpected stack trace line %q at index %d", gotStackTrace[i], i)) + } + } + stripped, err := isStripped() + if err != nil { + panic(err) + } + if stripped != *wantStrip { + panic(fmt.Sprintf("got stripped=%t, want stripped=%t", stripped, *wantStrip)) + } +} + +func panicAndRecover() (stackTrace string, err error) { + defer func() { + if r := recover(); r != nil { + stackTrace = string(debug.Stack()) + } + }() + panic("test") + return "", errors.New("should not reach here") +} + +func isStripped() (bool, error) { + ownLocation, err := os.Executable() + if err != nil { + return false, err + } + ownBinary, err := os.Open(ownLocation) + if err != nil { + return false, err + } + defer ownBinary.Close() + switch runtime.GOOS { + case "darwin": + return isStrippedMachO(ownBinary) + case "linux": + return isStrippedElf(ownBinary) + case "windows": + return isStrippedPE(ownBinary) + default: + return false, fmt.Errorf("unsupported OS: %s", runtime.GOOS) + } +} + +func isStrippedMachO(f io.ReaderAt) (bool, error) { + macho, err := macho.NewFile(f) + if err != nil { + return false, err + } + gotDwarf := macho.Segment("__DWARF") != nil + gotDebugInfo := macho.Section("__zdebug_info") != nil + if gotDwarf != gotDebugInfo { + return false, fmt.Errorf("inconsistent stripping: gotDwarf=%v, gotDebugInfo=%v", gotDwarf, gotDebugInfo) + } + return !gotDwarf, nil +} + +func isStrippedElf(f io.ReaderAt) (bool, error) { + elf, err := elf.NewFile(f) + if err != nil { + return false, err + } + var gotSymtab bool + for _, section := range elf.Sections { + if section.Name == ".symtab" { + gotSymtab = true + break + } + } + return !gotSymtab, nil +} + + +func isStrippedPE(f io.ReaderAt) (bool, error) { + pe, err := pe.NewFile(f) + if err != nil { + return false, err + } + symtab := pe.Section(".symtab") + if symtab == nil { + return false, fmt.Errorf("no .symtab section") + } + emptySymtab := (symtab.VirtualSize <= 4) && (symtab.Size <= 512) + return emptySymtab, nil +} + + +` + embedWantedStackTraces(), + }) +} + +func Test(t *testing.T) { + for _, test := range []struct { + desc, stripFlag, compilationMode string + wantStrip bool + }{ + { + desc: "run_auto", + wantStrip: true, + }, + { + desc: "run_fastbuild", + compilationMode: "fastbuild", + wantStrip: true, + }, + { + desc: "run_dbg", + compilationMode: "dbg", + }, + { + desc: "run_opt", + compilationMode: "opt", + }, + { + desc: "run_always", + stripFlag: "always", + wantStrip: true, + }, + { + desc: "run_always_opt", + stripFlag: "always", + compilationMode: "opt", + wantStrip: true, + }, + { + desc: "run_never", + stripFlag: "never", + }, + { + desc: "run_sometimes_fastbuild", + stripFlag: "sometimes", + compilationMode: "fastbuild", + wantStrip: true, + }, + { + desc: "run_sometimes_dbg", + stripFlag: "sometimes", + compilationMode: "dbg", + }, + { + desc: "run_sometimes_opt", + stripFlag: "sometimes", + compilationMode: "opt", + }, + } { + t.Run(test.desc, func(t *testing.T) { + args := []string{"run"} + if len(test.stripFlag) > 0 { + args = append(args, "--strip", test.stripFlag) + } + if len(test.compilationMode) > 0 { + args = append(args, "--compilation_mode", test.compilationMode) + } + args = append(args, "//:strip", "--", fmt.Sprintf("-wantstrip=%v", test.wantStrip)) + cmd := bazel_testing.BazelCmd(args...) + stderr := &bytes.Buffer{} + cmd.Stderr = stderr + t.Logf("running: bazel %s", strings.Join(args, " ")) + if err := cmd.Run(); err != nil { + var xerr *exec.ExitError + if !errors.As(err, &xerr) { + t.Fatalf("unexpected error: %v", err) + } + if xerr.ExitCode() == bazel_testing.BUILD_FAILURE { + t.Fatalf("unexpected build failure: %v\nstderr:\n%s", err, stderr.Bytes()) + return + } else if xerr.ExitCode() == bazel_testing.TESTS_FAILED { + t.Fatalf("error running %s:\n%s", strings.Join(cmd.Args, " "), stderr.Bytes()) + } else { + t.Fatalf("unexpected error: %v\nstderr:\n%s", err, stderr.Bytes()) + } + } + }) + } +} + +var wantStackTrace = []string{ + `^goroutine \d+ \[running\]:$`, + `^runtime/debug\.Stack\(\)$`, + `^ GOROOT/src/runtime/debug/stack\.go:\d+ \+0x[0-9a-f]+$`, + `^main\.panicAndRecover\.func1\(\)$`, + `^ strip\.go:\d+ \+0x[0-9a-f]+$`, + `^panic\({0x[0-9a-f]+, 0x[0-9a-f]+}\)$`, + `^ GOROOT/src/runtime/panic\.go:\d+ \+0x[0-9a-f]+$`, + `^main\.panicAndRecover\(\)$`, + `^ strip\.go:\d+ \+0x[0-9a-f]+$`, + `^main\.main\(\)$`, + `^ strip\.go:\d+ \+0x[0-9a-f]+$`, + `^$`, +} + +func embedWantedStackTraces() string { + buf := &bytes.Buffer{} + fmt.Fprintln(buf, "var wantStackTrace = []string{") + for _, s := range wantStackTrace { + fmt.Fprintf(buf, "`%s`,\n", s) + } + fmt.Fprintln(buf, "}") + return buf.String() +}