Skip to content

Commit 45ef927

Browse files
khueykhuey
committed
When LLVM's location discriminator value limit is exceeded, emit locations with dummy spans instead of dropping them entirely
Revert most of #133194 (except the test and the comment fixes). Then refix not emitting locations at all when the correct location discriminator value exceeds LLVM's capacity.
1 parent af952c1 commit 45ef927

File tree

8 files changed

+111
-52
lines changed

8 files changed

+111
-52
lines changed

‎compiler/rustc_codegen_gcc/src/debuginfo.rs

+9-9
Original file line numberDiff line numberDiff line change
@@ -113,15 +113,15 @@ fn make_mir_scope<'gcc, 'tcx>(
113113
let scope_data = &mir.source_scopes[scope];
114114
let parent_scope = if let Some(parent) = scope_data.parent_scope {
115115
make_mir_scope(cx, _instance, mir, variables, debug_context, instantiated, parent);
116-
debug_context.scopes[parent].unwrap()
116+
debug_context.scopes[parent]
117117
} else {
118118
// The root is the function itself.
119119
let file = cx.sess().source_map().lookup_source_file(mir.span.lo());
120-
debug_context.scopes[scope] = Some(DebugScope {
120+
debug_context.scopes[scope] = DebugScope {
121121
file_start_pos: file.start_pos,
122122
file_end_pos: file.end_position(),
123-
..debug_context.scopes[scope].unwrap()
124-
});
123+
..debug_context.scopes[scope]
124+
};
125125
instantiated.insert(scope);
126126
return;
127127
};
@@ -130,7 +130,7 @@ fn make_mir_scope<'gcc, 'tcx>(
130130
if !vars.contains(scope) && scope_data.inlined.is_none() {
131131
// Do not create a DIScope if there are no variables defined in this
132132
// MIR `SourceScope`, and it's not `inlined`, to avoid debuginfo bloat.
133-
debug_context.scopes[scope] = Some(parent_scope);
133+
debug_context.scopes[scope] = parent_scope;
134134
instantiated.insert(scope);
135135
return;
136136
}
@@ -157,12 +157,12 @@ fn make_mir_scope<'gcc, 'tcx>(
157157
// TODO(tempdragon): dbg_scope: Add support for scope extension here.
158158
inlined_at.or(p_inlined_at);
159159

160-
debug_context.scopes[scope] = Some(DebugScope {
160+
debug_context.scopes[scope] = DebugScope {
161161
dbg_scope,
162162
inlined_at,
163163
file_start_pos: loc.file.start_pos,
164164
file_end_pos: loc.file.end_position(),
165-
});
165+
};
166166
instantiated.insert(scope);
167167
}
168168

@@ -232,12 +232,12 @@ impl<'gcc, 'tcx> DebugInfoCodegenMethods<'tcx> for CodegenCx<'gcc, 'tcx> {
232232
}
233233

234234
// Initialize fn debug context (including scopes).
235-
let empty_scope = Some(DebugScope {
235+
let empty_scope = DebugScope {
236236
dbg_scope: self.dbg_scope_fn(instance, fn_abi, Some(llfn)),
237237
inlined_at: None,
238238
file_start_pos: BytePos(0),
239239
file_end_pos: BytePos(0),
240-
});
240+
};
241241
let mut fn_debug_context = FunctionDebugContext {
242242
scopes: IndexVec::from_elem(empty_scope, mir.source_scopes.as_slice()),
243243
inlined_function_scopes: Default::default(),

‎compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs

+22-37
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use rustc_middle::mir::{Body, SourceScope};
99
use rustc_middle::ty::layout::{FnAbiOf, HasTypingEnv};
1010
use rustc_middle::ty::{self, Instance};
1111
use rustc_session::config::DebugInfo;
12-
use rustc_span::{BytePos, hygiene};
12+
use rustc_span::{BytePos, DUMMY_SP, hygiene};
1313

1414
use super::metadata::file_metadata;
1515
use super::utils::DIB;
@@ -85,23 +85,15 @@ fn make_mir_scope<'ll, 'tcx>(
8585
discriminators,
8686
parent,
8787
);
88-
if let Some(parent_scope) = debug_context.scopes[parent] {
89-
parent_scope
90-
} else {
91-
// If the parent scope could not be represented then no children
92-
// can be either.
93-
debug_context.scopes[scope] = None;
94-
instantiated.insert(scope);
95-
return;
96-
}
88+
debug_context.scopes[parent]
9789
} else {
9890
// The root is the function itself.
9991
let file = cx.sess().source_map().lookup_source_file(mir.span.lo());
100-
debug_context.scopes[scope] = Some(DebugScope {
92+
debug_context.scopes[scope] = DebugScope {
10193
file_start_pos: file.start_pos,
10294
file_end_pos: file.end_position(),
103-
..debug_context.scopes[scope].unwrap()
104-
});
95+
..debug_context.scopes[scope]
96+
};
10597
instantiated.insert(scope);
10698
return;
10799
};
@@ -112,7 +104,7 @@ fn make_mir_scope<'ll, 'tcx>(
112104
{
113105
// Do not create a DIScope if there are no variables defined in this
114106
// MIR `SourceScope`, and it's not `inlined`, to avoid debuginfo bloat.
115-
debug_context.scopes[scope] = Some(parent_scope);
107+
debug_context.scopes[scope] = parent_scope;
116108
instantiated.insert(scope);
117109
return;
118110
}
@@ -145,14 +137,7 @@ fn make_mir_scope<'ll, 'tcx>(
145137
},
146138
};
147139

148-
let mut debug_scope = Some(DebugScope {
149-
dbg_scope,
150-
inlined_at: parent_scope.inlined_at,
151-
file_start_pos: loc.file.start_pos,
152-
file_end_pos: loc.file.end_position(),
153-
});
154-
155-
if let Some((_, callsite_span)) = scope_data.inlined {
140+
let inlined_at = scope_data.inlined.map(|(_, callsite_span)| {
156141
let callsite_span = hygiene::walk_chain_collapsed(callsite_span, mir.span);
157142
let callsite_scope = parent_scope.adjust_dbg_scope_for_span(cx, callsite_span);
158143
let loc = cx.dbg_loc(callsite_scope, parent_scope.inlined_at, callsite_span);
@@ -175,29 +160,29 @@ fn make_mir_scope<'ll, 'tcx>(
175160
// Note further that we can't key this hashtable on the span itself,
176161
// because these spans could have distinct SyntaxContexts. We have
177162
// to key on exactly what we're giving to LLVM.
178-
let inlined_at = match discriminators.entry(callsite_span.lo()) {
163+
match discriminators.entry(callsite_span.lo()) {
179164
Entry::Occupied(mut o) => {
180165
*o.get_mut() += 1;
166+
// NB: We have to emit *something* here or we'll fail LLVM IR verification
167+
// in at least some circumstances (see issue #135322) so if the required
168+
// discriminant cannot be encoded fall back to the dummy location.
181169
unsafe { llvm::LLVMRustDILocationCloneWithBaseDiscriminator(loc, *o.get()) }
170+
.unwrap_or_else(|| {
171+
cx.dbg_loc(callsite_scope, parent_scope.inlined_at, DUMMY_SP)
172+
})
182173
}
183174
Entry::Vacant(v) => {
184175
v.insert(0);
185-
Some(loc)
186-
}
187-
};
188-
match inlined_at {
189-
Some(inlined_at) => {
190-
debug_scope.as_mut().unwrap().inlined_at = Some(inlined_at);
191-
}
192-
None => {
193-
// LLVM has a maximum discriminator that it can encode (currently
194-
// it uses 12 bits for 4096 possible values). If we exceed that
195-
// there is little we can do but drop the debug info.
196-
debug_scope = None;
176+
loc
197177
}
198178
}
199-
}
179+
});
200180

201-
debug_context.scopes[scope] = debug_scope;
181+
debug_context.scopes[scope] = DebugScope {
182+
dbg_scope,
183+
inlined_at: inlined_at.or(parent_scope.inlined_at),
184+
file_start_pos: loc.file.start_pos,
185+
file_end_pos: loc.file.end_position(),
186+
};
202187
instantiated.insert(scope);
203188
}

‎compiler/rustc_codegen_llvm/src/debuginfo/mod.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -295,12 +295,12 @@ impl<'ll, 'tcx> DebugInfoCodegenMethods<'tcx> for CodegenCx<'ll, 'tcx> {
295295
}
296296

297297
// Initialize fn debug context (including scopes).
298-
let empty_scope = Some(DebugScope {
298+
let empty_scope = DebugScope {
299299
dbg_scope: self.dbg_scope_fn(instance, fn_abi, Some(llfn)),
300300
inlined_at: None,
301301
file_start_pos: BytePos(0),
302302
file_end_pos: BytePos(0),
303-
});
303+
};
304304
let mut fn_debug_context = FunctionDebugContext {
305305
scopes: IndexVec::from_elem(empty_scope, &mir.source_scopes),
306306
inlined_function_scopes: Default::default(),

‎compiler/rustc_codegen_ssa/src/mir/debuginfo.rs

+2-4
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,7 @@ use crate::traits::*;
1919

2020
pub struct FunctionDebugContext<'tcx, S, L> {
2121
/// Maps from source code to the corresponding debug info scope.
22-
/// May be None if the backend is not capable of representing the scope for
23-
/// some reason.
24-
pub scopes: IndexVec<mir::SourceScope, Option<DebugScope<S, L>>>,
22+
pub scopes: IndexVec<mir::SourceScope, DebugScope<S, L>>,
2523

2624
/// Maps from an inlined function to its debug info declaration.
2725
pub inlined_function_scopes: FxHashMap<Instance<'tcx>, S>,
@@ -232,7 +230,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
232230
&self,
233231
source_info: mir::SourceInfo,
234232
) -> Option<(Bx::DIScope, Option<Bx::DILocation>, Span)> {
235-
let scope = &self.debug_context.as_ref()?.scopes[source_info.scope]?;
233+
let scope = &self.debug_context.as_ref()?.scopes[source_info.scope];
236234
let span = hygiene::walk_chain_collapsed(source_info.span, self.mir.span);
237235
Some((scope.adjust_dbg_scope_for_span(self.cx, span), scope.inlined_at, span))
238236
}

‎tests/run-make/llvm-location-discriminator-limit-dummy-span/main.rs

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
fn main() {
2+
other::big_function();
3+
}

‎tests/run-make/llvm-location-discriminator-limit-dummy-span/other.rs

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
proc::declare_big_function!();

‎tests/run-make/llvm-location-discriminator-limit-dummy-span/proc.rs

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
extern crate proc_macro;
2+
use proc_macro::TokenStream;
3+
4+
#[proc_macro]
5+
pub fn declare_big_function(_input: TokenStream) -> TokenStream {
6+
include_str!("./generated.rs").parse().unwrap()
7+
}

‎tests/run-make/llvm-location-discriminator-limit-dummy-span/rmake.rs

+65
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
//! Regression test for <https://github.com/rust-lang/rust/issues/135332>.
2+
//!
3+
//! We can't simply drop debuginfo location spans when LLVM's location discriminator value limit is
4+
//! reached. Otherwise, with `-Z verify-llvm-ir` and fat LTO, LLVM will report a broken module for
5+
//!
6+
//! ```text
7+
//! inlinable function call in a function with debug info must have a !dbg location
8+
//! ```
9+
10+
//@ ignore-cross-compile
11+
//@ needs-dynamic-linking
12+
//@ only-nightly (requires unstable rustc flag)
13+
14+
#![deny(warnings)]
15+
16+
use run_make_support::{dynamic_lib_name, rfs, rust_lib_name, rustc};
17+
18+
// Synthesize a function that will have a large (`n`) number of functions
19+
// MIR-inlined into it. When combined with a proc-macro, all of these inline
20+
// callsites will have the same span, forcing rustc to use the DWARF
21+
// discriminator to distinguish between them. LLVM's capacity to store that
22+
// discriminator is not infinite (currently it allocates 12 bits for a
23+
// maximum value of 4096) so if this function gets big enough rustc's error
24+
// handling path will be exercised.
25+
fn generate_program(n: u32) -> String {
26+
let mut program = String::from("pub type BigType = Vec<Vec<String>>;\n\n");
27+
program.push_str("pub fn big_function() -> BigType {\n");
28+
program.push_str(" vec![\n");
29+
for i in 1..=n {
30+
program.push_str(&format!("vec![\"string{}\".to_owned()],\n", i));
31+
}
32+
program.push_str(" ]\n");
33+
program.push_str("}\n");
34+
program
35+
}
36+
37+
fn main() {
38+
// The reported threshold is around 1366 (4096/3), but let's bump it to
39+
// around 1500 to be less sensitive.
40+
rfs::write("generated.rs", generate_program(1500));
41+
42+
rustc()
43+
.input("proc.rs")
44+
.crate_type("proc-macro")
45+
.edition("2021")
46+
.arg("-Cdebuginfo=line-tables-only")
47+
.run();
48+
rustc()
49+
.extern_("proc", dynamic_lib_name("proc"))
50+
.input("other.rs")
51+
.crate_type("rlib")
52+
.edition("2021")
53+
.opt_level("3")
54+
.arg("-Cdebuginfo=line-tables-only")
55+
.run();
56+
rustc()
57+
.extern_("other", rust_lib_name("other"))
58+
.input("main.rs")
59+
.edition("2021")
60+
.opt_level("3")
61+
.arg("-Cdebuginfo=line-tables-only")
62+
.arg("-Clto=fat")
63+
.arg("-Zverify-llvm-ir")
64+
.run();
65+
}

0 commit comments

Comments
 (0)