-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
wasm-lld: set stack size to 1MB by default #12589
Conversation
Regardless of the build mode (build-exe, build-lib), always set the default stack size to 1MB. Previously, this was only done when using build-exe, making the inconsistancy confusing. The user can still override this behavior by providing the `--stack <size>` flag.
const arg = try std.fmt.allocPrint(arena, "stack-size={d}", .{stack_size}); | ||
try argv.append(arg); | ||
} | ||
} else if (self.base.options.entry == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure this is correct here? The previous version of the code did not check for entry
at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but a little bit above we set the entry if the user provided it. It's invalid to pass both an entry point and --no-entry
. I figured I'd get that fix in while at it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's invalid, should we perhaps flag it up as an error to the user?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so. --entry
is given by the user, whereas --no-entry
is a flag we set by default depending on the output mode. So the user never creates this invalid state, it's the Zig toolchain that should ensure it doesn't occur. Unless we disallow have an entry point in non build-exe modes, but given how Wasm runs in runtimes I think it's perfectly valid to have an environment where you do have a custom entry, but not the _start logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right, so we don't expose --no-entry
to the user? I'm curious though, what is the behavior of LLD upon receiving both flags in a single linker line, do you know perhaps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LLD will ignore the --entry
flag in such case, which may confuse the user, as they may be unaware that --no-entry
was passed to lld also and therefore have their own flag ignored.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for double checking! I am fine with the current approach. In the future though, I wonder if we should perhaps make it a hard error. Food for thought!
Regardless of the build mode (build-exe, build-lib), always set the default stack size to 1MB. Previously, this was only done when using build-exe, making the inconsistancy confusing. The user can still override this behavior by providing the
--stack <size>
flag.Closes #3735.