How to report extra information on crashes from global state without invalidating a pointer

code review
1

Hi, I'm building a compiler for my new language "SUS". Since it's a rather complex project, it will be the case that releases still contain ICEs. To streamline the error reporting process, I save the input data for the compiler to a folder in a special crash_reports directory. Also, to aid myself in debugging I have added a panic handler that prints the last 10 locations in source files that the compiler looked at last. (Tracked using span.debug() calls at various places in the codebase.)

For reporting spans, I want it to happen before the debugger triggers the "on panic" breakpoint, to be able to easily see what kind of things the compiler was working on right before the panic happened. I can only get this if I use panic::set_hook, because panic::catch_unwind only runs after my debugger breakpoint.

The issue is, I can only get the panic::set_hook version to work by storing a static *const FileData. I'm of course editing the compiler state through a &mut, and looking at [Do reference accesses really invalidate pointers?] this would invalidate that pointer. Now, it works and I've been using this method for a while, but is there any better (non-technically-UB) way? Is there some way I could implement it in an (acceptably unsafe) manner to be more similar to create_dump_on_panic?

Relevant code: (here it is, in context)

/// Set up the hook to print spans. Uses [std::panic::set_hook] instead of [std::panic::catch_unwind] because this runs before my debugger "on panic" breakpoint.
pub fn setup_span_panic_handler() {
    let default_hook = std::panic::take_hook();
    std::panic::set_hook(Box::new(move |info| {
        default_hook(info);

        DEBUG_STACK.with_borrow(|history| {
            if let Some(last_stack_elem) = history.debug_stack.last() {
                eprintln!(
                    "{}",
                    format!(
                        "Panic happened in Span-guarded context {} in {}",
                        last_stack_elem.stage.red(),
                        last_stack_elem.global_obj_name.red()
                    )
                    .red()
                );
                let file_data = unsafe { &*last_stack_elem.file_data };
                //pretty_print_span(file_data, span, label);
                print_most_recent_spans(file_data, last_stack_elem);
            } else {
                eprintln!("{}", "No Span-guarding context".red());
            }
            eprintln!("{}", "Most recent available debug paths:".red());
            for (ctx, d) in &history.recent_debug_options {
                if let Some(ctx) = ctx {
                    eprintln!("{}", format!("--debug-whitelist {ctx} --debug {d}").red());
                } else {
                    eprintln!("{}", format!("(no SpanDebugger Context) --debug {d}").red());
                }
            }
        })
    }));
}

pub fn create_dump_on_panic<R>(linker: &mut Linker, f: impl FnOnce(&mut Linker) -> R) -> R {
    if crate::config::config().no_redump {
        // Run without protection, don't create a dump on panic
        return f(linker);
    }

    use std::fs;
    use std::io::Write;

    let panic_info = match std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| f(linker))) {
        Ok(result) => return result,
        Err(panic_info) => panic_info,
    };
    // Get ./sus_crash_dumps/{timestamp}
    let cur_time = chrono::Local::now()
        .format("_%Y-%m-%d_%H:%M:%S")
        .to_string();

    // create crash_dump files ... 
}

#[derive(Debug)]
struct SpanDebuggerStackElement {
    stage: &'static str,
    global_obj_name: String,
    debugging_enabled: bool,
    span_history: CircularBuffer<SPAN_TOUCH_HISTORY_SIZE, Span>,
    file_data: *const FileData,
}

thread_local! {
    static DEBUG_STACK : RefCell<PerThreadDebugInfo> = const { RefCell::new(PerThreadDebugInfo{debug_stack: Vec::new(), recent_debug_options: CircularBuffer::new()}) };
    static MOST_RECENT_FILE_DATA: std::sync::atomic::AtomicPtr<FileData> = const {AtomicPtr::new(std::ptr::null_mut())}
}

/// Register a [crate::file_position::Span] for printing by [SpanDebugger] on panic.
pub fn add_debug_span(sp: Span) {
    // Convert to range so we don't invoke any of Span's triggers
    DEBUG_STACK.with_borrow_mut(|history| {
        let Some(last) = history.debug_stack.last_mut() else {
            return; // Can't track Spans not in a SpanDebugger region
        };

        last.span_history.push_back(sp);
    });
}
2

If the &mut is created from the raw pointer, then the pointer is not invalidated.

3

Oh interesting, so you're saying that I can implement create_dump_on_panic as:

static SAVED_PTR : Mutex<Option<*mut Linker>>;
fn create_dump_on_panic<R>(linker: &mut Linker, f: FnOnce(&mut Linker) -> R) -> R {
  let linker_ptr: *mut Linker = linker as *mut Linker;
  
  {
    let guard = SAVED_PTR.lock();
    *guard.get() = Some(linker_ptr);
  }

  let ptr_back_to_mut_ref : &mut Linker = unsafe{&mut *linker_ptr};
  
  //Now just call f, no more need for panic::catch_unwind, everything is in panic::set_hook
  let r = f(ptr_back_to_mut_ref);

  {
    let guard = SAVED_PTR.lock();
    *guard.get() = None; // unset so we don't have dangling pointer
  }

  r
}

Is this the approach other projects use? Or will it bite me in the future sometime?

4

Yes, as long as you never create multiple mutable references at the same time.

For example if a() creates a mutable reference and then calls b() that also creates a mutable reference, then the one in a() must not be used after the call to b().

5

Yes, the mutable borrow exclusion rule. What about pointers derived from this stored *mut? Say instead of storing linker_ptr: *mut Linker, I stored a *const FileData (a ptr to a field of linker_ptr). Would it be that as long as I created both the ptr_back_to_mut_ref: &mut Linker from the original ptr, and the *const FileData from the original ptr, that the *const FileData becomes valid to access again once the ptr_back_to_mut_ref: &mut Linker has gone out of scope?

Something like:

static SAVED_PTR : Mutex<Option<*const FileData>>;
fn set_panic_hook() {
  std::panic::set_hook(|_info| {
    let guard = SAVED_PTR.lock();
    if let Some(file_data_ptr) = guard.get() {
      // SAFETY: SAVED_PTR will only ever contain a valid pointer iff this panic is handled while within create_dump_on_panic, which in turn makes sure that any conflicting &mut Linker will have been disposed of. 
      let file_data: &FileData = unsafe {&*file_data_ptr};
      // Do stuff with file_data, print stuff, etc
    }
  });
}
fn create_dump_on_panic<R>(linker: &mut Linker, f: FnOnce(&mut Linker) -> R) -> R {
  let linker_ptr: *mut Linker = linker as *mut Linker;
  
  {
    let guard = SAVED_PTR.lock();
    let file_data_ptr: *const FileData = unsafe {&raw (*linker_ptr).file_data};
    *guard.get() = Some(file_data_ptr);
  }

  // This may panic, in which case the pointer as mut ref is no longer in scope, and SAVED_PTR can be safely accessed. 
  // SAFETY see [set_panic_hook]
  let r = f(unsafe{&mut *linker_ptr});

  {
    let guard = SAVED_PTR.lock();
    *guard.get() = None; // unset so we don't have dangling pointer
  }

  r
}

If this is not allowed, how come my previous example was allowed? Couldn't it be argued that the pointer stored in SAVED_PTR is "derived" from linker_ptr? What if I cast the *mut Linker to *const Linker and store that? Is that an operation that creates a derived pointer instead of "it's the same pointer"?