Skip to content

Commit df4d02c

Browse files
committed
Fix panic line numbers
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.
1 parent 4d14035 commit df4d02c

File tree

6 files changed

+134
-3
lines changed

6 files changed

+134
-3
lines changed

rust/private/rustc.bzl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -829,7 +829,7 @@ def construct_arguments(
829829
add_flags_for_binary = False,
830830
include_link_flags = True,
831831
stamp = False,
832-
remap_path_prefix = "",
832+
remap_path_prefix = ".",
833833
use_json_output = False,
834834
build_metadata = False,
835835
force_depend_on_objects = False,

test/unit/remap_path_prefix/BUILD.bazel

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
load("//rust:defs.bzl", "rust_library", "rust_test")
1+
load("//rust:defs.bzl", "rust_binary", "rust_library", "rust_test")
2+
load(":debug_transition.bzl", "dbg_rust_binary")
23

34
rust_library(
45
name = "dep",
@@ -12,3 +13,25 @@ rust_test(
1213
edition = "2018",
1314
deps = [":dep"],
1415
)
16+
17+
rust_binary(
18+
name = "panic_bin",
19+
srcs = ["panic_bin.rs"],
20+
edition = "2018",
21+
)
22+
23+
dbg_rust_binary(
24+
name = "panic_bin_dbg",
25+
binary = ":panic_bin",
26+
)
27+
28+
rust_test(
29+
name = "integration_test",
30+
srcs = ["integration_test.rs"],
31+
data = [":panic_bin_dbg"],
32+
edition = "2018",
33+
env = {
34+
"BINARY": "$(rlocationpath :panic_bin_dbg)",
35+
},
36+
deps = ["//rust/runfiles"],
37+
)
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
"""Internal utility for transitioning a target to -c dbg."""
2+
3+
load("@bazel_skylib//lib:paths.bzl", "paths")
4+
5+
def _transition_dbg_impl(_1, _2):
6+
return {"//command_line_option:compilation_mode": "dbg"}
7+
8+
_transition_dbg = transition(
9+
implementation = _transition_dbg_impl,
10+
inputs = [],
11+
outputs = ["//command_line_option:compilation_mode"],
12+
)
13+
14+
def _dbg_rust_binary_impl(ctx):
15+
# We need to forward the DefaultInfo provider from the underlying rule.
16+
# Unfortunately, we can't do this directly, because Bazel requires that the executable to run
17+
# is actually generated by this rule, so we need to symlink to it, and generate a synthetic
18+
# forwarding DefaultInfo.
19+
20+
result = []
21+
binary = ctx.attr.binary[0]
22+
23+
default_info = binary[DefaultInfo]
24+
files = default_info.files
25+
new_executable = None
26+
original_executable = default_info.files_to_run.executable
27+
runfiles = default_info.default_runfiles
28+
29+
if not original_executable:
30+
fail("Cannot transition a 'binary' that is not executable")
31+
32+
new_executable_name = original_executable.basename
33+
34+
# In order for the symlink to have the same basename as the original
35+
# executable (important in the case of proto plugins), put it in a
36+
# subdirectory named after the label to prevent collisions.
37+
new_executable = ctx.actions.declare_file(paths.join(ctx.label.name, new_executable_name))
38+
ctx.actions.symlink(
39+
output = new_executable,
40+
target_file = original_executable,
41+
is_executable = True,
42+
)
43+
files = depset(direct = [new_executable], transitive = [files])
44+
runfiles = runfiles.merge(ctx.runfiles([new_executable]))
45+
46+
result.append(
47+
DefaultInfo(
48+
files = files,
49+
runfiles = runfiles,
50+
executable = new_executable,
51+
),
52+
)
53+
54+
return result
55+
56+
dbg_rust_binary = rule(
57+
doc = "Transitions the binary to use the provided platform.",
58+
implementation = _dbg_rust_binary_impl,
59+
attrs = {
60+
"binary": attr.label(
61+
doc = "The target to transition.",
62+
allow_files = True,
63+
cfg = _transition_dbg,
64+
mandatory = True,
65+
),
66+
"_allowlist_function_transition": attr.label(
67+
default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
68+
),
69+
},
70+
executable = True,
71+
)
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
#[test]
2+
fn test_backtrace() {
3+
let runfiles = runfiles::Runfiles::create().unwrap();
4+
5+
let binary_env = std::env::var("BINARY").unwrap();
6+
7+
let binary_path = runfiles::rlocation!(runfiles, &binary_env).unwrap();
8+
9+
eprintln!("Running {}", binary_path.display());
10+
11+
let output = std::process::Command::new(binary_path).env("RUST_BACKTRACE", "full").output().unwrap();
12+
let stderr = String::from_utf8(output.stderr).unwrap();
13+
14+
let mut check_next = false;
15+
for line in stderr.split('\n') {
16+
if check_next {
17+
if !line.contains("./test/unit/remap_path_prefix/panic_bin.rs:6:5") {
18+
panic!("Expected line to contain ./test/unit/remap_path_prefix/panic_bin.rs:6:5 but was {}", line);
19+
}
20+
return;
21+
}
22+
if line.contains("panic_bin::do_call") {
23+
check_next = true;
24+
}
25+
}
26+
27+
eprintln!("Saw backtrace:\n{}", stderr);
28+
29+
assert_eq!("", stderr);
30+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
fn main() {
2+
do_call();
3+
}
4+
5+
fn do_call() {
6+
panic!("This is bad!!");
7+
}

test/unit/remap_path_prefix/test.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,6 @@
22
fn test_dep_file_name() {
33
assert_eq!(
44
dep::get_file_name::<()>(),
5-
"test/unit/remap_path_prefix/dep.rs"
5+
"./test/unit/remap_path_prefix/dep.rs"
66
);
77
}

0 commit comments

Comments
 (0)