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

Decide (and document) best practices for FFI declarations #554

Open
nicholasbishop opened this issue Nov 10, 2022 · 1 comment
Open

Decide (and document) best practices for FFI declarations #554

nicholasbishop opened this issue Nov 10, 2022 · 1 comment

Comments

@nicholasbishop
Copy link
Contributor

nicholasbishop commented Nov 10, 2022

We have a lot of FFI declarations in the code, and the style of these declarations is not always consistent:

  1. Most are declared unsafe, but not all.
  2. Some places *const/*mut, others use &/&mut. Using references can be correct since they are layout-compatible with pointers, but it's not always clear from the spec what the restrictions are on how a pointer gets used, so using references may lead to UB.
  3. Some places use wrappers like Option / NonNull. These can be perfectly correct in cases where the layout is well defined and FFI-compatible, but the inconsistency can still make it harder to verify correctness.

Another layer of confusion comes from the IN/OUT/OPTIONAL/CONST modifiers in the spec. Sometimes these are used in weird ways (e.g. OPTIONAL is defined as allowing NULL to be passed in, but sometimes it's applied to non-pointer parameters). Sometimes they seem more like guides than rules (IN and OUT in particular seem more like semantic hints). So in general the safest way to define an FFI pointer is probably to use *mut... but if that pointer is initialized from a & reference in a safe wrapper we provide then that might just be moving the location of possible UB down a level.

See also some good relevant discussion here: r-efi/r-efi#46

@timrobertsdev
Copy link
Contributor

timrobertsdev commented Nov 12, 2022

On a fun level, I've found that OPTIONAL doesn't always apply or doesn't apply to the first pointer in a pointer-to-a-pointer parameter as well.

There are more than a few inconsistencies between the spec and EDK2 (and other implementations, I would imagine). I tend to dive into the EDK2 source when something is ambiguous in the spec, but would this be correct? Specifically when it comes to function parameters and their modifiers.

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

No branches or pull requests

2 participants