-
Couldn't load subscription status.
- Fork 75
Document a setjmp/longjmp convention #225
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
Changes from 24 commits
8d80cb0
7a3233a
ddd1e17
933eba3
afc947c
07dfc54
871cab8
544f148
cf83c3e
a9689fe
13891ab
c2feeb4
119de61
a0cdee8
862948e
1f28a42
ed850b7
4a4d448
54ca9fb
6da62fc
c91b637
680e18f
82a90db
baa023e
8c2c54f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,235 @@ | ||||||
| # C setjmp/longjmp in WebAssembly | ||||||
|
|
||||||
| ## Overview | ||||||
|
|
||||||
| This document describes a convention to implement C setjmp/longjmp via | ||||||
| [WebAssembly exception-handling proposal]. | ||||||
|
|
||||||
| This document also briefly mentions another convention based on JavaScript | ||||||
| exceptions. | ||||||
|
|
||||||
| [WebAssembly exception-handling proposal]: https://github.com/WebAssembly/exception-handling | ||||||
|
|
||||||
| ## Runtime ABI | ||||||
|
|
||||||
| ### Linear memory structures | ||||||
|
|
||||||
| This convention uses a few structures on the WebAssembly linear memory. | ||||||
|
|
||||||
| #### Reserved area in jmp_buf | ||||||
|
|
||||||
| The first 6 words of C jmp_buf is reserved for the use by the runtime. | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add a note here making it explicit that the contents of these 6 words are not public? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok. |
||||||
| ("words" here are C pointer types specified in the [C ABI].) | ||||||
| It should have large enough alignment to store C pointers. | ||||||
| The actual contents of this area is private to the runtime implementation. | ||||||
|
||||||
| The actual contents of this area is private to the runtime implementation. | |
| The actual contents of this area are private to the runtime implementation. |
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.
fixed. thank you.
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.
Is this __WasmLongjmpArgs struct exposed as part of the ABI between libc and the compiler? If it isn't, would it make sense to remove this section, or at least add a caveat that this section is non-normative?
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.
it's a part of the ABI. the compiler-generated code needs to know how to read members of this structure.
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, I misunderstood that part then.
In that case, I wonder if it would make sense to further simplify the ABI, from this:
pop __WasmLongjmpArgs pointer from the operand stack
$env = __WasmLongjmpArgs.env
$val = __WasmLongjmpArgs.val
$label = $__wasm_setjmp_test($env, $func_invocation_id)
if ($label == 0) {
;; not for us. rethrow.
call $__wasm_longjmp($env, $val)
}
to this:
$args = pop __WasmLongJmpArgs pointer from the operand stack
$label = $__wasm_setjmp_test($args, $func_invocation_id)
doing the loading of $val and $env, as well as the if ($label == 0) and the __wasm_longjmp call while we're at it, all within the __wasm_setjmp_test call.
That way, we'd have less code inline. Would that make sense?
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.
well, at least $val needs to be visible to the catching logic as it's the return value of setjmp()
while we can make __wasm_setjmp_test somehow return $val as well, it seems like the opposite (at least incompatible) direction from multivalue TODO, where we can make $env and $val direct parameters of the exception.
personally i'm ok with either ways.
@aheejin do you have any preference? (asking because i guess the multivalue TODO comment is yours.)
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.
As @yamt said, val needs to be accessible from the catching code, so I'm not sure what's the point of passing arg to __wasm_setjmp_test and making it return val. Also
if ($label == 0) {
;; not for us. rethrow.
call $__wasm_longjmp($env, $val)
}
this part cannot go into __wasm_setjmp_test, no? It doesn't look like we can save a ton here, regardsless of whether we do multivalue or not.
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.
this part cannot go into __wasm_setjmp_test, no?
i guess it can. why not?
__wasm_setjmp_test can just call __wasm_longjmp internally. or throw the exception directly.
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.
Hmm, yeah, you're right. Would you like to submit a PR to the LLVM repo doing this?
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.
Hmm, yeah, you're right. Would you like to submit a PR to the LLVM repo doing this?
do you only mean "make __wasm_setjmp_test rethrow"?
or what sunfishcode suggested?
or both?
i added them to the "Future directions" section for now.
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.
probably it's also ok to rethrow with delegate/throw_ref?
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.
Hmm, yes, I think we can rethrow (or throw_ref) it. Not sure whether we need delegate though. But given that it is discouraged to use the current rethrow instruction in LLVM because it is replaced by the new throw_ref, and the new throw_ref has not been implemented in LLVM yet, we can probably try this later.
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.
yea, i meant rethrow, not delegate.
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.
Again, maybe we could mention emscripten_longjmp too
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.
| [WebAssemblyLowerEmscriptenEHSjLj.cpp]: https://github.com/llvm/llvm-project/blob/70deb7bfe90af91c68454b70683fbe98feaea87d/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp | |
| [WebAssemblyLowerEmscriptenEHSjLj.cpp]: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp |
Not sure if this needs to be from a specific commit version, given that we are not specifying specific lines
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 prefer to always use permalink as the files can be renamed/removed in future.
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 think in that case we should fix the link, because it means the doc is pointing to a wrong file.
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.
well, after all, what this doc refers to is today's version of the file.
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 I understand correctly, this is just an internal implementation detail, right? If so, would it make sense to omit it in this document?
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.
no. as mentioned above, it's a part of the current ABI. (thus making it use multivalue is unfortunately another ABI change.)
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.
My understanding is that this convention is now the default for LLVM, right? I think this document should say that.
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.
it's documented in the "Implementations" section.