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

Most sys enums are unsafe #277

Closed
adumbidiot opened this issue Nov 3, 2020 · 3 comments · Fixed by #278 or #295
Closed

Most sys enums are unsafe #277

adumbidiot opened this issue Nov 3, 2020 · 3 comments · Fixed by #278 or #295

Comments

@adumbidiot
Copy link
Collaborator

According to the bindgen docs for rustified enums:

Use this with caution, creating this in unsafe code (including FFI) with an invalid value will invoke undefined behaviour. You may want to use the newtype enum style instead.

This is problematic, as NAPI often adds to these enums across editions. This causes UB if, for example, an napi 3 module is loaded in an napi 6 context and calls typeof on a bigint, creating a rust enum with an invalid state.

This can be fixed by not using rust enums for these types and using constant values instead. Many From/Into impls will have to be replaced with TryFrom/TryInto, but these impls were unsafe anyways.

I ran into this issue while working on #271. If you want, I can push fixes there or I can just make a new PR fixing this.

@Brooooooklyn
Copy link
Sponsor Member

You may want to use the newtype enum style instead

So how about change bindgen to generate newtype enum?

@adumbidiot
Copy link
Collaborator Author

Ok, I'll open a new PR for that

@adumbidiot
Copy link
Collaborator Author

@Brooooooklyn This should be reopened. We use low level sys enums directly in napi calls again which can lead to them having an invalid state between versions of napi.

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