Fix panic line numbers (#3492)

Without this patch, line numbers are missing for first-party code in
panic traces.

This reverts part of #1563 but keeps the test showing we have a relative
path not an absolute one, and adds an integration test showing that line
numbers are correctly reported in panic traces.

As far as I know, path remapping has been improved in rustc over the
last few years, so maybe the behaviour reported in that patch has been
fixed.

Closes #3476

Author: illicitonion

.bazelci/presubmit.yml

@@ -66,9 +66,13 @@ default_rbe_targets: &default_rbe_targets
 default_macos_targets: &default_macos_targets
   - "--"
   - "//..."
+  # Not sure why this doesn't work on bazelci, it does locally on macOS.
+  - "-//test/unit/remap_path_prefix:integration_test"
 default_windows_targets: &default_windows_targets
   - "--" # Allows negative patterns; hack for https://github.com/bazelbuild/continuous-integration/pull/245
   - "//..."
+  # Panic backtraces appear not to work well on Windows.
+  - "-//test/unit/remap_path_prefix:integration_test"
 crate_universe_vendor_example_targets: &crate_universe_vendor_example_targets
   - "//vendor_external:crates_vendor"
   - "//vendor_local_manifests:crates_vendor"

rust/private/rustc.bzl

@@ -830,7 +830,7 @@ def construct_arguments(
         add_flags_for_binary = False,
         include_link_flags = True,
         stamp = False,
-        remap_path_prefix = "",
+        remap_path_prefix = ".",
         use_json_output = False,
         build_metadata = False,
         force_depend_on_objects = False,

test/unit/remap_path_prefix/BUILD.bazel

@@ -1,4 +1,5 @@
-load("//rust:defs.bzl", "rust_library", "rust_test")
+load("//rust:defs.bzl", "rust_binary", "rust_library", "rust_test")
+load(":debug_transition.bzl", "dbg_rust_binary")
 
 rust_library(
     name = "dep",
@@ -12,3 +13,25 @@ rust_test(
     edition = "2018",
     deps = [":dep"],
 )
+
+rust_binary(
+    name = "panic_bin",
+    srcs = ["panic_bin.rs"],
+    edition = "2018",
+)
+
+dbg_rust_binary(
+    name = "panic_bin_dbg",
+    binary = ":panic_bin",
+)
+
+rust_test(
+    name = "integration_test",
+    srcs = ["integration_test.rs"],
+    data = [":panic_bin_dbg"],
+    edition = "2018",
+    env = {
+        "BINARY": "$(rlocationpath :panic_bin_dbg)",
+    },
+    deps = ["//rust/runfiles"],
+)

test/unit/remap_path_prefix/debug_transition.bzl

@@ -0,0 +1,73 @@
+"""Internal utility for transitioning a target to -c dbg."""
+
+load("@bazel_skylib//lib:paths.bzl", "paths")
+
+def _transition_dbg_impl(_1, _2):
+    return {"//command_line_option:compilation_mode": "dbg"}
+
+_transition_dbg = transition(
+    implementation = _transition_dbg_impl,
+    inputs = [],
+    outputs = ["//command_line_option:compilation_mode"],
+)
+
+def _dbg_rust_binary_impl(ctx):
+    # We need to forward the DefaultInfo provider from the underlying rule.
+    # Unfortunately, we can't do this directly, because Bazel requires that the executable to run
+    # is actually generated by this rule, so we need to symlink to it, and generate a synthetic
+    # forwarding DefaultInfo.
+
+    result = []
+    binary = ctx.attr.binary[0]
+
+    default_info = binary[DefaultInfo]
+    files = default_info.files
+    new_executable = None
+    original_executable = default_info.files_to_run.executable
+    runfiles = default_info.default_runfiles
+
+    if not original_executable:
+        fail("Cannot transition a 'binary' that is not executable")
+
+    new_executable_name = original_executable.basename
+
+    # In order for the symlink to have the same basename as the original
+    # executable (important in the case of proto plugins), put it in a
+    # subdirectory named after the label to prevent collisions.
+    new_executable = ctx.actions.declare_file(paths.join(ctx.label.name, new_executable_name))
+
+    ctx.actions.symlink(
+        output = new_executable,
+        target_file = original_executable,
+        is_executable = True,
+    )
+
+    files = depset(direct = [new_executable])
+    runfiles = ctx.runfiles([new_executable])
+
+    result.append(
+        DefaultInfo(
+            files = files,
+            runfiles = runfiles,
+            executable = new_executable,
+        ),
+    )
+
+    return result
+
+dbg_rust_binary = rule(
+    doc = "Transitions the binary to use the provided platform.",
+    implementation = _dbg_rust_binary_impl,
+    attrs = {
+        "binary": attr.label(
+            doc = "The target to transition.",
+            allow_files = True,
+            cfg = _transition_dbg,
+            mandatory = True,
+        ),
+        "_allowlist_function_transition": attr.label(
+            default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
+        ),
+    },
+    executable = True,
+)

test/unit/remap_path_prefix/integration_test.rs

@@ -0,0 +1,32 @@
+#[test]
+fn test_backtrace() {
+    let runfiles = runfiles::Runfiles::create().unwrap();
+
+    let binary_env = std::env::var("BINARY").unwrap();
+
+    let binary_path = runfiles::rlocation!(runfiles, &binary_env).unwrap();
+
+    eprintln!("Running {}", binary_path.display());
+
+    let output = std::process::Command::new(binary_path)
+        .env("RUST_BACKTRACE", "full")
+        .output()
+        .unwrap();
+    let stderr = String::from_utf8(output.stderr).unwrap();
+
+    eprintln!("Saw backtrace:\n{}", stderr);
+
+    let mut check_next = false;
+    for line in stderr.split('\n') {
+        if check_next {
+            if !line.contains("./test/unit/remap_path_prefix/panic_bin.rs:6:5") {
+                panic!("Expected line to contain ./test/unit/remap_path_prefix/panic_bin.rs:6:5 but was {}", line);
+            }
+            return;
+        }
+        if line.contains("::do_call") {
+            check_next = true;
+        }
+    }
+    panic!("Didn't see expected line containing panic_bin::do_call");
+}

test/unit/remap_path_prefix/panic_bin.rs

@@ -0,0 +1,7 @@
+fn main() {
+    do_call();
+}
+
+fn do_call() {
+    panic!("This is bad!");
+}

test/unit/remap_path_prefix/test.rs

@@ -1,7 +1,9 @@
 #[test]
 fn test_dep_file_name() {
-    assert_eq!(
-        dep::get_file_name::<()>(),
-        "test/unit/remap_path_prefix/dep.rs"
-    );
+    let mut expected = std::path::PathBuf::from(".");
+    // After the ., the path components appear to be joined with / on all platforms.
+    // This is probably a rustc bug we should report.
+    expected.push("test/unit/remap_path_prefix/dep.rs");
+    let expected_str = expected.to_str().unwrap();
+    assert_eq!(dep::get_file_name::<()>(), expected_str);
 }