Port easy exits

[mrhota]

some of the mess in exit.rs is meant for dynlink. Read commit
messages for exit.c and friends for full details and
justifications.

[anp]

Looks great! I have a couple of questions about how you've replicated the logic from the C side. I suspect that knowing whether it's right is hard b/c the tests are unlikely to check the differences I'm seeing, but it seems like it's worth looking at.

[anp]

Please correct me if I'm wrong, but based on http://git.musl-libc.org/cgit/musl/tree/src/exit/_Exit.c this seems like it should be

loop { syscall!(EXIT, ec); }

since the C for loop will continue attempting to call the EXIT syscall until the process has aborted.

[mrhota]

looks like I lost my original reply in the ether.

the loop on line 7 is unreachable, since the EXIT syscall doesn't return. The loop serves to make the function divergent: without the loop, the return type of the syscall! macro, (), conflicts with the specified return type of !, so rustc refuses to compile it. Adding the loop lets rustc compile without complaint.

The loop is similar in spirit to the loop in the original C. See reasoning here: http://git.musl-libc.org/cgit/musl/commit/src/exit/_Exit.c?id=0c05bd3a9c165cf2f0b9d6fa23a1f96532ddcdb3

[anp]

Awesome, thanks for filling me in!

[anp]

Based on http://git.musl-libc.org/cgit/musl/tree/src/exit/exit.c#n18 it seems like this should perform some magic-y math on those void pointers before calling _fini(). If that's not necessary for some reason, could you include a comment that explains that here for me and any future readers?

[mrhota]

I think I'd rather add a TODO: those symbols are used by dynamic linker for purposes that are still mysterious to me. You can read about Rich Felker's motivations in his very descriptive commit messages here: http://git.musl-libc.org/cgit/musl/log/src/exit/exit.c

[anp]

Works for me. Can you include these links in the comment?

http://git.musl-libc.org/cgit/musl/commit/src/exit/exit.c?id=7586360badcae6e73f04eb1b8189ce630281c4b2
http://git.musl-libc.org/cgit/musl/commit/src/exit/exit.c?id=19caa25d0a8e587bb89b79c3f629085548709dd4

[mrhota]

@dikaiosune I removed the unsafe annotations. They receive simple integers as arguments and don't do anything crazy (or interesting, really).

I added comments explaining some reasoning and passing off to musl commit messages for further discussion.

[anp]

Cool. Looks good.

The functions don't do anything crazy, but we do mark their linkage as weak, and expect the linker to place its own definitions in for the dummies, right? I'm not sure what invariants we would expect a caller to enforce by requiring the calls be placed in an unsafe block, but it seems like it's maybe most appropriate to leave them as unsafe? I'll defer to your judgement.

[mrhota]

@dikaiosune I got inspired to do that because was re-reading https://huonw.github.io/blog/2014/07/what-does-rusts-unsafe-mean/ for some insight into marking fns unsafe vs using unsafe blocks internally.

I wouldn't say we expect the linker to provide strong symbols for things like _fini or __funcs_on_exit, but we are prepared for it by marking the symbols weak. If we expected the symbols, we'd probably just have some extern decls instead of definitions:

extern "C" {
    fn _fini();
    fn __funcs_on_exit();
}

[mrhota]

If it's up to me, I prefer fewer unsafes!

[anp]

Cool. Thinking about it more I think we kind of just have to trust that the linker will do what it says, and that there's nothing our userspace program can do to prevent it behaving poorly.

[anp]

I prefer fewer unsafes!

Heh, I actually have the opposite mindset. I would rather be overcautious about what should be declared unsafe lest we accidentally create a "safe" call chain that breaks things. I think in this case it's immaterial but an interesting philosophical question nonetheless.

One example I see is from this audit report I read this morning:

https://www.x41-dsec.de/reports/Kudelski-X41-Wire-Report-phase1-20170208.pdf

(pg 12, section 3.3) I think that this init function probably should have returned a Result<(), SomeError>, but if they were trying to match a C API they alternatively could have marked it as an unsafe function, signaling to a caller that if they didn't handle its failure correctly that there could be memory unsafety as a result. Ideally they would have done both: used a Result to make use of the must_use compiler lint but also mark the init function as unsafe so that the caller has to be aware of potential memory safety consequences of not handling its failure properly.

Anyways, it's late and I'm waiting for several builds so this is just a ramble.