Include the symlinked dwp/objcopy/strip files in the toolchain (#108)

* Fix including symlink extra files in dwp/objcopy

* Add strip filegroups to support strip inside sandbox

* tests: add a `.stripped` test

* tests: test that `.dwp` targets can be built

This has some things we'll eventually want to remove; see #109 for some context.

* tests: have `dwp_file` transition to `-c dbg` so .dwo files are generated with clang-12+

Issue #109 and [this comment](#108 (comment))
have some context; the short version is that `-gsplit-dwarf` no longer implies `-g2` which means under
`-c fastbuild` (implies `-g0`) no `.dwo` files are created.

There's a patch to `unix_cc_toolchain_config.bzl` but for now we'll just use `-c dbg` for this one
target using a transition.

* tests: migration fixes

Co-authored-by: Rahul Butani <rr.butani@gmail.com>

tests/BUILD.bazel

@@ -12,7 +12,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-load("@rules_cc//cc:defs.bzl", "cc_library", "cc_test")
+load("@bazel_skylib//rules:build_test.bzl", "build_test")
+load("@rules_cc//cc:defs.bzl", "cc_library", "cc_test", "cc_binary")
+load(":transitions.bzl", "dwp_file")
 
 cc_library(
  name = "stdlib",
@@ -29,13 +31,75 @@ cc_test(
  deps = [":stdlib"],
 )
 
+# We want to test a `.stripped` target to make sure `llvm-strip` can be called.
+#
+# For this we need `cc_binary`; `cc_test` does not create `.stripped` targets.
+cc_binary(
+ name = "stdlib_bin",
+ srcs = ["stdlib_test.cc"],
+ deps = [":stdlib"],
+)
+
+build_test(
+ name = "stripped_binary_test",
+ targets = [
+ ":stdlib_bin.stripped",
+ ]
+)
+
+# We want to test that `llvm-dwp` (used when assembling a `.dwp` file from
+# `.dwo` files) can be called.
+#
+# `--fission=yes` enables this for all compilation modes but we also need to
+# enable the `per_object_debug_info` feature manually because
+# `unix_cc_toolchain_config.bzl`'s` `cc_toolchain_config` does not (see #109).
+#
+# Additionally, newer versions of clang (12+) require a non-zero `-g` setting to
+# actually produce the `.dwo` files in addition to the `-gsplit-dwarf` flag. The
+# feature in `unix_cc_toolchain_config.bzl` should be updated to reflect this
+# and pass in an appropriate `-g` flag.
+#
+# bazelbuild/rules_cc#115 is a patch that does this
+# (https://github.com/bazelbuild/rules_cc/pull/115).
+#
+# bazelbuild/bazel#14028 tracks this
+# (https://github.com/bazelbuild/bazel/issues/14038).
+#
+# #109 in this repo and this comment
+# (https://github.com/grailbio/bazel-toolchain/pull/108#issuecomment-928839768)
+# have some additional details.
+#
+# For now, we'll specify `-c dbg` when building `.dwo` and `.dwp` files.
+#
+# This (setting the fission flag, enabling the `per_object_debug_info` feature,
+# and setting the compilation mode to `dbg`) is what `dwp_file` does using a
+# transition.
+#
+# Unfortunately `per_object_debug_info` breaks on macOS which is why this target
+# is marked as only being compatible with Linux (see #109).
+dwp_file(
+ name = "stdlib.dwp",
+ src = ":stdlib_bin",
+ # NOTE: we should eventually we able to drop this; see #109.
+ override_compilation_mode = "dbg",
+ target_compatible_with = [
+ "@platforms//os:linux",
+ ],
+)
+
+build_test(
+ name = "dwp_test",
+ targets = [
+ ":stdlib.dwp",
+ ],
+)
+
 cc_test(
  name = "pthread_link_test",
  srcs = ["pthread_link_test.cc"],
  linkstatic = False,
 )
 
-
 sh_test(
  name = "file_dependency_test",
  srcs = ["file_dependency_test.sh"],

tests/transitions.bzl

@@ -0,0 +1,85 @@
+"""Helper transitions for tests."""
+
+# This transition function sets `--features=per_object_debug_info` and
+# `--fission` as well as the compilation mode.
+#
+# These three Bazel flags influence whether or not `.dwo` and `.dwp` are
+# created.
+def _fission_transition_impl(settings, attr):
+ features = settings["//command_line_option:features"]
+ if "per_object_debug_info" in features:
+ features.remove("per_object_debug_info")
+
+ enable_per_object_debug_info = attr.per_object_debug_info
+ if enable_per_object_debug_info:
+ features.append("per_object_debug_info")
+
+ compilation_mode = settings["//command_line_option:compilation_mode"]
+ if attr.override_compilation_mode:
+ compilation_mode = attr.override_compilation_mode
+
+ return {
+ "//command_line_option:compilation_mode": compilation_mode,
+ "//command_line_option:fission": attr.fission,
+ "//command_line_option:features": features,
+ }
+
+fission_transition = transition(
+ implementation = _fission_transition_impl,
+ inputs = [
+ "//command_line_option:compilation_mode",
+ "//command_line_option:features",
+ ],
+ outputs = [
+ "//command_line_option:compilation_mode",
+ "//command_line_option:features",
+ "//command_line_option:fission",
+ ],
+)
+
+def _dwp_file_impl(ctx):
+ file = ctx.attr.name
+ file = ctx.actions.declare_file(file)
+ ctx.actions.symlink(
+ output = file,
+ target_file = ctx.attr.src[0][DebugPackageInfo].dwp_file,
+ )
+
+ return [DefaultInfo(files = depset([file]))]
+
+dwp_file = rule(
+ implementation = _dwp_file_impl,
+ attrs = {
+ "src": attr.label(
+ cfg = fission_transition,
+ mandatory = True,
+ doc = "The actual target to build and grab the .dwp file from.",
+ providers = [DebugPackageInfo],
+ ),
+ # NOTE: we should eventually be able to remove this (see #109).
+ "per_object_debug_info": attr.bool(
+ default = True,
+ ),
+ "fission": attr.string(
+ default = "yes",
+ values = ["yes", "no", "dbg", "fastbuild", "opt"],
+ ),
+ # NOTE: this should eventually not be necessary; see #109 for context
+ # and also:
+ # - https://reviews.llvm.org/D80391
+ # - https://github.com/bazelbuild/bazel/issues/14038
+ # - https://github.com/bazelbuild/rules_cc/pull/115
+ #
+ # Essentially, we now need to specify `-g2` explicitly to generate
+ # `.dwo` files.
+ "override_compilation_mode": attr.string(
+ default = "",
+ mandatory = False,
+ values = ["dbg", "fastbuild", "opt"],
+ ),
+ "_allowlist_function_transition": attr.label(
+ default = "@bazel_tools//tools/allowlists/function_transition_allowlist"
+ ),
+ },
+ incompatible_use_toolchain_transition = True,
+)

toolchain/BUILD.llvm_repo

@@ -119,3 +119,8 @@ filegroup(
  name = "readelf",
  srcs = ["bin/llvm-readelf"],
 )
+
+filegroup(
+ name = "strip",
+ srcs = ["bin/llvm-strip"],
+)

toolchain/internal/configure.bzl

@@ -320,18 +320,21 @@ filegroup(name = "all-files-{suffix}", srcs = [":all-components-{suffix}"{extra_
 filegroup(name = "archiver-files-{suffix}", srcs = ["{toolchain_root}:ar"{extra_files_str}])
 filegroup(name = "assembler-files-{suffix}", srcs = ["{toolchain_root}:as"{extra_files_str}])
 filegroup(name = "compiler-files-{suffix}", srcs = [":compiler-components-{suffix}"{extra_files_str}])
+filegroup(name = "dwp-files-{suffix}", srcs = ["{toolchain_root}:dwp"{extra_files_str}])
 filegroup(name = "linker-files-{suffix}", srcs = [":linker-components-{suffix}"{extra_files_str}])
+filegroup(name = "objcopy-files-{suffix}", srcs = ["{toolchain_root}:objcopy"{extra_files_str}])
+filegroup(name = "strip-files-{suffix}", srcs = ["{toolchain_root}:strip"{extra_files_str}])
 
 cc_toolchain(
  name = "cc-clang-{suffix}",
  all_files = "all-files-{suffix}",
  ar_files = "archiver-files-{suffix}",
  as_files = "assembler-files-{suffix}",
  compiler_files = "compiler-files-{suffix}",
- dwp_files = "{toolchain_root}:dwp",
+ dwp_files = "dwp-files-{suffix}",
  linker_files = "linker-files-{suffix}",
- objcopy_files = "{toolchain_root}:objcopy",
- strip_files = ":empty",
+ objcopy_files = "objcopy-files-{suffix}",
+ strip_files = "strip-files-{suffix}",
  toolchain_config = "local-{suffix}",
 )
 """