Skip to content

Commit 3074ed3

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 3074ed3

File tree

6 files changed

+138
-3
lines changed

6 files changed

+138
-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: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
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+
39+
ctx.actions.symlink(
40+
output = new_executable,
41+
target_file = original_executable,
42+
is_executable = True,
43+
)
44+
45+
files = depset(direct = [new_executable])
46+
runfiles = ctx.runfiles([new_executable])
47+
48+
result.append(
49+
DefaultInfo(
50+
files = files,
51+
runfiles = runfiles,
52+
executable = new_executable,
53+
),
54+
)
55+
56+
return result
57+
58+
dbg_rust_binary = rule(
59+
doc = "Transitions the binary to use the provided platform.",
60+
implementation = _dbg_rust_binary_impl,
61+
attrs = {
62+
"binary": attr.label(
63+
doc = "The target to transition.",
64+
allow_files = True,
65+
cfg = _transition_dbg,
66+
mandatory = True,
67+
),
68+
"_allowlist_function_transition": attr.label(
69+
default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
70+
),
71+
},
72+
executable = True,
73+
)
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
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)
12+
.env("RUST_BACKTRACE", "full")
13+
.output()
14+
.unwrap();
15+
let stderr = String::from_utf8(output.stderr).unwrap();
16+
17+
eprintln!("Saw backtrace:\n{}", stderr);
18+
19+
let mut check_next = false;
20+
for line in stderr.split('\n') {
21+
if check_next {
22+
if !line.contains("./test/unit/remap_path_prefix/panic_bin.rs:6:5") {
23+
panic!("Expected line to contain ./test/unit/remap_path_prefix/panic_bin.rs:6:5 but was {}", line);
24+
}
25+
return;
26+
}
27+
if line.contains("panic_bin::do_call") {
28+
check_next = true;
29+
}
30+
}
31+
panic!("Didn't see expected line containing panic_bin::do_call");
32+
}
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)