Implement Emscripten jmp support in indirect function calls (invoke_xxx)

[jerbob92]

This PR implements Emscripten jmp support from indirect function calls, which we have missed before. We missed this mainly due to me lazily patching out Emscripten methods in standalone builds, in this case _emscripten_throw_longjmp, which is a pretty vital part of doing setjmp/longjmp from indirect function calls.

How that seems to work in the JS version:

• C calls longjmp from an indirect function call
• This causes the C code to call _emscripten_throw_longjmp, which is a host method (https://github.com/emscripten-core/emscripten/blob/6b6f7e7ed4aa8c1f2232443403fc8491056b01f6/system/lib/compiler-rt/emscripten_setjmp.c#L69),
• The host method then throws something identifiable (JS chose for Infinity, I chose a Go error)
• In the invoke function it implements stackSave/stackRestore that are exported by Emscripten, when the Indirect function call fails, it restores the stack, and when the error does not match the _emscripten_throw_longjmp type, it rethrows the error
• If the error does match up, it calls setThrew, probably to reset the throw status in C, because it wasn't actually a throw, it was a longjmp

I have tried to implement it in a very similar way in Wazero, but I have to say that I'm not aware of any edge cases where this does not work.

Sample C code:

#include <stdio.h>
#include <stdlib.h>
#include <setjmp.h>

void jmpfunction(jmp_buf env_buf) {
   longjmp(env_buf, 2);
}

int main() {
   int val;
   jmp_buf env_buffer;

   /* save calling environment for longjmp */
   val = setjmp( env_buffer );
   
   if( val != 0 ) {
      printf("Returned from a longjmp() with value = %d\n", val);
      exit(0);
   }
   
   printf("Jump function call\n");
   jmpfunction( env_buffer );

   return(0);
}

[jerbob92]

I think the tests currently fail because the invoke.wat/invoke.wasm do not contain the needed methods (stackSave/stackRestore/setThrew) right now, however, they are always included when you have invoke_ methods, so we can assume that they are always there.

[ncruces]

Looks good. I assume the same from the failing tests.

stackSave seems easy to stub out (do nothing), stackRestore I have no idea if it's safe to just ignore.

[ncruces]

Can you attach a wasm from built from your sample so can avoid figuring out how to build it? 😳

[jerbob92]

Looks good. I assume the same from the failing tests.

stackSave seems easy to stub out (do nothing), stackRestore I have no idea if it's safe to just ignore.

I think in the current tests both are safe to stub out, as the tests won't end up in the error branch. If we would make a separate test for the jumps we would have to implement them.

Can you attach a wasm from built from your sample so can avoid figuring out how to build it? flushed

Here you go, compiled with latest emscripten using emcc -g sample.c -o sample.wasm
sample.zip

[ncruces]

I think a better implementation would be as such:

	// Lookup the table index we will call.
	t := m.Tables[0] // Note: Emscripten doesn't use multiple tables
	f, err := m.Engine.LookupFunction(t, typeID, tableOffset)
	if err != nil {
		panic(err)
	}

	// Call: savedStack = stackSave()
	var savedStack [2]uint64
	err = mod.ExportedFunction("stackSave").CallWithStack(ctx, savedStack[:])
	if err != nil {
		panic(err) // stackSave failed
	}

	err = f.CallWithStack(ctx, stack)
	if err != nil {
		// Module closed: any calls will just fail with the same error
		if _, ok := err.(*sys.ExitError); ok {
			panic(err)
		}

		// Call: stackRestore(savedStack)
		if err := mod.ExportedFunction("stackRestore").CallWithStack(ctx, savedStack[:]); err != nil {
			panic(err) // stackRestore failed
		}
		if !errors.Is(err, ThrowLongjmpError) {
			panic(err) // f failed
		}

		// Call: setThrew(1, 0)
		savedStack[0] = 1
		savedStack[1] = 0
		err = mod.ExportedFunction("setThrew").CallWithStack(ctx, savedStack[:])
		if err != nil {
			panic(err) // setThrew failed
		}
	}

This uses CallWithStack to avoid a few allocs (to the cost of some obfuscation). We can further keep the [2]uint64 in InvokeFunc to avoid one alloc per call.

More importantly: stackRestore and setThrew here (hopefully that's the correct source) are in the catch block, so their exceptions interrupt flow. We should do the same.

And it's pointless to call stackRestore with a sys.ExitError, because it'll just do the throw out the sys.ExitError again.

[ncruces]

My changes, including updating tests so make test passes:
jerbob92/wazero@feature/implement-emscripten-jmp...ncruces:wazero:ncruces-emscripten-jmp

[ncruces]

OK, so, sorry for the noise, but here's another idea: what if we just skip the entire dance if mod.ExportedFunction("stackSave") returns nil?

[jerbob92]

OK, so, sorry for the noise, but here's another idea: what if we just skip the entire dance if mod.ExportedFunction("stackSave") returns nil?

Fine with me, but it should never be nil if it comes into the invoke call (except for not real emscripten like the Wazero tests).

I think a better implementation would be as such:

	// Lookup the table index we will call.
	t := m.Tables[0] // Note: Emscripten doesn't use multiple tables
	f, err := m.Engine.LookupFunction(t, typeID, tableOffset)
	if err != nil {
		panic(err)
	}

	// Call: savedStack = stackSave()
	var savedStack [2]uint64
	err = mod.ExportedFunction("stackSave").CallWithStack(ctx, savedStack[:])
	if err != nil {
		panic(err) // stackSave failed
	}

	err = f.CallWithStack(ctx, stack)
	if err != nil {
		// Module closed: any calls will just fail with the same error
		if _, ok := err.(*sys.ExitError); ok {
			panic(err)
		}

		// Call: stackRestore(savedStack)
		if err := mod.ExportedFunction("stackRestore").CallWithStack(ctx, savedStack[:]); err != nil {
			panic(err) // stackRestore failed
		}
		if !errors.Is(err, ThrowLongjmpError) {
			panic(err) // f failed
		}

		// Call: setThrew(1, 0)
		savedStack[0] = 1
		savedStack[1] = 0
		err = mod.ExportedFunction("setThrew").CallWithStack(ctx, savedStack[:])
		if err != nil {
			panic(err) // setThrew failed
		}
	}

This uses CallWithStack to avoid a few allocs (to the cost of some obfuscation). We can further keep the [2]uint64 in InvokeFunc to avoid one alloc per call.

More importantly: stackRestore and setThrew here (hopefully that's the correct source) are in the catch block, so their exceptions interrupt flow. We should do the same.

And it's pointless to call stackRestore with a sys.ExitError, because it'll just do the throw out the sys.ExitError again.

These changes sound good to me!

[ncruces]

Want to integrate them in your branch? Not sure I can.

[mathetake]

it seems the failing tests legit but not directly relevant to this change (I guess it's something to do with unsafe.pointer/uintptr cast thingy)

[mathetake]

no luck so far. The only thing I can say is this is not compiler specific (happens to interpreter as well), but I hanve't been able to reproduce