Skip to content

Avoid creating references to out params #256

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

Merged
merged 7 commits into from
Apr 19, 2022
Merged

Avoid creating references to out params #256

merged 7 commits into from
Apr 19, 2022

Conversation

jsha
Copy link
Collaborator

@jsha jsha commented Apr 13, 2022

Creating a reference to uninitialized memory is undefined behavior. Callers of this library are likely to pass pointers to uninitialized memory as out params. We should support that by not turning out params into references.

Instead, keep them as raw pointers and use an unsafe block where we actually write the param.

Fixes #245

Creating a reference to uninitialized memory is undefined behavior.
Callers of this library are likely to pass pointers to uninitialized
memory as out params. We should support that by not turning out params
into references.

Instead, keep them as raw pointers. This adds a null_check! macro to
replace the try_mut_from_ptr! we used to use, and puts an unsafe block
at the location where we actually write to the out param.
@jsha jsha requested review from tgeoghegan and djc April 16, 2022 05:29
@jsha jsha marked this pull request as ready for review April 16, 2022 05:29
@jsha
Copy link
Collaborator Author

jsha commented Apr 16, 2022

@tgeoghegan @djc if either of you is available to review this, I'd appreciate it. Thanks!

@jsha
Copy link
Collaborator Author

jsha commented Apr 18, 2022

I updated the issue with some related links: #245 (comment). In particular, Miri can't catch this problem. From the Miri README:

In particular, Miri does currently not check that integers/floats are initialized or that references point to valid data.

@djc
Copy link
Member

djc commented Apr 19, 2022

This looks reasonable to me (though I haven't reviewed the unsafe invariants in depth).

@jsha jsha merged commit a9fe5cf into main Apr 19, 2022
@jsha jsha deleted the safer-outparam branch April 19, 2022 16:42
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 this pull request may close these issues.

Possible undefined behavior in out parameter handling
2 participants