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 unnecessary _stack_start from Rust code #765

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

matthiasgoergens
Copy link
Collaborator

Just like __global_pointer or _start, we don't need to mention _stack_start in Rust to make use of it in the assembly.

@lispc
Copy link
Collaborator

lispc commented Dec 18, 2024

i am not sure whether it is a good idea. Will the extern C code block make IDE happier?

it can make readers easy to understand? "Oh these will be a extern var named _stack_start". Many programmers knows extern C, while few knows linker stuff.

@matthiasgoergens
Copy link
Collaborator Author

Well, as mentioned in the PR description, there's a bunch of other symbols we only take from the linker, too. So _stack_start ain't special.

We should either define all of them here redundantly, or none.

(In general, this file should only be edited by people who have looked at least a bit into the linker scripts.)

@lispc
Copy link
Collaborator

lispc commented Dec 18, 2024

i prefer putting them all in extern C

@matthiasgoergens
Copy link
Collaborator Author

matthiasgoergens commented Dec 18, 2024

i prefer putting them all in extern C

OK, I just did some experiments. For example, if you mistype the name of the variable in the extern "C" declaration like this:

extern "C" {
    // The address of this variable is the start of the stack (growing downwards).
    static _stack_start_this_is_a_typo: u8;
}

then you still don't get an error message. So it seems like having this declaration is no better than just adding a comment. And perhaps worse, because everyone knows that human readable comments are only there for humans and that the compiler doesn't check them; but the extern "C" variable declaration looks like it might be checked, but ain't really.

So I suggest I'll add a comment to the assembly code instead. See the new version, please.

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