Fix invalid drop impl call in Report::downcast (#143)

The context chain downcast called converted to the wrong inner type
which caused the wrong drop impl to be called.

The tests did not catch this because they had compatible drop
implementations due to matching type layout. Solved by swizzling the
fields of the chain test types to force incompatible layouts

Resolves: #141
This commit is contained in:
Freja Roberts 2023-12-30 02:32:31 +01:00 committed by GitHub
parent 32d84dcbac
commit 770ac3fa14
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 15 additions and 10 deletions

View File

@ -698,13 +698,13 @@ where
// ptr::read to take ownership of that value. // ptr::read to take ownership of that value.
if TypeId::of::<D>() == target { if TypeId::of::<D>() == target {
unsafe { unsafe {
e.cast::<ErrorImpl<ContextError<ManuallyDrop<E>, E>>>() e.cast::<ErrorImpl<ContextError<ManuallyDrop<D>, E>>>()
.into_box() .into_box()
}; };
} else { } else {
debug_assert_eq!(TypeId::of::<E>(), target); debug_assert_eq!(TypeId::of::<E>(), target);
unsafe { unsafe {
e.cast::<ErrorImpl<ContextError<E, ManuallyDrop<E>>>>() e.cast::<ErrorImpl<ContextError<D, ManuallyDrop<E>>>>()
.into_box() .into_box()
}; };
} }

View File

@ -24,11 +24,13 @@ impl Flag {
#[derive(Debug)] #[derive(Debug)]
pub struct DetectDrop { pub struct DetectDrop {
has_dropped: Flag, has_dropped: Flag,
label: &'static str,
} }
impl DetectDrop { impl DetectDrop {
pub fn new(has_dropped: &Flag) -> Self { pub fn new(label: &'static str, has_dropped: &Flag) -> Self {
DetectDrop { DetectDrop {
label,
has_dropped: Flag { has_dropped: Flag {
atomic: Arc::clone(&has_dropped.atomic), atomic: Arc::clone(&has_dropped.atomic),
}, },
@ -46,6 +48,7 @@ impl Display for DetectDrop {
impl Drop for DetectDrop { impl Drop for DetectDrop {
fn drop(&mut self) { fn drop(&mut self) {
eprintln!("Dropping {}", self.label);
let already_dropped = self.has_dropped.atomic.swap(true, SeqCst); let already_dropped = self.has_dropped.atomic.swap(true, SeqCst);
assert!(!already_dropped); assert!(!already_dropped);
} }

View File

@ -19,9 +19,10 @@ fn test_inference() -> Result<()> {
macro_rules! context_type { macro_rules! context_type {
($name:ident) => { ($name:ident) => {
#[derive(Debug)] #[derive(Debug)]
#[repr(C)]
struct $name { struct $name {
message: &'static str,
_drop: DetectDrop, _drop: DetectDrop,
message: &'static str,
} }
impl Display for $name { impl Display for $name {
@ -37,6 +38,7 @@ context_type!(MidLevel);
#[derive(Error, Debug)] #[derive(Error, Debug)]
#[error("{message}")] #[error("{message}")]
#[repr(C)]
struct LowLevel { struct LowLevel {
message: &'static str, message: &'static str,
drop: DetectDrop, drop: DetectDrop,
@ -67,14 +69,14 @@ fn make_chain() -> (Report, Dropped) {
let low = LowLevel { let low = LowLevel {
message: "no such file or directory", message: "no such file or directory",
drop: DetectDrop::new(&dropped.low), drop: DetectDrop::new("LowLevel", &dropped.low),
}; };
// impl Report for Result<T, E> // impl Report for Result<T, E>
let mid = Err::<(), LowLevel>(low) let mid = Err::<(), LowLevel>(low)
.wrap_err(MidLevel { .wrap_err(MidLevel {
message: "failed to load config", message: "failed to load config",
_drop: DetectDrop::new(&dropped.mid), _drop: DetectDrop::new("MidLevel", &dropped.mid),
}) })
.unwrap_err(); .unwrap_err();
@ -82,7 +84,7 @@ fn make_chain() -> (Report, Dropped) {
let high = Err::<(), Report>(mid) let high = Err::<(), Report>(mid)
.wrap_err(HighLevel { .wrap_err(HighLevel {
message: "failed to start server", message: "failed to start server",
_drop: DetectDrop::new(&dropped.high), _drop: DetectDrop::new("HighLevel", &dropped.high),
}) })
.unwrap_err(); .unwrap_err();

View File

@ -11,7 +11,7 @@ fn test_convert() {
maybe_install_handler().unwrap(); maybe_install_handler().unwrap();
let has_dropped = Flag::new(); let has_dropped = Flag::new();
let error: Report = Report::new(DetectDrop::new(&has_dropped)); let error: Report = Report::new(DetectDrop::new("TestConvert", &has_dropped));
let box_dyn = Box::<dyn StdError + Send + Sync>::from(error); let box_dyn = Box::<dyn StdError + Send + Sync>::from(error);
assert_eq!("oh no!", box_dyn.to_string()); assert_eq!("oh no!", box_dyn.to_string());
drop(box_dyn); drop(box_dyn);

View File

@ -109,7 +109,7 @@ fn test_drop() {
maybe_install_handler().unwrap(); maybe_install_handler().unwrap();
let has_dropped = Flag::new(); let has_dropped = Flag::new();
let error: Report = Report::new(DetectDrop::new(&has_dropped)); let error: Report = Report::new(DetectDrop::new("DetectDrop", &has_dropped));
drop(error.downcast::<DetectDrop>().unwrap()); drop(error.downcast::<DetectDrop>().unwrap());
assert!(has_dropped.get()); assert!(has_dropped.get());
} }

View File

@ -31,6 +31,6 @@ fn test_drop() {
maybe_install_handler().unwrap(); maybe_install_handler().unwrap();
let has_dropped = Flag::new(); let has_dropped = Flag::new();
drop(Report::new(DetectDrop::new(&has_dropped))); drop(Report::new(DetectDrop::new("TestDrop", &has_dropped)));
assert!(has_dropped.get()); assert!(has_dropped.get());
} }