Libsystem, or, the great libstd refactor

[skade]

I agree that it’s not so great today, but with the stabilization of libcore I would expect the experience here to get much better (as an ecosystem around this would actually exist!)

I can, by the way, recommend to walk the walk. I’m a huge proponent of not developing on nightly unless absolute need and propagate that by trying to port everything I see to stable and do pull requests. I can see similar things happening for libs that just require core. (although that is obviously harder, as things like String and Vec are in std - rightfully, but often used and people might be hesistant to code without)

[arcnmx]

Well this is roughly my final draft of the refactor. Just a ton of moving things around… Is there a nice way to build/test/run this against as many platforms as possible?

libstd::sys refactor

Also, see this commit as an example/outline of what needs to be implemented to port libstd to an entirely new platform…

late-bind standalone target_family

[briansmith]

I like some of the ideas here, but there were some things that would help me understand what’s going on:

Is your refactoring 100% backward compatible with the current (at that time) libstd? If not, what are the backward-incompatible differences?

Your patch changes 166 files, +3342/-3030. This is too much for somebody to read at once. Indeed, GitHub won’t even display the whole patch in its web UI. Is there any way that the patch could be broken up into parts?

For example, could we deep-dive into, say, just the part that refactors the implementation of std::thread, to see what kind of new capability we would gain from the refactoring? In particular, you mentioned above the idea of a libc-dependency-free implementation of libstd. What would the libc-dependency-free direct-syscall implementation of std::thread on Linux look like?

If std::thread is too big of a chunk for that kind of exercise, then what’s a smaller thing that we could do it on that would show the benefit of your approach?

[arcnmx]

Is your refactoring 100% backward compatible with the current (at that time) libstd? If not, what are the backward-incompatible differences?

It is fully compatible, yes. It’s a pure refactor: I simply move things around so that there’s a platform-independent API layer (libsystem or libstd::sys) that libstd uses to implement various interfaces.

Your patch changes 166 files, +3342/-3030. This is too much for somebody to read at once. Indeed, GitHub won’t even display the whole patch in its web UI. Is there any way that the patch could be broken up into parts?

I’m thinking about trying that if the design is accepted. It’s tough though, there tends to be a lot of interdependency between these parts for some reason, so moving them a bit at a time can be awkward.

For example, could we deep-dive into, say, just the part that refactors the implementation of std::thread, to see what kind of new capability we would gain from the refactoring?

Well I mean, you can git diff src/libstsd/sys/*/thread* src/libstd/thread/* or whatever. There isn’t any new functionality being gained here though. It’s just things being moved. The idea being that you can look at sys/thread and see exactly what functions you need to implement in order to port libstd to a given platform.

particular, you mentioned above the idea of a libc-dependency-free implementation of libstd. What would the libc-dependency-free direct-syscall implementation of std::thread on Linux look like?

Basically look at https://github.com/arcnmx/rust/commit/30d290f6cd456bf64a5be0930463dc7525ab4a90#diff-9adecdd6e4eaa5e6e34f584c3f0c9c5dR387

You’d need to fill that in and implement thread::new, Thread::{join, set_name, yield_, sleep}

[briansmith]

Is your refactoring 100% backward compatible with the current (at that time) libstd? If not, what are the backward-incompatible differences?

It is fully compatible, yes.

Awesome.

briansmith: Your patch changes 166 files, +3342/-3030. This is too much for somebody to read at once. Indeed, GitHub won’t even display the whole patch in its web UI. Is there any way that the patch could be broken up into parts?

I’m thinking about trying that if the design is accepted.

I’m not a Rust reviewer, but IMO that’s probably got the order of operations backwards. The main benefit, as I see it, of the refactoring is that it makes it easier to create alternative implementations of libstd not based on libc, either for a port to a non-unixy platform, or for some other reason. I think that is an important advantage, and it doesn’t seem to have negative consequences, but it’s not a high priority for others. Regardless, what you proposed seems too big for people to evaluate as one item. Also, I think “don’t ask for permission” applies here. So, I recommend just breaking out a few changes into small-enough-to-review pieces and submitting PRs.

Also, people might have read your initial comments as “the structure of libstd is bad and I’m fixing it” whereas it might be better to convey the message “Here’s some patches to make porting libstd easier.”

In particular, you mentioned above the idea of a libc-dependency-free implementation of libstd. What would the libc-dependency-free direct-syscall implementation of std::thread on Linux look like?

Basically look at https://github.com/arcnmx/rust/commit/30d290f6cd456bf64a5be0930463dc7525ab4a90#diff-9adecdd6e4eaa5e6e34f584c3f0c9c5dR387

You’d need to fill that in and implement thread::new, Thread::{join, set_name, yield_, sleep}

As far as I understand your idea, shouldn’t most of what was added to liballoc_system/lib.rs be in a separate file, similar to the added bind.rs? Then the added section of liballoc_system/lib.rs would be something like:

#[cfg(target_family = "bind")]
pub use super::bind::{
    allocate,
    deallocate, 
    reallocate,
    reallocate_inplace,
    usable_size
};

And, in fact, that could be encapsulated in a macro that takes the module from which to re-export items as a parameter, I think.

Your patch mixes two ideas:

1. It should be possible to implement libstd without depending on the libc crate.
2. The libc-less replacement will be primarily implemented in non-Rust code.

I think it would be useful to split these two ideas, because the late-binding-to-another-library approach isn’t the only interesting one. In particular, the invoke-syscall-directly-from-Rust case doesn’t require all the late binding machinery.

Also, your change to OsRng hints at another refactoring: If there was a sys::rand::fill_bytes standalone function, then we could share one implementation of OsRng, and each libstd port would only have to implement sys::rand::fill_bytes. In fact, I think that this refactoring should be done separately, probably before the other refactorings.

Anyway, I think that your overall idea is good and I encourage you to move forward with it. I’m happy to look over your pull requests. (Keep in mind I’m not a Rust stdlib reviewer, so I can’t approve changes.)

[arcnmx]

As far as I understand your idea, shouldn’t most of what was added to liballoc_system/lib.rs be in a separate file, similar to the added bind.rs? Then the added section of liballoc_system/lib.rs would be something like

And, in fact, that could be encapsulated in a macro that takes the module from which to re-export items as a parameter, I think.

Your patch mixes two ideas:

It should be possible to implement libstd without depending on the libc crate. The libc-less replacement will be primarily implemented in non-Rust code.

Note that the late-binding commit is not something I plan to ever be included in tree. It was for my own curiosity to see what the absolute minimum is required for porting. I link to it as an example of what Rust prototypes need to be implemented in order to port to a new platform, not as an exemplar that should actually be taken seriously for anything, and certainly not a style guide/suggestion as far as the module and file layout is concerned

Also, I think “don’t ask for permission” applies here. So, I recommend just breaking out a few changes into small-enough-to-review pieces and submitting PRs.

I guess I can try that… It’s just a massive amount of work that I can’t reasonably start on again without some indication that it’s the right direction to go ahead with.

[jethrogb]

I think this is a great idea to support increased exposure in the future, e.g. for embedded devices.

[briansmith]

I guess I can try that… It’s just a massive amount of work that I can’t reasonably start on again without some indication that it’s the right direction to go ahead with.

Alex already said “Ok this sounds like a good idea to me! The standard library has been through a lot of iterations and we’ve never really had a chance once things have settled to take a look at the story of all the littered #[cfg] directives, and it’d be nice to consolidate!” I don’t think you’re going to get a much more positive go-ahead than that. Nobody’s going to promise that patches gets accepted before they see the actual patches.

I’m looking forward to seeing the PRs if you make them.

[arcnmx]

Alex already said “Ok this sounds like a good idea to me! The standard library has been through a lot of iterations and we’ve never really had a chance once things have settled to take a look at the story of all the littered #[cfg] directives, and it’d be nice to consolidate!” I don’t think you’re going to get a much more positive go-ahead than that. Nobody’s going to promise that patches gets accepted before they see the actual patches.

Well his more recent comment was pretty dismissive so it seems unlikely that’s a good approach to take… I’m not so much looking for approval as much as indication from someone that I’m not doing something very wrong here, but no one seems to care enough to look. As for an RFC… “cut-paste anything in libstd within an adhoc conditional #[config(target_os = ...)] to the sys module” doesn’t seem like a very productive one to me so meh.

[lambda]

I don’t think his comment was dismissive of the idea in general. The current form of the patch isn’t really possible to review; a single giant patch with no discussion whatsoever.

This is a big enough change, that affects enough of the internals of the std library, how easy it is to port, and would cause any other WIP branches to require manual merging so would need some timing and coordination to land, that an RFC does seem reasonable. Basically, something discussing why this move is a good idea, what porting use cases it would help out, if there are any alternatives, etc.

In addition to that, once the RFC process is done, I think this would be better done split up into a series of patches; one huge patch makes it hard to break down and verify that each move is sound and doesn’t change anything important; particular concerns might be privacy, error reporting, and the like.

I would not take his response as dismissive; just saying that once seeing the change, it’s clear that it will require a little more discussion before it’s reviewable and mergeable.

[arcnmx]

I don’t think his comment was dismissive of the idea in general. The current form of the patch isn’t really possible to review; a single giant patch with no discussion whatsoever. I would not take his response as dismissive; just saying that once seeing the change, it’s clear that it will require a little more discussion before it’s reviewable and mergeable.

Sorry, by that I just meant dismissive of the actual design and layout being proposed here. I simply haven’t received any feedback on that whatsoever on any front, which is kind of what I’m looking for… The lack of any discussion or feedback on the changeset itself simply doesn’t instill much hope that an RFC would generate much more discussion. I get that it’s large but that doesn’t preclude being able to see what kind of changes have been made and whether they’re a good idea or not.

I think this would be better done split up into a series of patches; one huge patch makes it hard to break down and verify that each move is sound and doesn’t change anything important; particular concerns might be privacy, error reporting, and the like.

Agreed.

This is a big enough change, that affects enough of the internals of the std library, how easy it is to port, and would cause any other WIP branches to require manual merging so would need some timing and coordination to land, that an RFC does seem reasonable. Basically, something discussing why this move is a good idea, what porting use cases it would help out, if there are any alternatives, etc.

I guess my view is that it’s currently impossibly difficult to port, and libstd changes are far and few enough between right now that it’d be a good time to land. Though mostly I’m just impatient because after the few months it takes an RFC to get through I won’t have the time or energy to actually make the changes anymore - I’m currently on a rare break with some actual time to get things done.

[briansmith]

Well his more recent comment was pretty dismissive so it seems unlikely that’s a good approach to take… I’m not so much looking for approval as much as indication from someone that I’m not doing something very wrong here, but no one seems to care enough to look.

Nobody has time to read a 166 file +3342/-3030 patch. If you want somebody to look at a patch, you need to make one that is very much smaller. If you won’t break up the patch into smaller parts until the Rust community agrees that they want to split all the platform-dependent bits of libstd into separate modules, then the way to get that kind of agreement is through an RFC.

[quote] As for an RFC… “cut-paste anything in libstd within an adhoc conditional #[config(target_os = …)] to the sys module” doesn’t seem like a very productive one to me so meh. [/quote]

Probably more people would be supportive of that wording than the original way it was proposed.

But, your proposal is more than that, right? I’ve made an attempt to help you make the RFC here: https://gist.github.com/briansmith/d4540b86c113eba8ee93. I intentionally left out the technical details in the “Detailed Design” section as you know them better than us. Please fill them in and make other changes for any other parts of your proposal that I’ve misunderstood.

[arcnmx]

Nobody has time to read a 166 file +3342/-3030 patch. If you want somebody to look at a patch, you need to make one that is very much smaller.

Eh one can glance it over and look at the file structure and an example module or two ;_;

I’ve made an attempt to help you make the RFC here

You are good at this communication thing. The “formal verification” part is a bit hard to define though. We’re not really defining the standard library so much as the layer underneath it that’s used to interact with the underlying OS. And even that definition isn’t really that formalized.

[briansmith]

Eh one can glance it over and look at the file structure and an example module or two ;_;

That’s generally not how people work, for better or worse.

The “formal verification” part is a bit hard to define though. We’re not really defining the standard library so much as the layer underneath it that’s used to interact with the underlying OS. And even that definition isn’t really that formalized.

I agree I wasn’t so clear, but I’m not sure how to improve it. The point I was trying to make is that having a clear interface to the platform-specific bits should help the development of a formal definition of the Rust standard library by facilitating the removal of assumptions that depend on the semantics of libc…eventually.

[oli-obk]

How about doing this incrementally?

Add a libsystem, but only port a single thing. Then port the next thing in another PR. At some point libstd should be free of system dependent code, and libsystem should only offer the minimal public API necessary for libstd to work.

[Zoxc]

As someone who wants to add support for another OS to libstd I like the idea. Keeping the OS specific code separated also makes it easier to maintain a fork which supports an additional OS.

[lilred]

+1, I’d love for this to happen.

It’d be really neat if libstd could be implemented inside Rust code the same way libcore is - by just providing x,y,z language items.

[briansmith]

See this Hacker News thread: https://news.ycombinator.com/item?id=11111059

Rust programs vulnerable to buffer overflow because of glibc.

@arcnmx, Are you going to be able to fill out the technical details of the RFC? It doesn’t have to be perfect. Just an outline of the various refactorings you did, before you completely forget what they are.

[arcnmx]

Yeah, sorry, I’ve been busy… I will try to get to that asap so at least we have some talking points and can get to it all sooner rather than later.

[steveklabnik]

Rust programs vulnerable to buffer overflow because of glibc.

Unless you’re using a MUSL-related rustc. Which is admittedly a minority.