Skip to content

Commit 156887d

Browse files
LegNeatoLegNeato
committed
codegen: harden panic message decoding for nightly call shapes
1 parent 899a7b0 commit 156887d

3 files changed

Lines changed: 164 additions & 161 deletions

File tree

‎crates/rustc_codegen_spirv/src/builder/format_args_decompiler.rs‎

Lines changed: 150 additions & 157 deletions
Original file line numberDiff line numberDiff line change
@@ -200,12 +200,10 @@ impl<'tcx> DecodedFormatArgs<'tcx> {
200200
SpirvValue {
201201
kind: SpirvValueKind::Def(a_id),
202202
ty: a_ty,
203-
..
204203
},
205204
SpirvValue {
206205
kind: SpirvValueKind::Def(b_id),
207206
ty: b_ty,
208-
..
209207
},
210208
ref other_args @ ..,
211209
] = args[..]
@@ -238,12 +236,10 @@ impl<'tcx> DecodedFormatArgs<'tcx> {
238236
SpirvValue {
239237
kind: SpirvValueKind::Def(template_id),
240238
ty: template_ty_id,
241-
..
242239
},
243240
SpirvValue {
244241
kind: SpirvValueKind::Def(rt_args_or_tagged_len_id),
245242
ty: rt_args_or_tagged_len_ty_id,
246-
..
247243
},
248244
ref trailing @ ..,
249245
] if trailing.len() <= 2
@@ -462,165 +458,98 @@ impl<'tcx> DecodedFormatArgs<'tcx> {
462458
FormatArgsNotRecognized("fmt::Arguments::new callee not registered".into())
463459
})
464460
};
465-
let (ctor, call_args_storage) = if let Some((
466-
template_id,
467-
template_ty_id,
468-
rt_args_ptr_id,
469-
rt_args_ptr_ty_id,
470-
)) = split_fmt_args
471-
{
472-
let ctor = if let (Some(template_len), Some(rt_args_count)) = (
473-
const_ptr_to_composite_len(template_id)
474-
.or_else(|| array_len_from_ptr_type(template_ty_id)),
475-
const_ptr_to_composite_len(rt_args_ptr_id)
476-
.or_else(|| array_len_from_ptr_type(rt_args_ptr_ty_id)),
477-
) {
478-
FmtArgsCtor::NewTemplate {
479-
template_len,
480-
rt_args_count,
481-
}
482-
} else if let Some(&[Inst::Call(_, callee_id, ref call_args)]) =
483-
try_rev_take(-1).as_deref()
484-
&& call_args.len() == 2
485-
&& [call_args[0], call_args[1]] == [template_id, rt_args_ptr_id]
486-
{
487-
// Consume the matched call instruction.
488-
try_rev_take(1).unwrap();
489-
lookup_fmt_args_ctor(callee_id)?
490-
} else {
491-
// We failed to recover constructor metadata for an already-split
492-
// `fmt::Arguments` value. Keep panic lowering sound by falling
493-
// back to an unknown panic message, without requiring decompilation.
494-
return Ok(decoded_format_args);
495-
};
496-
497-
(
498-
ctor,
499-
SmallVec::<[Word; 8]>::from_slice(&[template_id, rt_args_ptr_id]),
500-
)
501-
} else {
502-
// Newer rustc can pass the `fmt::Arguments::new_*` result directly to
503-
// panic entry points (single trailing call), while older versions go
504-
// through a local temporary (`Call -> Store -> Load`).
505-
let fmt_args_new_call_inst_count = if matches!(
506-
try_rev_take(-3).as_deref(),
507-
Some(&[Inst::Call(_, _, _), Inst::Store(_, _), Inst::Load(_, _)])
508-
) {
509-
3
510-
} else if let Some(&[Inst::Call(call_ret_id, callee_id, _)]) =
511-
try_rev_take(-1).as_deref()
461+
let (ctor, call_args_storage) =
462+
if let Some((template_id, template_ty_id, rt_args_ptr_id, rt_args_ptr_ty_id)) =
463+
split_fmt_args
512464
{
513-
if call_ret_id == format_args_id
514-
|| cx.fmt_args_new_fn_ids.borrow().contains_key(&callee_id)
465+
let ctor = if let (Some(template_len), Some(rt_args_count)) = (
466+
const_ptr_to_composite_len(template_id)
467+
.or_else(|| array_len_from_ptr_type(template_ty_id)),
468+
const_ptr_to_composite_len(rt_args_ptr_id)
469+
.or_else(|| array_len_from_ptr_type(rt_args_ptr_ty_id)),
470+
) {
471+
FmtArgsCtor::NewTemplate {
472+
template_len,
473+
rt_args_count,
474+
}
475+
} else if let Some(&[Inst::Call(_, callee_id, ref call_args)]) =
476+
try_rev_take(-1).as_deref()
477+
&& call_args.len() == 2
478+
&& [call_args[0], call_args[1]] == [template_id, rt_args_ptr_id]
515479
{
516-
1
480+
// Consume the matched call instruction.
481+
try_rev_take(1).unwrap();
482+
lookup_fmt_args_ctor(callee_id)?
517483
} else {
518-
0
519-
}
520-
} else if matches!(
521-
try_rev_take(-5).as_deref(),
522-
Some(&[
523-
Inst::Call(call_ret_id, _, _),
524-
Inst::CompositeExtract(extracted0, from0, 0),
525-
Inst::CompositeExtract(extracted1, from1, 1),
526-
Inst::CompositeInsert(inserted0, value0, _, 0),
527-
Inst::CompositeInsert(inserted1, value1, inserted0_prev, 1),
528-
]) if [from0, from1] == [call_ret_id; 2]
529-
&& [value0, value1] == [extracted0, extracted1]
530-
&& inserted0 == inserted0_prev
531-
&& inserted1 == format_args_id
532-
) {
533-
5
534-
} else if try_rev_take(-1).is_some() {
535-
0
536-
} else {
537-
// HACK(eddyb) gather context for new call patterns before bailing.
538-
let mut insts = SmallVec::<[Inst<Word>; 32]>::new();
539-
while let Some(extra_inst) = try_rev_take(1) {
540-
insts.extend(extra_inst);
541-
if insts.len() >= 32 {
542-
break;
543-
}
544-
}
545-
insts.reverse();
546-
547-
if insts.is_empty() {
548-
return Ok(decoded_format_args);
549-
}
550-
if !insts.iter().any(|inst| matches!(inst, Inst::Call(_, _, _))) {
484+
// We failed to recover constructor metadata for an already-split
485+
// `fmt::Arguments` value. Keep panic lowering sound by falling
486+
// back to an unknown panic message, without requiring decompilation.
551487
return Ok(decoded_format_args);
552-
}
553-
return Err(FormatArgsNotRecognized(format!(
554-
"fmt::Arguments::new call sequence ({insts:?})",
555-
)));
556-
};
557-
if fmt_args_new_call_inst_count == 0 {
558-
// HACK(eddyb) gather context for new call patterns before bailing.
559-
let mut insts = SmallVec::<[Inst<Word>; 32]>::new();
560-
while let Some(extra_inst) = try_rev_take(1) {
561-
insts.extend(extra_inst);
562-
if insts.len() >= 32 {
563-
break;
564-
}
565-
}
566-
insts.reverse();
488+
};
567489

568-
if insts.is_empty() {
569-
return Ok(decoded_format_args);
570-
}
571-
if !insts.iter().any(|inst| matches!(inst, Inst::Call(_, _, _))) {
572-
return Ok(decoded_format_args);
573-
}
574-
return Err(FormatArgsNotRecognized(format!(
575-
"fmt::Arguments::new call sequence ({insts:?})",
576-
)));
577-
}
578-
let fmt_args_new_call_insts = try_rev_take(fmt_args_new_call_inst_count).unwrap();
579-
match fmt_args_new_call_insts[..] {
580-
[
581-
Inst::Call(call_ret_id, callee_id, ref call_args),
582-
Inst::Store(st_dst_id, st_val_id),
583-
Inst::Load(ld_val_id, ld_src_id),
584-
] if call_ret_id == st_val_id
585-
&& st_dst_id == ld_src_id
586-
&& ld_val_id == format_args_id =>
490+
(
491+
ctor,
492+
SmallVec::<[Word; 8]>::from_slice(&[template_id, rt_args_ptr_id]),
493+
)
494+
} else {
495+
// Newer rustc can pass the `fmt::Arguments::new_*` result directly to
496+
// panic entry points (single trailing call), while older versions go
497+
// through a local temporary (`Call -> Store -> Load`).
498+
let fmt_args_new_call_inst_count = if matches!(
499+
try_rev_take(-3).as_deref(),
500+
Some(&[Inst::Call(_, _, _), Inst::Store(_, _), Inst::Load(_, _)])
501+
) {
502+
3
503+
} else if let Some(&[Inst::Call(call_ret_id, callee_id, _)]) =
504+
try_rev_take(-1).as_deref()
587505
{
588-
require_local_var(st_dst_id, "fmt::Arguments::new destination")?;
589-
590-
(
591-
lookup_fmt_args_ctor(callee_id)?,
592-
call_args.iter().copied().collect(),
593-
)
594-
}
595-
[Inst::Call(call_ret_id, callee_id, ref call_args)]
596506
if call_ret_id == format_args_id
597-
|| cx.fmt_args_new_fn_ids.borrow().contains_key(&callee_id) =>
598-
{
599-
(
600-
lookup_fmt_args_ctor(callee_id)?,
601-
call_args.iter().copied().collect(),
602-
)
603-
}
604-
[
605-
Inst::Call(call_ret_id, callee_id, ref call_args),
606-
Inst::CompositeExtract(extracted0, from0, 0),
607-
Inst::CompositeExtract(extracted1, from1, 1),
608-
Inst::CompositeInsert(inserted0, value0, _, 0),
609-
Inst::CompositeInsert(inserted1, value1, inserted0_prev, 1),
610-
] if [from0, from1] == [call_ret_id; 2]
611-
&& [value0, value1] == [extracted0, extracted1]
612-
&& inserted0 == inserted0_prev
613-
&& inserted1 == format_args_id =>
614-
{
615-
(
616-
lookup_fmt_args_ctor(callee_id)?,
617-
call_args.iter().copied().collect(),
618-
)
619-
}
620-
_ => {
621-
// HACK(eddyb) this gathers more context before reporting.
622-
let mut insts = fmt_args_new_call_insts;
507+
|| cx.fmt_args_new_fn_ids.borrow().contains_key(&callee_id)
508+
{
509+
1
510+
} else {
511+
0
512+
}
513+
} else if matches!(
514+
try_rev_take(-5).as_deref(),
515+
Some(&[
516+
Inst::Call(call_ret_id, _, _),
517+
Inst::CompositeExtract(extracted0, from0, 0),
518+
Inst::CompositeExtract(extracted1, from1, 1),
519+
Inst::CompositeInsert(inserted0, value0, _, 0),
520+
Inst::CompositeInsert(inserted1, value1, inserted0_prev, 1),
521+
]) if [from0, from1] == [call_ret_id; 2]
522+
&& [value0, value1] == [extracted0, extracted1]
523+
&& inserted0 == inserted0_prev
524+
&& inserted1 == format_args_id
525+
) {
526+
5
527+
} else if try_rev_take(-1).is_some() {
528+
0
529+
} else {
530+
// HACK(eddyb) gather context for new call patterns before bailing.
531+
let mut insts = SmallVec::<[Inst<Word>; 32]>::new();
532+
while let Some(extra_inst) = try_rev_take(1) {
533+
insts.extend(extra_inst);
534+
if insts.len() >= 32 {
535+
break;
536+
}
537+
}
623538
insts.reverse();
539+
540+
if insts.is_empty() {
541+
return Ok(decoded_format_args);
542+
}
543+
if !insts.iter().any(|inst| matches!(inst, Inst::Call(_, _, _))) {
544+
return Ok(decoded_format_args);
545+
}
546+
return Err(FormatArgsNotRecognized(format!(
547+
"fmt::Arguments::new call sequence ({insts:?})",
548+
)));
549+
};
550+
if fmt_args_new_call_inst_count == 0 {
551+
// HACK(eddyb) gather context for new call patterns before bailing.
552+
let mut insts = SmallVec::<[Inst<Word>; 32]>::new();
624553
while let Some(extra_inst) = try_rev_take(1) {
625554
insts.extend(extra_inst);
626555
if insts.len() >= 32 {
@@ -629,12 +558,76 @@ impl<'tcx> DecodedFormatArgs<'tcx> {
629558
}
630559
insts.reverse();
631560

561+
if insts.is_empty() {
562+
return Ok(decoded_format_args);
563+
}
564+
if !insts.iter().any(|inst| matches!(inst, Inst::Call(_, _, _))) {
565+
return Ok(decoded_format_args);
566+
}
632567
return Err(FormatArgsNotRecognized(format!(
633568
"fmt::Arguments::new call sequence ({insts:?})",
634569
)));
635570
}
636-
}
637-
};
571+
let fmt_args_new_call_insts = try_rev_take(fmt_args_new_call_inst_count).unwrap();
572+
match fmt_args_new_call_insts[..] {
573+
[
574+
Inst::Call(call_ret_id, callee_id, ref call_args),
575+
Inst::Store(st_dst_id, st_val_id),
576+
Inst::Load(ld_val_id, ld_src_id),
577+
] if call_ret_id == st_val_id
578+
&& st_dst_id == ld_src_id
579+
&& ld_val_id == format_args_id =>
580+
{
581+
require_local_var(st_dst_id, "fmt::Arguments::new destination")?;
582+
583+
(
584+
lookup_fmt_args_ctor(callee_id)?,
585+
call_args.iter().copied().collect(),
586+
)
587+
}
588+
[Inst::Call(call_ret_id, callee_id, ref call_args)]
589+
if call_ret_id == format_args_id
590+
|| cx.fmt_args_new_fn_ids.borrow().contains_key(&callee_id) =>
591+
{
592+
(
593+
lookup_fmt_args_ctor(callee_id)?,
594+
call_args.iter().copied().collect(),
595+
)
596+
}
597+
[
598+
Inst::Call(call_ret_id, callee_id, ref call_args),
599+
Inst::CompositeExtract(extracted0, from0, 0),
600+
Inst::CompositeExtract(extracted1, from1, 1),
601+
Inst::CompositeInsert(inserted0, value0, _, 0),
602+
Inst::CompositeInsert(inserted1, value1, inserted0_prev, 1),
603+
] if [from0, from1] == [call_ret_id; 2]
604+
&& [value0, value1] == [extracted0, extracted1]
605+
&& inserted0 == inserted0_prev
606+
&& inserted1 == format_args_id =>
607+
{
608+
(
609+
lookup_fmt_args_ctor(callee_id)?,
610+
call_args.iter().copied().collect(),
611+
)
612+
}
613+
_ => {
614+
// HACK(eddyb) this gathers more context before reporting.
615+
let mut insts = fmt_args_new_call_insts;
616+
insts.reverse();
617+
while let Some(extra_inst) = try_rev_take(1) {
618+
insts.extend(extra_inst);
619+
if insts.len() >= 32 {
620+
break;
621+
}
622+
}
623+
insts.reverse();
624+
625+
return Err(FormatArgsNotRecognized(format!(
626+
"fmt::Arguments::new call sequence ({insts:?})",
627+
)));
628+
}
629+
}
630+
};
638631
let call_args = call_args_storage.as_slice();
639632
enum PiecesSource {
640633
Slice { ptr_id: Word, len: usize },

‎crates/rustc_codegen_spirv/src/codegen_cx/declare.rs‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -230,9 +230,19 @@ impl<'tcx> CodegenCx<'tcx> {
230230
}
231231
let is_nonlocal = !def_id.is_local();
232232
let is_nonlocal_core = is_nonlocal && self.tcx.crate_name(def_id.krate) == sym::core;
233-
let is_spirv_std_byte_addressable_bounds_check = is_nonlocal
234-
&& self.tcx.def_path_str(def_id) == "spirv_std::byte_addressable_buffer::bounds_check";
235-
if (is_nonlocal_core && demangled_symbol_name.ends_with("::bounds_check"))
233+
let nonlocal_def_path = is_nonlocal.then(|| self.tcx.def_path_str(def_id));
234+
let is_nonlocal_core_bounds_check = is_nonlocal_core
235+
&& nonlocal_def_path
236+
.as_deref()
237+
.is_some_and(|path| path.ends_with("::bounds_check"));
238+
let is_nonlocal_core_precondition_check = is_nonlocal_core
239+
&& nonlocal_def_path
240+
.as_deref()
241+
.is_some_and(|path| path.ends_with("::precondition_check"));
242+
let is_spirv_std_byte_addressable_bounds_check = nonlocal_def_path.as_deref()
243+
== Some("spirv_std::byte_addressable_buffer::bounds_check");
244+
if is_nonlocal_core_bounds_check
245+
|| is_nonlocal_core_precondition_check
236246
|| is_spirv_std_byte_addressable_bounds_check
237247
{
238248
self.panic_entry_points.borrow_mut().insert(def_id);

‎tests/compiletests/ui/dis/panic_builtin_bounds_check.stderr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ OpExtension "SPV_KHR_non_semantic_info"
44
OpMemoryModel Logical Simple
55
OpEntryPoint Fragment %2 "main"
66
OpExecutionMode %2 OriginUpperLeft
7-
%3 = OpString "/n[Rust panicked at $DIR/panic_builtin_bounds_check.rs:27:5]/n <unknown message> (failed to find/decode `format_args!` expansion)/n in main()/n"
7+
%3 = OpString "/n[Rust panicked at $DIR/panic_builtin_bounds_check.rs:27:5]/n <unknown message>/n in main()/n"
88
%4 = OpString "$DIR/panic_builtin_bounds_check.rs"
99
OpDecorate %5 ArrayStride 4
1010
%6 = OpTypeVoid

0 commit comments

Comments
 (0)