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

Basic support for dynamically loading syntax extensions #6735

Closed
wants to merge 2 commits into from

Conversation

huonw
Copy link
Member

@huonw huonw commented May 25, 2013

This adds a wrapper around dlopen/dlsym etc to std::unstable, with some effort to get the lifetimes of the symbols from the library to be restricted to the lifetime of the dynamic library (it requires #5922 to work properly). (This lifetime stuff is helpful rather than a safety guarantee, since callbacks into the library can be saved out of view of the compiler (as happens in the syntax extension code).)

(I'm quite uncertain about this part, since I've only occasionally used these functions on Linux, and never on any other platform.)

It also adds a #[syntax_extension="library"] attribute which will load the library (requires the full filename), and call register_syntax_extensions to retrieve a vec of new syntax extensions which are added to the macros that are expanded.

See ext/dynamic.rs for a more detailed description.

I'm only been able to test on x86-64 Linux and it doesn't break anything there, but I'm not really sure how to write tests for this.

(This is definitely not ready for general use (I've hidden it behind a -Z dynamic-syntax-extensions flag), but makes experimenting with/modifiying syntax extensions much easier, since they can be copied out of libsyntax for experimentation and then don't require a full rebuild.)

@Aatch
Copy link
Contributor

Aatch commented May 25, 2013

Wow, nice work!

huonw added 2 commits May 25, 2013 20:47
…ons.

This adds a `#[syntax_extension]` attribute that loads an external Rust
library and allows it to register arbitrary syntax extensions. This requires
the `-Z dynamic-syntax-extension` flag.

The library name to load must be specified in full, and the attribute only
works at the crate level so this will not work cross-platform.
@mstewartgallus
Copy link
Contributor

Hi! I'm just commenting here because I've already been using my own personal binding to the dynamical link libraries facilities on Linux here: http://gitorious.org/rust-dlfcn/rust-dlfcn/trees , and so have some experience with the issues involved in this. Firstly, is there a way to have something like #[cfg (posix)](it just bugs me that all the platforms are explicitly layed out?) Secondly, I don't think the return value of get_symbol is quite right. Can't a pointer of lifetime 'r still point to a pointer of lifetime 's which may be greater than the 'r? Unfortunately, this is not implemented yet but i believe the proper way to accomplish this would be to use some kind of constraint over T, as in T : 'r (I might be mistaken, I'm unsure of this.) Finally, it just bugs me that get_symbol is not labelled as an unsafe function. Can name mangling guarantee that the result will always be of the right type? If so, than the interface could be revised to have two functions, an unsafe get_symbol, and a safe get_mangled_symbol. Thank you very much for this start in making the Rust compiler more extensible.

@huonw
Copy link
Member Author

huonw commented May 26, 2013

@sstewartgallus I like your interface. (I can't quite believe I forgot about Path.)

And you raise a good point, I think T should really be restricted to 'static, possibly with Owned and/or Const bounds (since these are always global symbols), however the point of the lifetimes is to try to make sure the pointer in to the library is valid (i.e. the dynamic library hasn't been closed), not say anything about the value it points to.

For my implementation, get_symbol isn't unsafe: it can never cause a crash (assuming that dlsym doesn't crash on invalid input (?)), since the only way to use the return value is via the TaggedSymbol type. The .get method on this type is dangerous, and is marked unsafe.

(I have no idea about name mangling, this could be a higher level interface built on top of the dynamic library wrappers.)

(I wouldn't be upset if you were to get a dl interface into Rust, over the one I have here.)

@graydon
Copy link
Contributor

graydon commented May 27, 2013

\o/

I think if you want this to stick and avoid bitrotting, it's going to need some tests. But it's a great first step!

@huonw
Copy link
Member Author

huonw commented May 27, 2013

Yeah, it definitely needs tests, but it relies on the file name of the library (including the hash and I think the extension too) at the moment, so any tests would be easy to break and platform dependent. I (or anyone else) was going to add some after making the interface higher level.

@vadimcn
Copy link
Contributor

vadimcn commented Sep 22, 2013

@huonw: So what's the status of this? Why wasn't it merged?

@huonw
Copy link
Member Author

huonw commented Sep 22, 2013

@vadimcn dead, unlikely to be revived until after #8652. The two biggest reasons are:

  • Not cross platform, i.e. one had to write #[syntax_extension="libfoo-14827398247594.so"] on linux, and something else on windows and mac, and the method by which it searched for the libs was very poor (just using the system linker): it should be hooked into rustpkg, or something like that.
  • Very, very unsafe; this didn't/couldn't verify the types of the symbols that it was loading, and so was very open to crashes and/or hostile code doing arbitrary things. This should read the metadata to check that the appropriate symbol is safe.

@vadimcn
Copy link
Contributor

vadimcn commented Sep 22, 2013

I agree with you on the first point, - but searching could be made uniform with not that much more effort.

I disagree on the second one: yes, symbol type validation might be nice for debugging, but it would do nothing whatsoever for security. If you are loading native code, you are already trusting it implicitly.

@huonw
Copy link
Member Author

huonw commented Sep 22, 2013

Re the second point: it's mainly relevant because Rust has the information to do much better (i.e. the metadata is all there), it's just hard to use at the moment.

@huonw huonw deleted the dynamic-syntax branch December 4, 2014 02:02
flip1995 pushed a commit to flip1995/rust that referenced this pull request Feb 25, 2021
lintcheck: accept env var to set crates.toml file

*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: lintcheck: accept  LINTCHECK_TOML env var to set list of crates to be checked.
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.

5 participants