Fixes for rust_binary that depends on a cc_library that depends on a …

…rust_library (#825)

Consider a case of a `rust_binary` that depends on a `cc_library` that itself depends
on a `rust_library`:

```
.
├── bin.rs
├── BUILD
└── lib.rs
```
```bazel
load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_library")

rust_library(
    name = "rust_lib",
    srcs = ["lib.rs"],
)

cc_library(
    name = "cc_lib",
    srcs = [],
    deps = [":rust_lib"],
)

rust_binary(
    name = "bin",
    srcs = ["bin.rs"],
    deps = [":cc_lib"],
)
```

```rust
// bin.rs
fn main() {
    println!("hi");
}
```

```rust
// lib.rs: empty
```


Running `bazel build //project:bin` runs into several linker errors, which this PR tries to address:
- When passing an argument like `-lcrate`, the linker looks for a static library named `libcrate.a`.
- the CC link actions get confused by static library names that have multiple dots, e.g., end in "libcrate.rlib.a", so
updated the scripts to produce them as "libcrate-rlib.a" instead.
- The libraries `panic_abort` and `panic_unwind` are alternatives; attempting to pass both to the linker runs into duplicate symbol errors. Updated the rules to prefer `panic_unwind` if both are available. This is since the standard library by default depends on `panic_unwind`. Note that in platforms where `panic_unwind` is not available, `panic_abort` will be picked. In the future we could add a feature that allows users to pick the panic strategy, aligned with how it will be done in upstream rustc (rust-lang/wg-cargo-std-aware#29).

Another thing worth noting is that the way we currently process `.rlib`-s into static libs to pass on to CC rules doesn't quite work when using the current stable `rustc 1.53.0`; it works for the beta `rustc` and beyond: on Linux, a `crate.rlib` is an archive that contains .o files (good) and a Rust metadata file `lib.rmeta` (caution). We symlink `crate.rlib` into `libcrate.a`. The linker generally checks that all members of an archive passed in a `--whole-archive` section are ELF files. The commit rust-lang/rust@c79419a on Jun 4 updated `rustc` so that the produced `lib.rmeta` is an ELF file (previously it was a binary blob), which satisfies the linker checks. This PR does not attempt to address the case where`lib.rmeta` is not an ELF file, hence even with this PR in, trying to run the example with the current stable `rustc 1.53.0` will produce linker errors complaining that an archive member is not recognized, like this one `/usr/bin/ld.gold: error: rust_lib: plugin failed to claim member lib.rmeta at 2480`.

With this PR, `bazel build //project:bin` works (with sufficiently recent `rustc`, as described above).

rust/private/rustc.bzl

@@ -27,6 +27,7 @@ load(
     "find_cc_toolchain",
     "get_lib_name",
     "get_preferred_artifact",
+    "make_static_lib_symlink",
     "relativize",
 )
 
@@ -635,8 +636,7 @@ def establish_cc_info(ctx, crate_info, toolchain, cc_toolchain, feature_configur
         # bazel hard-codes a check for endswith((".a", ".pic.a",
         # ".lib")) in create_library_to_link, so we work around that
         # by creating a symlink to the .rlib with a .a extension.
-        dot_a = ctx.actions.declare_file(crate_info.name + ".a", sibling = crate_info.output)
-        ctx.actions.symlink(output = dot_a, target_file = crate_info.output)
+        dot_a = make_static_lib_symlink(ctx.actions, crate_info.output)
 
         # TODO(hlopko): handle PIC/NOPIC correctly
         library_to_link = cc_common.create_library_to_link(

rust/private/utils.bzl

@@ -296,3 +296,30 @@ def dedent(doc_string):
         # Remove the leading block of spaces from the current line
         block = " " * space_count
         return "\n".join([line.replace(block, "", 1).rstrip() for line in lines])
+
+def make_static_lib_symlink(actions, rlib_file):
+    """Add a .a symlink to an .rlib file.
+
+    The name of the symlink is derived from the <name> of the <name>.rlib file as follows:
+    * `<name>.a`, if <name> starts with `lib`
+    * `lib<name>.a`, otherwise.
+
+    For example, the name of the symlink for
+    * `libcratea.rlib` is `libcratea.a`
+    * `crateb.rlib` is `libcrateb.a`.
+
+    Args:
+        actions (actions): The rule's context actions object.
+        rlib_file (File): The file to symlink, which must end in .rlib.
+
+    Returns:
+        The symlink's File.
+    """
+    if not rlib_file.basename.endswith(".rlib"):
+        fail("file is not an .rlib: ", rlib_file.basename)
+    basename = rlib_file.basename[:-5]
+    if not basename.startswith("lib"):
+        basename = "lib" + basename
+    dot_a = actions.declare_file(basename + ".a", sibling = rlib_file)
+    actions.symlink(output = dot_a, target_file = rlib_file)
+    return dot_a

rust/toolchain.bzl

@@ -1,21 +1,7 @@
 """The rust_toolchain rule definition and implementation."""
 
 load("//rust/private:common.bzl", "rust_common")
-load("//rust/private:utils.bzl", "dedent", "find_cc_toolchain")
-
-def _make_dota(ctx, file):
-    """Add a symlink for a file that ends in .a, so it can be used as a staticlib.
-
-    Args:
-        ctx (ctx): The rule's context object.
-        file (File): The file to symlink.
-
-    Returns:
-        The symlink's File.
-    """
-    dot_a = ctx.actions.declare_file(file.basename + ".a", sibling = file)
-    ctx.actions.symlink(output = dot_a, target_file = file)
-    return dot_a
+load("//rust/private:utils.bzl", "dedent", "find_cc_toolchain", "make_static_lib_symlink")
 
 def _rust_stdlib_filegroup_impl(ctx):
     rust_lib = ctx.files.srcs
@@ -35,7 +21,7 @@ def _rust_stdlib_filegroup_impl(ctx):
         #
         # alloc depends on the allocator_library if it's configured, but we
         # do that later.
-        dot_a_files = [_make_dota(ctx, f) for f in std_rlibs]
+        dot_a_files = [make_static_lib_symlink(ctx.actions, f) for f in std_rlibs]
 
         alloc_files = [f for f in dot_a_files if "alloc" in f.basename and "std" not in f.basename]
         between_alloc_and_core_files = [f for f in dot_a_files if "compiler_builtins" in f.basename]
@@ -140,13 +126,36 @@ def _make_libstd_and_allocator_ccinfo(ctx, rust_lib, allocator_library):
             transitive = [between_alloc_and_core_inputs],
             order = "topological",
         )
+
+        # The libraries panic_abort and panic_unwind are alternatives.
+        # The std by default requires panic_unwind.
+        # Exclude panic_abort if panic_unwind is present.
+        # TODO: Provide a setting to choose between panic_abort and panic_unwind.
+        filtered_between_core_and_std_files = rust_stdlib_info.between_core_and_std_files
+        has_panic_unwind = [
+            f
+            for f in filtered_between_core_and_std_files
+            if "panic_unwind" in f.basename
+        ]
+        if has_panic_unwind:
+            filtered_between_core_and_std_files = [
+                f
+                for f in filtered_between_core_and_std_files
+                if "panic_abort" not in f.basename
+            ]
         between_core_and_std_inputs = depset(
-            [_ltl(f, ctx, cc_toolchain, feature_configuration) for f in rust_stdlib_info.between_core_and_std_files],
+            [
+                _ltl(f, ctx, cc_toolchain, feature_configuration)
+                for f in filtered_between_core_and_std_files
+            ],
             transitive = [core_inputs],
             order = "topological",
         )
         std_inputs = depset(
-            [_ltl(f, ctx, cc_toolchain, feature_configuration) for f in rust_stdlib_info.std_files],
+            [
+                _ltl(f, ctx, cc_toolchain, feature_configuration)
+                for f in rust_stdlib_info.std_files
+            ],
             transitive = [between_core_and_std_inputs],
             order = "topological",
         )

test/unit/cc_info/cc_info_test.bzl

@@ -36,6 +36,8 @@ def _assert_cc_info_has_library_to_link(env, tut, type, ccinfo_count):
         asserts.equals(env, None, library_to_link.resolved_symlink_dynamic_library)
         asserts.equals(env, None, library_to_link.resolved_symlink_interface_library)
         asserts.true(env, library_to_link.static_library != None)
+        if type in ("rlib", "lib"):
+            asserts.true(env, library_to_link.static_library.basename.startswith("lib" + tut.label.name))
         asserts.true(env, library_to_link.pic_static_library != None)
         asserts.equals(env, library_to_link.static_library, library_to_link.pic_static_library)
 

test/unit/stdlib/BUILD.bazel

@@ -0,0 +1,4 @@
+load(":stdlib.bzl", "stdlib_suite")
+
+############################ UNIT TESTS #############################
+stdlib_suite(name = "stdlib_suite")

test/unit/stdlib_ordering/stdlib_ordering.bzl → test/unit/stdlib/stdlib.bzl

@@ -1,4 +1,4 @@
-"""Unittest to verify ordering of rust stdlib in rust_library() CcInfo"""
+"""Unittest to verify contents and ordering of rust stdlib in rust_library() CcInfo"""
 
 load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts")
 load("//rust:defs.bzl", "rust_library")
@@ -26,17 +26,41 @@ def _dedup_preserving_order(list):
         r.append(e)
     return r
 
+def _stdlibs(tut):
+    """Given a target, return the list of its standard rust libraries."""
+    libs = [
+        lib.static_library
+        for li in tut[CcInfo].linking_context.linker_inputs.to_list()
+        for lib in li.libraries
+    ]
+    stdlibs = [lib for lib in libs if (tut.label.name not in lib.basename)]
+    return stdlibs
+
 def _libstd_ordering_test_impl(ctx):
     env = analysistest.begin(ctx)
     tut = analysistest.target_under_test(env)
-    libs = [lib.static_library for li in tut[CcInfo].linking_context.linker_inputs.to_list() for lib in li.libraries]
-    rlibs = [_categorize_library(lib.basename) for lib in libs if ".rlib" in lib.basename]
-    set_to_check = _dedup_preserving_order([lib for lib in rlibs if lib != "other"])
+    stdlib_categories = [_categorize_library(lib.basename) for lib in _stdlibs(tut)]
+    set_to_check = _dedup_preserving_order([lib for lib in stdlib_categories if lib != "other"])
     asserts.equals(env, ["std", "core", "compiler_builtins", "alloc"], set_to_check)
     return analysistest.end(env)
 
 libstd_ordering_test = analysistest.make(_libstd_ordering_test_impl)
 
+def _libstd_panic_test_impl(ctx):
+    # The libraries panic_unwind and panic_abort are alternatives.
+    # Check that they don't occur together.
+    env = analysistest.begin(ctx)
+    tut = analysistest.target_under_test(env)
+    stdlibs = _stdlibs(tut)
+    has_panic_unwind = [lib for lib in stdlibs if "panic_unwind" in lib.basename]
+    if has_panic_unwind:
+        has_panic_abort = [lib for lib in stdlibs if "panic_abort" in lib.basename]
+        asserts.false(env, has_panic_abort)
+
+    return analysistest.end(env)
+
+libstd_panic_test = analysistest.make(_libstd_panic_test_impl)
+
 def _native_dep_test():
     rust_library(
         name = "some_rlib",
@@ -48,7 +72,12 @@ def _native_dep_test():
         target_under_test = ":some_rlib",
     )
 
-def stdlib_ordering_suite(name):
+    libstd_panic_test(
+        name = "libstd_panic_test",
+        target_under_test = ":some_rlib",
+    )
+
+def stdlib_suite(name):
     """Entry-point macro called from the BUILD file.
 
     Args: