Move to UseSite for build_error span recovery

Author: nbdd0121

src/binary_analysis/build_error.rs

@@ -1,8 +1,11 @@
 use object::{File, Object, ObjectSection, ObjectSymbol, RelocationTarget};
 use rustc_middle::mir::mono::MonoItem;
-use rustc_middle::ty::{Instance, TyCtxt};
+use rustc_middle::ty::{Instance, TypingEnv};
 use rustc_span::Span;
 
+use crate::ctxt::AnalysisCtxt;
+use crate::diagnostic::use_stack::{UseSite, UseSiteKind};
+
 #[derive(Diagnostic)]
 #[diag(klint_build_error_referenced_without_symbol)]
 struct BuildErrorReferencedWithoutSymbol;
@@ -14,15 +17,20 @@ struct BuildErrorReferencedWithoutInstance<'a> {
 }
 
 #[derive(Diagnostic)]
-#[diag(klint_build_error_referenced)]
-struct BuildErrorReferenced<'tcx> {
+#[diag(klint_build_error_referenced_without_debug)]
+struct BuildErrorReferencedWithoutDebug<'tcx> {
     #[primary_span]
     pub span: Span,
     pub kind: &'static str,
     pub instance: Instance<'tcx>,
+    pub err: String,
 }
 
-pub fn build_error_detection<'tcx, 'obj>(tcx: TyCtxt<'tcx>, file: &File<'obj>) {
+#[derive(Diagnostic)]
+#[diag(klint_build_error_referenced)]
+struct BuildErrorReferenced;
+
+pub fn build_error_detection<'tcx, 'obj>(cx: &AnalysisCtxt<'tcx>, file: &File<'obj>) {
     let Some(build_error_symbol) = file.symbol_by_name("rust_build_error") else {
         // This object file contains no reference to `build_error`, all good!
         return;
@@ -41,55 +49,42 @@ pub fn build_error_detection<'tcx, 'obj>(tcx: TyCtxt<'tcx>, file: &File<'obj>) {
 
     // Collect all mono items, which we will use to find out which symbol is problematic.
     let mono_items = crate::monomorphize_collector::collect_crate_mono_items(
-        tcx,
+        cx.tcx,
         crate::monomorphize_collector::MonoItemCollectionStrategy::Lazy,
     )
     .0;
 
     for section in file.sections() {
         for (offset, relocation) in section.relocations() {
             if relocation.target() == relo_target_needle {
-                // Found a relocation that points to `build_error`. Emit and error.
+                // Found a relocation that points to `build_error`. Emit an error.
                 let Some((symbol, _)) =
                     super::find_symbol_from_section_offset(file, &section, offset)
                 else {
-                    tcx.dcx().emit_err(BuildErrorReferencedWithoutSymbol);
+                    cx.dcx().emit_err(BuildErrorReferencedWithoutSymbol);
                     continue;
                 };
 
                 let Some(mono) = mono_items
                     .iter()
-                    .find(|item| item.symbol_name(tcx).name == symbol)
+                    .find(|item| item.symbol_name(cx.tcx).name == symbol)
                 else {
-                    tcx.dcx()
+                    cx.dcx()
                         .emit_err(BuildErrorReferencedWithoutInstance { symbol });
                     continue;
                 };
 
-                let mut diag = tcx.dcx().create_err(match mono {
-                    MonoItem::Fn(instance) => BuildErrorReferenced {
-                        span: tcx.def_span(instance.def_id()),
-                        kind: "fn",
-                        instance: *instance,
-                    },
-                    MonoItem::Static(def_id) => BuildErrorReferenced {
-                        span: tcx.def_span(def_id),
-                        kind: "static",
-                        instance: Instance::mono(tcx, *def_id),
-                    },
-                    MonoItem::GlobalAsm(_) => {
-                        // We're not going to be covered by symbols inside global asm.
-                        bug!();
-                    }
-                });
-
                 let loader = super::dwarf::DwarfLoader::new(file)
                     .expect("DWARF loader creation should not fail");
+
+                let mut diag = cx.dcx().create_err(BuildErrorReferenced);
                 let mut frame = match mono {
-                    MonoItem::Fn(instance) => Some(*instance),
-                    _ => None,
+                    MonoItem::Fn(instance) => *instance,
+                    MonoItem::Static(def_id) => Instance::mono(cx.tcx, *def_id),
+                    MonoItem::GlobalAsm(_) => bug!(),
                 };
 
+                let mut recovered_call_stack = Vec::new();
                 let result: Result<_, super::dwarf::Error> = try {
                     let call_stack = loader.inline_info(section.index(), offset)?;
                     if let Some(first) = call_stack.first() {
@@ -100,16 +95,17 @@ pub fn build_error_detection<'tcx, 'obj>(tcx: TyCtxt<'tcx>, file: &File<'obj>) {
                         }
                     }
                     for call in call_stack {
-                        if let Some(caller) = frame.take() {
-                            if let Some((callee, span)) = super::reconstruct::recover_fn_call_span(
-                                tcx,
-                                caller,
-                                &call.callee,
-                                call.location.as_ref(),
-                            ) {
-                                frame = Some(callee);
-                                diag.span_note(span, format!("which calls `{callee}`"));
-                            }
+                        if let Some((callee, site)) = super::reconstruct::recover_fn_call_span(
+                            cx.tcx,
+                            frame,
+                            &call.callee,
+                            call.location.as_ref(),
+                        ) {
+                            recovered_call_stack.push(UseSite {
+                                instance: TypingEnv::fully_monomorphized().as_query_input(frame),
+                                kind: site,
+                            });
+                            frame = callee;
                         }
                     }
                 };
@@ -124,34 +120,60 @@ pub fn build_error_detection<'tcx, 'obj>(tcx: TyCtxt<'tcx>, file: &File<'obj>) {
                         super::dwarf::Error::UnexpectedDwarf("cannot find line number info"),
                     )?;
 
-                    if let Some(frame) = frame
-                        && let Some((_, span)) = super::reconstruct::recover_fn_call_span(
-                            tcx,
-                            frame,
-                            "rust_build_error",
-                            Some(&loc),
-                        )
-                    {
-                        diag.span_note(
-                            span,
-                            "which contains a `build_error` call that is not optimized out",
-                        );
+                    if let Some((_, site)) = super::reconstruct::recover_fn_call_span(
+                        cx.tcx,
+                        frame,
+                        "rust_build_error",
+                        Some(&loc),
+                    ) {
+                        recovered_call_stack.push(UseSite {
+                            instance: TypingEnv::fully_monomorphized().as_query_input(frame),
+                            kind: site,
+                        });
                     } else {
-                        let span = super::reconstruct::recover_span_from_line_no(tcx, &loc).ok_or(
-                            super::dwarf::Error::Other("cannot find file in compiler session"),
-                        )?;
-                        diag.span_note(
-                            span,
-                            "which contains a `build_error` reference that is not optimized out",
-                        );
+                        let span = super::reconstruct::recover_span_from_line_no(cx.tcx, &loc)
+                            .ok_or(super::dwarf::Error::Other(
+                                "cannot find file in compiler session",
+                            ))?;
+                        recovered_call_stack.push(UseSite {
+                            instance: TypingEnv::fully_monomorphized().as_query_input(frame),
+                            kind: UseSiteKind::Other(
+                                span,
+                                "which is referenced by this function".to_string(),
+                            ),
+                        })
                     }
                 };
                 if let Err(err) = result {
-                    diag.note(format!(
-                        "attempt to reconstruct line information from DWARF failed: {err}"
-                    ));
+                    diag.cancel();
+
+                    // If even line number cannot be recovered, emit a different diagnostic.
+                    cx.dcx().emit_err(match mono {
+                        MonoItem::Fn(instance) => BuildErrorReferencedWithoutDebug {
+                            span: cx.def_span(instance.def_id()),
+                            kind: "fn",
+                            instance: *instance,
+                            err: err.to_string(),
+                        },
+                        MonoItem::Static(def_id) => BuildErrorReferencedWithoutDebug {
+                            span: cx.def_span(def_id),
+                            kind: "static",
+                            instance: Instance::mono(cx.tcx, *def_id),
+                            err: err.to_string(),
+                        },
+                        MonoItem::GlobalAsm(_) => {
+                            // We're not going to be covered by symbols inside global asm.
+                            bug!();
+                        }
+                    });
+                    continue;
                 }
 
+                cx.note_use_stack(&mut diag, &recovered_call_stack);
+                diag.span_note(
+                    cx.def_span(mono.def_id()),
+                    format!("reference contained in `{}`", mono),
+                );
                 diag.emit();
             }
         }

src/binary_analysis/mod.rs

@@ -2,18 +2,19 @@ use std::fs::File;
 use std::path::Path;
 
 use object::{File as ObjectFile, Object, ObjectSection, ObjectSymbol, Section, SymbolSection};
-use rustc_middle::ty::TyCtxt;
+
+use crate::ctxt::AnalysisCtxt;
 
 mod build_error;
 mod dwarf;
 mod reconstruct;
 
-pub fn binary_analysis<'tcx>(tcx: TyCtxt<'tcx>, path: &Path) {
+pub fn binary_analysis<'tcx>(cx: &AnalysisCtxt<'tcx>, path: &Path) {
     let file = File::open(path).unwrap();
     let mmap = unsafe { rustc_data_structures::memmap::Mmap::map(file) }.unwrap();
     let object = ObjectFile::parse(&*mmap).unwrap();
 
-    build_error::build_error_detection(tcx, &object);
+    build_error::build_error_detection(cx, &object);
 }
 
 fn find_symbol_from_section_offset<'obj>(

src/binary_analysis/reconstruct.rs

@@ -2,6 +2,8 @@ use rustc_middle::ty::{Instance, TyCtxt};
 use rustc_middle::{mir, ty};
 use rustc_span::{BytePos, DUMMY_SP, FileName, Span};
 
+use crate::diagnostic::use_stack::UseSiteKind;
+
 pub fn recover_span_from_line_no<'tcx>(
     tcx: TyCtxt<'tcx>,
     location: &super::dwarf::Location,
@@ -45,11 +47,11 @@ pub fn recover_fn_call_span<'tcx>(
     caller: Instance<'tcx>,
     callee: &str,
     location: Option<&super::dwarf::Location>,
-) -> Option<(Instance<'tcx>, Span)> {
+) -> Option<(Instance<'tcx>, UseSiteKind)> {
     let mir = tcx.instance_mir(caller.def);
 
     let mut callee_instance = None;
-    let mut spans = Vec::new();
+    let mut sites = Vec::new();
 
     for block in mir.basic_blocks.iter() {
         let terminator = block.terminator();
@@ -62,7 +64,7 @@ pub fn recover_fn_call_span<'tcx>(
             continue;
         }
 
-        let instance = match terminator.kind {
+        match terminator.kind {
             mir::TerminatorKind::Call { ref func, .. }
             | mir::TerminatorKind::TailCall { ref func, .. } => {
                 let callee_ty = func.ty(mir, tcx);
@@ -75,13 +77,20 @@ pub fn recover_fn_call_span<'tcx>(
                 let ty::FnDef(def_id, args) = *callee_ty.kind() else {
                     continue;
                 };
-                ty::Instance::expect_resolve(
+
+                let instance = ty::Instance::expect_resolve(
                     tcx,
                     ty::TypingEnv::fully_monomorphized(),
                     def_id,
                     args,
                     terminator.source_info.span,
-                )
+                );
+                if tcx.symbol_name(instance).name != callee {
+                    continue;
+                }
+
+                callee_instance = Some(instance);
+                sites.push(UseSiteKind::Call(terminator.source_info.span));
             }
             mir::TerminatorKind::Drop { ref place, .. } => {
                 let ty = place.ty(mir, tcx).ty;
@@ -90,18 +99,21 @@ pub fn recover_fn_call_span<'tcx>(
                     ty::TypingEnv::fully_monomorphized(),
                     ty::EarlyBinder::bind(ty),
                 );
-                Instance::resolve_drop_in_place(tcx, ty)
+
+                let instance = Instance::resolve_drop_in_place(tcx, ty);
+                if tcx.symbol_name(instance).name != callee {
+                    continue;
+                }
+
+                callee_instance = Some(instance);
+                sites.push(UseSiteKind::Drop {
+                    drop_span: terminator.source_info.span,
+                    place_span: mir.local_decls[place.local].source_info.span,
+                });
             }
 
             _ => continue,
         };
-
-        if tcx.symbol_name(instance).name != callee {
-            continue;
-        }
-
-        callee_instance = Some(instance);
-        spans.push(terminator.source_info.span);
     }
 
     // In addition to direct function calls, we should also inspect inlined functions.
@@ -114,7 +126,7 @@ pub fn recover_fn_call_span<'tcx>(
             }
 
             callee_instance = Some(instance);
-            spans.push(span);
+            sites.push(UseSiteKind::Call(span));
         }
     }
 
@@ -124,8 +136,8 @@ pub fn recover_fn_call_span<'tcx>(
     };
 
     // If there's only a single span, then it has to be the correct span.
-    if spans.len() == 1 {
-        return Some((callee_instance, spans[0]));
+    if sites.len() == 1 {
+        return Some((callee_instance, sites.pop().unwrap()));
     }
 
     // Otherwise, we need to use the DWARF location information to find the best related span.
@@ -135,7 +147,7 @@ pub fn recover_fn_call_span<'tcx>(
             caller,
             callee_instance
         );
-        return Some((callee_instance, spans[0]));
+        return Some((callee_instance, sites.pop().unwrap()));
     };
 
     let Some(recovered_span) = recover_span_from_line_no(tcx, loc) else {
@@ -144,16 +156,16 @@ pub fn recover_fn_call_span<'tcx>(
             caller,
             callee_instance
         );
-        return Some((callee_instance, spans[0]));
+        return Some((callee_instance, sites.pop().unwrap()));
     };
 
     // Now we have a recovered span. Use this span to match spans that we have.
-    for span in spans {
-        if recover_span(recovered_span, span) {
-            return Some((callee_instance, span));
+    for site in sites {
+        if recover_span(recovered_span, site.span()) {
+            return Some((callee_instance, site));
         }
     }
 
     // No perfect match, just use the recovered span that we have.
-    Some((callee_instance, recovered_span))
+    Some((callee_instance, UseSiteKind::Call(recovered_span)))
 }

src/main.rs

@@ -126,7 +126,7 @@ impl driver::CallbacksExt for MyCallbacks {
     fn after_codegen<'tcx>(&mut self, cx: &'tcx AnalysisCtxt<'tcx>) {
         let outputs = cx.output_filenames(());
         if outputs.outputs.contains_key(&OutputType::Object) {
-            binary_analysis::binary_analysis(cx.tcx, outputs.path(OutputType::Object).as_path());
+            binary_analysis::binary_analysis(cx, outputs.path(OutputType::Object).as_path());
         }
     }
 }

src/messages.ftl

@@ -14,5 +14,9 @@ klint_build_error_referenced_without_symbol =
 klint_build_error_referenced_without_instance =
     symbol `{$symbol}` references `build_error` in the object file, but no associated instance is found
 
-klint_build_error_referenced =
+klint_build_error_referenced_without_debug =
     `{$kind} {$instance}` contains reference to `build_error`
+    .note = attempt to reconstruct line information from DWARF failed: {$err}
+
+klint_build_error_referenced =
+    this `build_error` reference is not optimized away