eyre-rs / eyre

A trait object based error handling type for easy idiomatic error handling and reporting in Rust applications
Apache License 2.0
1.41k stars 68 forks source link

Fix stacked borrows when dropping #81

Closed TimDiekmann closed 2 years ago

TimDiekmann commented 2 years ago

🌟 What is the purpose of this PR?

While adding a compatibility layer in error-stack for eyre, miri has detected a bug we also encountered when implementing error-stack.

error: Undefined Behavior: trying to reborrow <226876> for Unique permission at alloc95485[0x18], but that tag does not exist in the borrow stack for this location
   --> .cargo/registry/src/github.com-1ecc6299db9ec823/eyre-0.6.8/src/error.rs:521:20
    |
521 |     let unerased = mem::transmute::<Box<ErrorImpl<()>>, Box<ErrorImpl<E>>>(e);
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |                    |
    |                    trying to reborrow <226876> for Unique permission at alloc95485[0x18], but that tag does not exist in the borrow stack for this location
    |                    this error occurs as part of a reborrow at alloc95485[0x0..0x30]
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information

🔗 Related links

🔍 What does this change?

yaahc commented 2 years ago

Wow, is this all that's needed to satisfy miri? I was under the impression from https://github.com/yaahc/eyre/issues/59 that it would be more involved.

yaahc commented 2 years ago

I'll make sure to cut a new release later today, thank you for the fix!

TimDiekmann commented 2 years ago

I cannot guarantee, that every test is passing using miri because some tests use features, which are not available with miri and I didn't investigate further. However, simple tests (create a Report, wrap it a few times, drop it) are now passing.

If you need help setting up miri for CI feel free to ping me! 🙂

yaahc commented 2 years ago

If you need help setting up miri for CI feel free to ping me! 🙂

I mean, if you wanted to do that please do so! I haven't been great about making time for maint on eyre and appreciate all the help I can get.