Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong cast() call causes access violation at vtable access #1183

Closed
saschanaz opened this issue Sep 26, 2021 · 4 comments · Fixed by #1753
Closed

Wrong cast() call causes access violation at vtable access #1183

saschanaz opened this issue Sep 26, 2021 · 4 comments · Fixed by #1753

Comments

@saschanaz
Copy link

saschanaz commented Sep 26, 2021

From this function where JXLWICBitmapFrameDecode implements IWICBitmapFrameDecode:

let frame_decode = JXLWICBitmapFrameDecode::new(frame.data.clone(), basic_info);
let casted: IWICBitmapFrameDecode = frame_decode.cast()?;

let mut width = 0u32;
let mut height = 0u32;
casted.GetSize(&mut width, &mut height)?;

The call to GetSize() throws without calling the actual implemented function:

Exception has occurred: W32/0xC0000005
Unhandled exception at 0x0000000000000000 in rundll32.exe: 0xC0000005: Access violation executing location 0x0000000000000000.

image

Not sure what's wrong here, maybe the metadata, ᅟor the use of cast(), or 🤷

The full code is here: https://github.com/saschanaz/jxl-winthumb/blob/caca0636f3802b8c7c26a6d37c6fb6976f868dbe/src/wic.rs#L119

@saschanaz saschanaz changed the title implement(IWICBitmapFrameDecode) causes access violation at vtable access Wrong cast() call causes access violation at vtable access Sep 26, 2021
@saschanaz
Copy link
Author

Okay, it should be into() instead of cast() when casting from Rust type into Windows API type. But shouldn't cast() return Err in that case?

@kennykerr
Copy link
Collaborator

There should be some kind of assert that you've already boxed the object (via into) but right now cast assumes that's already happened.

@rylev
Copy link
Contributor

rylev commented Mar 18, 2022

The way that this is handled in com-rs is by doing the following:

  • not exposing a new function which simply stack allocates (and relying that the user does the right thing and heap allocates)
  • providing a type called ClassAllocation which is a wrapper around ManuallyDrop<Pin<Box<T>>>
    • this type ensures the invariant that the implementation is heap allocated, will not move, and is dropped by some external force (i.e., when the ref count is 0)
  • The user is only ever exposed an allocate function which does the dance of putting the type on the heap and pinning it.

This works quite well and ensures that the user can never stack allocate the implementation type. Would something like this work for windows-rs as well?

@kennykerr
Copy link
Collaborator

I don't think that would work without some modification because the windows crate's implement macro is a proc_macro_attribute rather than the proc_macro in the com crate where you basically rewrite the entire implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants