Skip to content

Remove unnecessary _stack_start from Rust code#765

Open
matthiasgoergens wants to merge 4 commits into
masterfrom
matthias/remove-stack-start
Open

Remove unnecessary _stack_start from Rust code#765
matthiasgoergens wants to merge 4 commits into
masterfrom
matthias/remove-stack-start

Conversation

@matthiasgoergens

Copy link
Copy Markdown
Contributor

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

lispc commented Dec 18, 2024

Copy link
Copy Markdown
Collaborator

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
Copy Markdown
Contributor 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

lispc commented Dec 18, 2024

Copy link
Copy Markdown
Collaborator

i prefer putting them all in extern C

@matthiasgoergens

matthiasgoergens commented Dec 18, 2024

Copy link
Copy Markdown
Contributor Author

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