Skip to content

Catch #[no_mangle] #128

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 21 commits into from
Dec 5, 2022
Merged

Conversation

workingjubilee
Copy link
Contributor

@workingjubilee workingjubilee commented Dec 1, 2022

This PR adds code sufficient to catch the usage of #[no_mangle]. Note that deploying PL/Rust with the given Rust toolchain currently indicated by our rust-toolchain.toml will not in fact catch #[link_section], despite the branch name. That requires an upgrade to a more recent version of Rust, which is incompatible with use with postgrestd without patching that, which is itself incompatible with other requirements that have been communicated to me.

Copy link
Contributor

@eeeebbbbrrrr eeeebbbbrrrr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in general this is fine, code wise. It definitely needs some code comments/commentary about what's happening, why it works, the possible problems it prevents, etc, etc.

Move some comments onto types so they show up
with cargo doc --document-private-items
and make sure those docs look good.
Copy link
Contributor

@eeeebbbbrrrr eeeebbbbrrrr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ship it!

@eeeebbbbrrrr
Copy link
Contributor

Thanks for the clarifications offline, @workingjubilee. I know I can be dense sometimes. I'd like to enshrine them here:

So PGX already makes every function into function and function_wrapper
This is usually elided in common thought and usage of the library, but here we peel that curtain back. We don't need function_wrapper yet, and it's the "inherently unsafe" part of PGX code. 
So we write the library as if it only had whatever code the user function contains, which is not actually enough to make it work, but it is still Rust, and it still contains "whatever the function does".
Then we check that.
Then rewrite it using the full text, including the macro annotations that create function_wrapper 
Then build that.
This is valid because PGX's macros don't substantively change the function's code but rather most of what they do is creating the wrapper.

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.

2 participants