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

remove segmented stack preludes or at least make them optional #11871

Closed
thestinger opened this issue Jan 28, 2014 · 11 comments
Closed

remove segmented stack preludes or at least make them optional #11871

thestinger opened this issue Jan 28, 2014 · 11 comments

Comments

@thestinger
Copy link
Contributor

This is unable to truly provide safety since nearly all Rust code makes calls into native libraries. These libraries do not have the segmented stack preludes. The libraries may have been built with GCC using -fcheck-stack.

At the moment, Rust is unable to build a binary without the dependency on segmented stacks. Rust installs neither llc or clang, so it provides no way to compile a freestanding binary at all. Using llc is not ideal since it uses a weird optimization stack, and clang isn't built at all (clang 3.3 or 3.4 are not defined to work on the bytecode from trunk).
#10781 covers provide full stack safety via a guard page with the assumption that libraries are compiled with -fcheck-stack or do not have large uninitialized stack frames. However, there really needs to be a way to disable this now as it prevents using rustc to compile any freestanding/runtimeless code.

@alexcrichton
Copy link
Member

In the past we have generally disagreed with the option of turning off stack safety prologues. In terms of user code, you have to explicitly opt-in to this by setting the stack bounds manually to (0, infinity). In terms of kernel code, you have to go through the difficulty of setting up segment selectors (at least on x86) which is a pain, sadly.

I believe that the best solution to this problem is to find a solution to stack safety that requires no runtime at all. The guard page idea is essentially that, but we can't "just use chkstk" because it's not designed for unwinding. Any future solution we have to our stack-safety situation should be designed with unwinding in mind (on stack overflow).

@thestinger
Copy link
Contributor Author

The segmented stack prelude doesn't work for unwinding either. It shouldn't block switching to a better solution. It adds code size overhead to every function and requires runtime support. The inability of Rust's compiler to build freestanding code should be taken more seriously.

@brson
Copy link
Contributor

brson commented Jan 30, 2014

I agree that the freestanding use case is important enough that we need to provide ways to turn off all implicit runtime dependencies, including the call to __morestack.

@brson
Copy link
Contributor

brson commented Jan 30, 2014

I don't understand a lot of points in the op. Don't we use guard pages to prevent native code from overflowing? In what non-freestanding situation does rust not prevent stack overflow? Is it just very large frames that don't touch the stack every page?

Why do Rust's segmented stacks require a dependency on llc and clang?

@thestinger
Copy link
Contributor Author

@brson: Guard pages do prevent most cases of stack overflow, but a full solution requires something like GCC's stack checking so large uninitialized frames can't go beyond the guard page. LLVM has this logic to output chkstk on Windows, but doesn't offer it elsewhere yet.

At the moment, it's possible to avoid Rust's segmented stacks preludes by going through llc or clang which is why I'm mentioning that.

@bharrisau
Copy link
Contributor

I'm pretty unfamiliar with LLVM, but can it be as 'simple' as creating a MachineFunctionPass in rustllvm and adding it to the codegen pass manager? We addRequired<PEI> to make it run after the prologue/epilogue and then copy & simplify the adjustForSegmentedStacks function?

The function looks pretty simple, the only issue is that it has to make machine specific code. But if the logic is just checking the stack size against a constant, the only machine specific call is the branch to __chkstk. Everything else is machine independent.

Won't help out the MMU-less users, but I guess they have the option of implementing a better prologue for their architecture.

@alexcrichton
Copy link
Member

I had never thought about using a custom pass of ours instead of modifying LLVM itself, I didn't even know it was capable of doing that!

If we could move all our morestack code to our own pass, then there should be nothing blocking us from using the system LLVM install (hurray!).

My current idea of the "ideal future" is one that doesn't involve the chkstk function. Rather, all functions will look like this:

function_foobar:
    load $(-1 * page_size)(%rsp), %rax
    load $(-2 * page_size)(%rsp), %rax
    ...
    load $(-stack_size)(%rsp), %rax
    < rest of function >

That way we have 0 runtime dependencies and you should trigger a fault when entering every function. Additionally page_size could actually be "guard zone size" which means we could emit some load instructions for larger stacks.

The benefit of this is that for small functions the overhead is still very small (just one load instruction), and for larger functions it doesn't matter too much anyway. I wanted to avoid needing a chkstk or related symbol.

Unwinding-wise, the fault handler would detect a fault, detect it was in a guard zone, and then arrange the thread to execute at the the beginning of a piece of code that triggers unwinding. This is the part that's really hazy and I'm not quite sure that we can do just yet.

@thestinger
Copy link
Contributor Author

@alexcrichton: Implementing an equivalent to the GCC -fcheck-stack function has zero size/performance overhead for 99% of functions and doesn't require adding a symbol. It will also be something upstream will be interested in merging and maintaining.

@bharrisau
Copy link
Contributor

Ok, just had a play around with inserting a custom pass. I got it registered and running (build fails due to PEI issue). Code is clearly ugly, but I just wanted to see if it worked. bharrisau/rust@c4b3c4f

Biggest issue is that you can only append a Pass on the PassManager. The pass dependencies aren't yet working exactly as I want them to so things aren't running in the right order.

I'll keep trying.

@bharrisau
Copy link
Contributor

Don't know if it is a particularly elegant solution, but the simplest solution it to send a dummy PassManager into Target->addPassesToEmitFile(). Collect all the passes in the dummy then add them to the real PassManager ourselves (inserting the custom pass in the correct location).

@thestinger
Copy link
Contributor Author

Closing in favour of #16012 since -C no-stack-check now exists.

flip1995 pushed a commit to flip1995/rust that referenced this issue Dec 28, 2023
…plit, r=llogiq

Extend `UNNECESSARY_TO_OWNED` to handle `split`

Fixes rust-lang/rust-clippy#9965.

When you have `to_string().split('a')` or equivalent, it'll suggest to remove the `to_owned`/`to_string` part.

r? `@flip1995`

changelog: Extend `UNNECESSARY_TO_OWNED` to handle `split`
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

No branches or pull requests

4 participants