Document a setjmp/longjmp convention #225

yamt · 2024-04-01T05:40:26Z

No description provided.

yamt · 2024-04-01T05:44:23Z

SetjmpLongjmp.md

+  binaryen has a conversion from the old instructions to the latest
+  instructions. (`try_table` etc.)
+
+* Emscripten (TBD version) has the runtime support for the convention


i meant the Emscripten version which contains emscripten-core/emscripten#21502 here

The next release, which I expect to be done within a few days now, will contain this, so I think we can remove the 'TBD' part. Also we can have a link to the file here:
https://github.com/emscripten-core/emscripten/blob/main/system/lib/compiler-rt/emscripten_setjmp.c

will it be 3.1.57?
i prefer to mention versions for each components as not everyone uses the latest.

Yeah 3.1.57 is the next one

yamt · 2024-04-01T06:12:08Z

@aheejin @sbc100 @sunfishcode please review if you have time. thank you.

aheejin · 2024-04-01T15:08:29Z

Will be OOO for 3~4 days. Can I take a look after I return?

sunfishcode

Thanks for writing this!

sunfishcode · 2024-04-01T15:28:30Z

SetjmpLongjmp.md

+single longjmp execution.
+A runtime can use a part of `jmp_buf` for this structure. It's also ok to use
+a separate thread-local storage to place this structure. A runtime without
+multi-threading support can simply place this structure in a global variable.


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?

it's a part of the ABI. the compiler-generated code needs to know how to read members of this structure.

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?

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.)

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.

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.

Hmm, yeah, you're right. Would you like to submit a PR to the LLVM repo doing this?

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.

sunfishcode · 2024-04-01T19:31:31Z

SetjmpLongjmp.md

+```
+void __wasm_setjmp(void *env, uint32_t label, void *func_invocation_id);
+uint32_t __wasm_setjmp_test(void *env, void *func_invocation_id);
+void __wasm_longjmp(void *env, int val);


Would it be correct to change these void *envs to jmp_buf envs? That would make it a little clearer which env these are talking about.

sunfishcode · 2024-04-01T19:33:23Z

SetjmpLongjmp.md

+
+#### Reserved area in jmp_buf
+
+The first 6 words of C jmp_buf is reserved for the use by the runtime.


Could you add a note here making it explicit that the contents of these 6 words are not public?

sunfishcode · 2024-04-01T19:36:50Z

SetjmpLongjmp.md

+
+## Future directions
+
+* `__WasmLongjmpArgs` can be replaced with WebAssembly multivalue.


If I understand correctly, this is just an internal implementation detail, right? If so, would it make sense to omit it in this document?

no. as mentioned above, it's a part of the current ABI. (thus making it use multivalue is unfortunately another ABI change.)

yamt · 2024-04-02T01:46:03Z

Will be OOO for 3~4 days. Can I take a look after I return?

of course. no hurry. thank you.

aheejin

Thank you for working on this! Sorry for the late reply, and thank you for waiting for me.

aheejin · 2024-04-10T06:20:35Z

SetjmpLongjmp.md

+This document describes a convention to implement C setjmp/longjmp via
+[WebAssembly exception-handling proposal].
+
+[WebAssembly exception-handling proposal]: https://github.com/WebAssembly/exception-handling


80% of what this doc describes does not seem to be wasm EH-specific. They also apply to the old Emscripten (JS-based) SjLj. How about stating that most of this doc applies to both ways of SjLj handling described in https://emscripten.org/docs/porting/setjmp-longjmp.html and explaining about some wasm-EH specific parts within the doc, such as __WasmLongjmpArgs?

aheejin · 2024-04-10T07:01:54Z

SetjmpLongjmp.md

+single longjmp execution.
+A runtime can use a part of `jmp_buf` for this structure. It's also ok to use
+a separate thread-local storage to place this structure. A runtime without
+multi-threading support can simply place this structure in a global variable.


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.

aheejin · 2024-04-10T07:24:51Z

SetjmpLongjmp.md

+`__wasm_longjmp` is similar to C `longjmp`.
+If `val` is 0, it's `__wasm_longjmp`'s responsibility to convert it to 1.
+It performs a long jump by filling a `__WasmLongjmpArgs` structure and
+throwing `__c_longjmp` exception with its address.


How about mentioning that we throw an exception with a tag C_LONGJMP? It can be hard to associate what this function does with the 'Exception' section above.

Also, given that most of this doc is not exclusive to Wasm EH, we can mention we have emscriptne_longjmp JS-based EH in which we throw a JS exception.

i added some explanation about JS exception based convention.
i'm not sure how far it should be explained in this doc though.

aheejin · 2024-04-10T07:26:52Z

SetjmpLongjmp.md

+This convention uses a WebAssembly exception to perform a non-local jump
+for C `longjmp`.
+
+The name of exception is `__c_longjmp`.


Where does this name come from and what meaning does this name have?

We have this enum
https://github.com/llvm/llvm-project/blob/299b636a8f1c9cb2382f9dce4cdf6ec6330a79c6/llvm/include/llvm/CodeGen/WasmEHFuncInfo.h#L27
but this is just the name of an enum and doesn't have any meaning outside of LLVM. (Also the enum name is C_LONGJMP)

Should we instead say we assume the tag index 1 to be the longjmp index in C-based toolchain like LLVM?

Where does this name come from and what meaning does this name have?

see https://github.com/llvm/llvm-project/blob/b7a93bc1f230fe01f38f3648437cee74f339c5ac/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp#L110

We have this enum
https://github.com/llvm/llvm-project/blob/299b636a8f1c9cb2382f9dce4cdf6ec6330a79c6/llvm/include/llvm/CodeGen/WasmEHFuncInfo.h#L27
but this is just the name of an enum and doesn't have any meaning outside of LLVM. (Also the enum name is C_LONGJMP)

Should we instead say we assume the tag index 1 to be the longjmp index in C-based toolchain like LLVM?

the name is exposed to the outside for dynamic-linking modules.

The C symbol name used for the exception tag is ..

At the very least mention that it is a tag that we are talking about?

i improved exception vs tag distinction a bit.

aheejin · 2024-04-10T07:43:17Z

SetjmpLongjmp.md

+```
+    void
+    f(void)
+    {
+        jmp_buf env;
+        if (!setjmp(env)) {
+            might_call_longjmp(env);
+        }
+    }


Suggested change

```

void

f(void)

{

jmp_buf env;

if (!setjmp(env)) {

might_call_longjmp(env);

}

}

```c

void f(void) {

jmp_buf env;

if (!setjmp(env)) {

might_call_longjmp(env);

}

}

Nit: LLVM and Emscripten's clang-format style

Specify the block is in C for (slightly) better highlighting

Nit: LLVM and Emscripten's clang-format style

well, although i have no strong opinions about the style, this is not llvm or emscripten.

Why not? I pasted the code block into any LLVM source file and clang-formatted it, and it showed no changes (from what I suggested). LLVM's clang-format style is two-space indentation. Which part do you think this is different from LLVM's clang-format style? (And Emscripten's .clang-format uses two-space indentation too)

i meant that this repo (tool-conventions) is not a part of emscripten or llvm. thus does not necessarily follow their coding style.

if you like their coding style for some reasons, it's fine. i can change.

Ah I see what you meant. Yeah I know it's a nitpick (which is the reason I said 'Nit') but I just preferred it to be more like the code style we work on most of the times. Also having a separate line for the return type doesn't seem to be very common either (unless the line exceeds 80-col). I'd appreciate if you change it. Thanks!

but I just preferred it to be more like the code style we work on most of the times. Also having a separate line for the return type doesn't seem to be very common either (unless the line exceeds 80-col).

well, projects i work on most of the times use different styles.
i suspect there is no single "very common" style even among wasm related projects.
cf.
https://github.com/bytecodealliance/wasm-micro-runtime/blob/main/.clang-format
https://github.com/yamt/toywasm/blob/master/.clang-format

i guess C++ projects tend to prefer smaller indentation width than C projects for obvious reasons.

aheejin · 2024-04-10T08:12:28Z

SetjmpLongjmp.md

+
+* A PR to add the runtime support to wasi-libc is under review: https://github.com/WebAssembly/wasi-libc/pull/483
+
+[WebAssemblyLowerEmscriptenEHSjLj.cpp]: https://github.com/llvm/llvm-project/blob/70deb7bfe90af91c68454b70683fbe98feaea87d/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp


Suggested change

[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

i prefer to always use permalink as the files can be renamed/removed in future.

I think in that case we should fix the link, because it means the doc is pointing to a wrong file.

well, after all, what this doc refers to is today's version of the file.

aheejin · 2024-04-10T08:14:49Z

SetjmpLongjmp.md

+  documented above. It also supports a few traditional variations of the
+  setjmp/longjmp ABI.
+
+* A PR to add the runtime support to wasi-libc is under review: https://github.com/WebAssembly/wasi-libc/pull/483


Given that the PR was merged, can we have a link to a specific file, like https://github.com/WebAssembly/wasi-libc/blob/main/libc-top-half/musl/src/setjmp/wasm32/rt.c ?

aheejin · 2024-04-10T08:29:28Z

SetjmpLongjmp.md

+  binaryen has a conversion from the old instructions to the latest
+  instructions. (`try_table` etc.)
+
+* Emscripten (TBD version) has the runtime support for the convention


The next release, which I expect to be done within a few days now, will contain this, so I think we can remove the 'TBD' part. Also we can have a link to the file here:
https://github.com/emscripten-core/emscripten/blob/main/system/lib/compiler-rt/emscripten_setjmp.c

aheejin · 2024-04-10T08:44:47Z

SetjmpLongjmp.md

+
+  Note: older LLVM versions have been using a slightly different runtime ABI,
+  which is supported by Emscripten. It has been switched to the ABI documented
+  above by https://github.com/llvm/llvm-project/pull/84137.


Suggested change

Note: older LLVM versions have been using a slightly different runtime ABI,

which is supported by Emscripten. It has been switched to the ABI documented

above by https://github.com/llvm/llvm-project/pull/84137.

Not sure if we need to have a link to the PR, and given that presumably it is a non-goal of this doc to describe the history, I think it's fine to describe only the current status quo.

aheejin · 2024-04-10T08:47:38Z

SetjmpLongjmp.md

+## References
+
+* [A discussion about the runtime ABI](https://docs.google.com/document/d/1ZvTPT36K5jjiedF8MCXbEmYjULJjI723aOAks1IdLLg/edit#heading=h.metx1fc16ots)


Do we need this? We don't have any problem-exploring design doc links elsewhere in this repo, once we settle on a solution.

aheejin

Thank you for working on this! Sorry for the late reply, and thank you for waiting for me.

yamt · 2024-04-11T01:06:18Z

SetjmpLongjmp.md

+the last `__wasm_setjmp` call for the jmp_buf. The compiler-generated logic
+can use the label value to pretend a return from the corresponding setjmp.
+Otherwise, `__wasm_setjmp_test` returns 0. In that case, the
+compiler-generated logic should rethrow the exception by calling


probably it's also ok to rethrow with delegate/throw_ref?

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.

yea, i meant rethrow, not delegate.

aheejin

Thank you!

sbc100 · 2024-04-12T05:07:11Z

SetjmpLongjmp.md

+#### Reserved area in jmp_buf
+
+The first 6 words of C jmp_buf is reserved for the use by the runtime.
+It should also have large enough alignment to store C pointers.


You use the word "word" in the previous sentence and "C pointer" in this sentence, but are we not talking about the same thing? IIUC normally when folks way "word" they mean "pointer sized thing", so should we just stick to using one or other in this document?

(or am I misunderstand and are you using pointer and word to mean different sized things)

You use the word "word" in the previous sentence and "C pointer" in this sentence, but are we not talking about the same thing? IIUC normally when folks way "word" they mean "pointer sized thing", so should we just stick to using one or other in this document?

i added explanation.

aheejin · 2024-04-24T06:17:19Z

I think we can merge this? Anyone has any more comments?

dschuff · 2024-04-24T20:31:14Z

SetjmpLongjmp.md

+
+## Overview
+
+This document describes a convention to implement C setjmp/longjmp via


My understanding is that this convention is now the default for LLVM, right? I think this document should say that.

it's documented in the "Implementations" section.

dschuff · 2024-04-24T20:31:40Z

SetjmpLongjmp.md

+The first 6 words of C jmp_buf is reserved for the use by the runtime.
+("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.


Suggested change

The actual contents of this area is private to the runtime implementation.

The actual contents of this area are private to the runtime implementation.

fixed. thank you.

aheejin · 2024-05-09T21:45:11Z

I guess I can merge this now? Thanks for working on it again!

Document a setjmp/longjmp convention

8d80cb0

yamt mentioned this pull request Apr 1, 2024

Add libsetjmp.a/so WebAssembly/wasi-libc#483

Merged

yamt commented Apr 1, 2024

View reviewed changes

yamt added 2 commits April 1, 2024 14:46

typo

7a3233a

a note about the reserved area

ddd1e17

dynamic-linking consideration

933eba3

sbc100 approved these changes Apr 1, 2024

View reviewed changes

sunfishcode reviewed Apr 1, 2024

View reviewed changes

yamt added 2 commits April 2, 2024 10:55

make it clear "env" arguments are jmp_buf

afc947c

note that the layout of the 6-word area is not public

07dfc54

aheejin reviewed Apr 10, 2024

View reviewed changes

yamt added 10 commits April 10, 2024 18:26

add links to runtime implementations

871cab8

remove google doc reference

544f148

remove a note about history

cf83c3e

be explicit about LLVM version

a9689fe

specify language code blocks for possibly better rendering

13891ab

specify language for a few more code blocks

c2feeb4

fix indentation

119de61

mentions emscriptes js exception

a0cdee8

fill emscripten version

862948e

add some headweaving about catching logic

1f28a42

yamt commented Apr 11, 2024

View reviewed changes

yamt added 4 commits April 11, 2024 12:44

fix a typo

ed850b7

use "..." instead of ":" to mean omission

4a4d448

reduce indentation in wat

54ca9fb

reformat C code with LLVM's .clang-format

6da62fc

aheejin approved these changes Apr 11, 2024

View reviewed changes

yamt added 2 commits April 12, 2024 13:16

distinguish exception and tag a bit more clearly

c91b637

add a few ideas to the "Future directions" section

680e18f

sbc100 reviewed Apr 12, 2024

View reviewed changes

yamt added 2 commits April 12, 2024 14:34

make it clear what "words" are

82a90db

capitalize

baa023e

dschuff reviewed Apr 24, 2024

View reviewed changes

a gramatical fix

8c2c54f

aheejin merged commit 5b40a86 into WebAssembly:main May 9, 2024


		#### Reserved area in jmp_buf

		The first 6 words of C jmp_buf is reserved for the use by the runtime.


		## Future directions

		* `__WasmLongjmpArgs` can be replaced with WebAssembly multivalue.


		* A PR to add the runtime support to wasi-libc is under review: https://github.com/WebAssembly/wasi-libc/pull/483

		[WebAssemblyLowerEmscriptenEHSjLj.cpp]: https://github.com/llvm/llvm-project/blob/70deb7bfe90af91c68454b70683fbe98feaea87d/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp

		## References

		* [A discussion about the runtime ABI](https://docs.google.com/document/d/1ZvTPT36K5jjiedF8MCXbEmYjULJjI723aOAks1IdLLg/edit#heading=h.metx1fc16ots)


		## Overview

		This document describes a convention to implement C setjmp/longjmp via

	The actual contents of this area is private to the runtime implementation.
	The actual contents of this area are private to the runtime implementation.

Document a setjmp/longjmp convention #225

Document a setjmp/longjmp convention #225

Uh oh!

Conversation

yamt commented Apr 1, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yamt commented Apr 1, 2024

Uh oh!

aheejin commented Apr 1, 2024

Uh oh!

sunfishcode left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yamt commented Apr 2, 2024

Uh oh!

aheejin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yamt Apr 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yamt Apr 10, 2024 •

edited

Loading