Skip to content
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

Mapping Java null-pointer exceptions to WasmGC #208

Closed
jakobkummerow opened this issue Apr 26, 2021 · 68 comments · Fixed by #340
Closed

Mapping Java null-pointer exceptions to WasmGC #208

jakobkummerow opened this issue Apr 26, 2021 · 68 comments · Fixed by #340
Labels
Post-MVP Ideas for Post-MVP extensions

Comments

@jakobkummerow
Copy link
Contributor

Some more feedback from the J2CL team:

Many operations in Java, such as reading a member field of an object, throw a NullPointerException if the object is null. Translating such a member field access to a plain struct.get operation does not preserve Java semantics, because per the current proposal, struct.get traps if the object is null, and traps are not catchable in Wasm. (One might assume that NPE-throwing Java programs are buggy anyway and hence will not be executed in production; however unfortunately it turns out that such an assumption would be overly idealistic: real-world programs do rely on catching their own NPEs.) The obvious workaround is to put custom instruction sequences into the Wasm module, such as br_on_null before every {struct,array}.{get,set}. There is a concern that these custom checks bloat module size, and possibly hurt performance (depending on what exactly they'll look like, they might e.g. duplicate a check that the engine has to perform anyway -- though I expect that at least in some/simple cases, engines will be able to optimize away implicit checks when an explicit check has just been performed, i.e. struct.get should be able to not perform a null check if it follows a br_on_null instruction). The J2CL team is actively working on finalizing their implementation of such custom checks, which should give us some concrete numbers about their cost. I'll definitely follow up here once we have such data.

In the meantime, I already wanted to create awareness of this issue. Assuming the cost of custom checks ends up being painful, there are a few things we could do instead:

  • we could make traps catchable, or introduce a new kind of catchable trap. (I expect that this idea will be unpopular; just mentioning it for completeness. There's no debate that uncatchable Wasm traps are a good way to represent C/C++ segfaults; however with the GC proposal we are clearly moving way beyond the conventions of C-like languages, and it may be worth re-evaluating this choice.)

  • we could rebase the GC proposal on top of the exception handling proposal, and specify struct.get and friends to throw an exception instead of trapping. That would make WasmGC's own semantics a closer match to those managed languages that use NPEs or similar concepts, letting those languages piggyback more often on default Wasm behavior, instead of having to wrap so many operations in their own error-handling implementations.

  • other ideas?

(Of course, we can also do nothing. As discussed many times before, it's impossible for WasmGC to be a perfect match for every source language anyway, and maybe we shouldn't even try to match some source languages particularly well, so as to be source language agnostic. On the other hand, there is still the concern that WasmGC has to prove its usefulness in competitive comparison to existing, well-established and well-optimized technologies such as compiling to JS, so catering especially to certain popular use cases might just be the thing that makes the overall project viable and desirable.)

@jakobkummerow
Copy link
Contributor Author

Here's another idea (thanks to @ecmziegler for bringing it up):

  • allow modules to configure a "hook" function that's invoked before trapping; this function can be used to throw an exception (which would make the trap itself not-reached, and hence effectively convert the trap into an exception). Benefits:
    (1) It makes the whole thing opt-in, and e.g. modules that don't care about throwing (maybe because they do consider NPEs to be fatal errors) don't have to emit any exception-handling code;
    (2) This puts modules in control of what exception exactly they want to throw, as opposed to having to define built-in standard exceptions.
    This core idea leaves several open questions about details:
    (1) What's the granularity of defining trap hooks? Per-generating-instruction ("if struct.get traps, first invoke function X"), per trap reason ("if any instruction traps due to an object being null, first invoke function X"), both dimensions ("if struct.get traps due to the object being null, first invoke function X"), something else?
    (2) What's the signature of the trap hook functions? Presumably they return no value; should their parameters include trap-specific information (such as the object that was null, or its struct property / array index that was being accessed? Should they include some sort of source location information? Should they be statically typed (with module-specified types, as opposed to something generic like anyref)?
    (3) Should the trap hook actually be a hook-before-the-trap (as assumed above), or should it replace the trap entirely? The latter would imply that we have to specify what happens when the function returns normally (maybe another trap?).

I like this idea. One particular nicety is that we could easily make this a post-MVP follow-up (if we decide that long-term we want it for improved efficiency, but short-term it's not urgent enough to add to the MVP).

@lars-t-hansen
Copy link
Contributor

I feel the "this would bloat the module size" argument takes us back to the discussion about compression, since nearly any compressor would do a good job compressing the branch-around + operate-safely pattern. It would be good to see some data. The wasm generator should itself be able to elide null checks where they are redundant, or binaryen might.

The hook idea seems unpleasantly noncompositional and nonlocal and also like a new type of exception handling mechanism. I think it would be better to look for a local solution with properly exposed control flow. If the pair of nullcheck + operate is too large, maybe an instruction that merges the two is better, though eventually we'll have multibyte opcodes so there's a limit to how compact we can make that.

@jakobkummerow
Copy link
Contributor Author

If the pair of nullcheck + operate is too large

It's more than that: a br_on_null also needs a jump target, and that place needs to contain instructions to e.g. compose an error message and throw an exception. (Obviously we'll need to look at implementations to assess how big exactly the overhead would be.)

@fgmccabe
Copy link

Supporting 'compile-time' functions aka macros would go a long way to addressing code size problems.

@Horcrux7
Copy link

As a writer of a java compiler I like the idea of registering a global function without any parameter as a hook. The registering should be pro trap type. Then it is possible to register the same function, different functions or nothing. The hook functions must throw an exception.

possible trap types of interest:

  • field access of null object
  • array index out of range
  • division by zero

If we want add add parameters to the hook functions then every hook needs different parameters

  • NPE: type and field
  • AIOOBE: array index
  • DBZE: nothing

@tlively
Copy link
Member

tlively commented Apr 26, 2021

Perhaps I'm biased toward having lots of instructions by working on SIMD, but I think the simplest and most composable solution would be to create throwing versions of all the relevant instructions that currently trap. For example, we could have i64.div_u_throw $e that takes an event index immediate and behaves like a throw $e when the denominator is 0.

@aardappel
Copy link

At least for use in browsers, can't JS catch a Wasm trap and convert it into an exception of whatever? I understand we'd prefer a solution in pure Wasm, but this could work for J2CL presumably?

Is anything specified about what a WASI host can do with Wasm traps?

@jakobkummerow
Copy link
Contributor Author

can't JS catch a Wasm trap?

Yes it can, but doing so unrolls the Wasm stack all the way to its entry point (i.e. the topmost JS frame). So if you have a Java function f1() { try { f2(); } catch(...) {...} and f2 triggers an NPE, then you'd need a fairly convoluted way to translate that to a combination of Wasm+JS so that the trap generated by f2 can somehow be caught and routed to the catch block in f1.

@tlively
Copy link
Member

tlively commented Apr 26, 2021

I guess for now the options are to trampoline through JS before executing trapping operations to convert traps into exceptions or to guard against trapping operations explicitly in the Wasm. Based on our experience doing something like the former to support C++ exceptions in Emscripten without the EH proposal, my guess is that the explicit guards in WebAssembly would be more attractive.

@fgmccabe
Copy link

IMO, having a 'standard' trap->exception function seems like a 'bad idea'(tm). It makes the specification much more difficult to reason about. This is especially true if that becomes an environmental aspect (like whether the code is running in a browser or not).

@aardappel
Copy link

but doing so unrolls the Wasm stack all the way to its entry point (i.e. the topmost JS frame)

Well, since this issue is considering all sorts of possible new additions to Wasm, having a way to create a new JS frame on top of the stack upon trap would be a possibility? Basically tell the JS API to not unwind. Or are there engine/JS limitations that would make this a bad idea? Could be useful for other things than just catching Java NPEs.

@aheejin
Copy link
Member

aheejin commented Apr 27, 2021

FYI, this is a summary of the rationale on why we decided not to catch traps, after long discussions: WebAssembly/exception-handling#1 (comment)

I think reasons in this summary still stand, and I don't think making all traps catchable is a good idea. But I agree that there are ways we can think more about, including suggestions in this issue. To summarize what have been suggested here:

  1. Make all traps catchable
  • I don't think this is a good idea because of the rationale I provided above. Also it will likely to increase code size for C++ and make its debugging chaotic.
  1. Make throwing variants of some trapping instructions
  • Q: How many of those instructions do we need? div, load, store, all GC instructions that traps on null, ... Is this a scalable approach?
  1. Make a class of catchable traps
  • This also boils down to 2; because while we can make traps from new GC instructions special "catchable traps", we can't make all traps catchable for existing instructions like div, load, or store, which will affect other languages too.
  • I think this approach is similar to one of what @jakobkummerow suggested, which lets instructions like struct.get throw an exception instead. That approach also has to address existing instructions like div.
  1. Make a hook for traps
  • I like this option that it is opt-in and we can tackle this separately from the core spec. Not sure if we can do it in the JS API side, in which there is the default JS API and also a customized API for J2CL. When some instruction traps, at which point does it become a JS exception? Can we do something at that point?
  1. Make a macro-like things in wasm for repeated pattern of instructions
  • This is a rather new concept in wasm. Should we propose "macro" spec for this or something?
  1. Do nothing, hoping the compressor will do a good job
    🤷🏻

@Horcrux7
Copy link

For 5. and 6.
Is code size the real problem? Is it not also a performance problem? Can the WASM runtime produce effective native code in this case? For all the different constructs from many different languages?

@gkdn
Copy link

gkdn commented Apr 27, 2021

I sounds like that we are trying to work around existing assumptions due existing languages target of WASM.

In managed environments, apps cannot result in trap / segmentation fault or anything that would cause a crash (assuming no VM bugs). For JVM in particular, every app level problem including even stack overflow, hitting heap memory limits or even excessive GC are catchable at application level so they could be handled gracefully.

So I believe having traps and being them not catchable is already favors particular language style instead of making it less language dependent. I think having ability to influence this behavior at module level (either opting in to make traps catchable or ability to change what is thrown with a hook) seems reasonable to me.

@gkdn
Copy link

gkdn commented Apr 27, 2021

With respect to instrumentation option:

Generally speaking, I believe having more bloat makes it harder to reason about the code at optimization (either offline or engine level). Macros seems generally useful to reduce the size in this context and many other patterns that we generate with J2CL but it doesn't directly address the complexity impact from the optimizations perspective.
Being said that we should be able to experiment with instrumentation and see the real life implications.

@titzer
Copy link
Contributor

titzer commented Apr 28, 2021

I was just about to suggest something similar to the hook function, and then I saw it was the second comment. This is the least invasive change to Wasm, but it has problems with composability that need to be worked around. For one, composing modules from different languages would be difficult if the hook is global--it might need to be per-module, at the least. While wasm doesn't yet have threads or thread-local state, it would probably be best to specify the hook in a way that is forward-compatible with thread-local variables, i.e. that it can be mutated at least by the thread itself. E.g. there might be different contexts within one module that want to handle traps in different ways.

In general, having implicit nullchecks (via trapping on null) is a very important optimization for Java. There are just far too many sites to add explicit code, and compression only helps module size, not the size of the resulting machine code.

As @aheejin mentions, I think catching traps by default is not a good option, because Java exceptions generally have stacktraces associated with them, and I think we want to avoid requiring engines to collect stacktraces for exceptions if we can. Even though one can suppress stacktraces for Java exceptions, and VMs sometimes optimize them away, it is generally the case that they are needed.

@rossberg
Copy link
Member

From @aheejin's list, I think (2) is by far the most attractive option.

  1. Make all traps catchable

I agree this isn't a good idea, and as @aheejin points out, that is already the recorded consensus of the CG.

  1. Make throwable variants of some trapping instructions

This wouldn't be too bad, I believe. There aren't that many instructions that trap, and not all of them need catchable alternatives. For example, memory out of bounds accesses, i.e., loads and stores probably don't. And it's only fairly few other cases, e.g, I count 5 relevant instructions in the GC MVP. (And even in case we wanted catchable loads/stores, they could be represented with just a flag bit in the instruction encoding.)

  1. Make a class of catchable traps

That seems more complex and less clear and adaptable than (2). For example, there are certain cases of null that are programmatically useful in suitable contexts (e.g., when a struct is null), while others can only ever originate from fatal compiler bugs (e.g., when an RTT is null).

  1. Make a hook for traps

As @lars-t-hansen points out, this does not compose. As a general rule, there must not be any stateful behaviour modes that are global or tied to modules, because either fundamentally conflicts with modular composition, transformations, and refactorings. The only way in which this would not cause serious issues is as an explicit, scoped control-flow construct, essentially like an exception handler. But then it's simpler to reuse exceptions themselves.

  1. Make a macro-like things in wasm for repeated pattern of instructions

AFAICS, this doesn't address this use case well, since an engine would still have to recognise certain instruction patterns to optimise them.

@titzer
Copy link
Contributor

titzer commented Apr 28, 2021

Make a hook for traps
As @lars-t-hansen points out, this does not compose. As a general rule, there must not be any stateful behaviour modes that are global or tied to modules, because either fundamentally conflicts with modular composition, transformations, and refactorings. The only way in which this would not cause serious issues is as an explicit, scoped control-flow construct, essentially like an exception handler. But then it's simpler to reuse exceptions themselves.

It doesn't have to be an exception handler. If we have thread-local variables, we could have a control flow construct that introduces a let-like scope, assigning a value to a thread-local variable at the beginning of the scope and restoring the variable to the previous value upon exit (all paths) from the scope.

@tlively
Copy link
Member

tlively commented Apr 28, 2021

Adding new throwing instructions seems clearly simpler than adding hooks or new control flow constructs because it does not introduce anything fundamentally new to the spec and does not raise any composability or forward compatibility questions (at least so far!). For those who prefer a hook mechanism, what downside do you see in adding new throwing instructions?

@rossberg
Copy link
Member

It doesn't have to be an exception handler. If we have thread-local variables, we could have a control flow construct that introduces a let-like scope, assigning a value to a thread-local variable at the beginning of the scope and restoring the variable to the previous value upon exit (all paths) from the scope.

Where would you resume after the hook was invoked? AFAICS, it can only be after the end of that construct. And then it's pretty much isomorphic to an exception handler.

@titzer
Copy link
Contributor

titzer commented Apr 28, 2021 via email

@fgmccabe
Copy link

If you are going to have an instruction throw an exception on trap, you must also solve the next two problems: what is the nature of that exception and (assuming you solve the first) how do I map that exception to C++ exceptions/Python exceptions/JS exceptions.

@Horcrux7
Copy link

what downside do you see in adding new throwing instructions?

@tlively with throwing instruction there is the problem that the exception must be converted to an exception that the languages are expected. For example for Java there must be allocated an object NullPointerException on the heap. With a hook this can be simple throw an exception that is compatible to all the try catch constructs of the language.

@RossTate
Copy link
Contributor

Adding to that, if ever WebAssembly wants to support in-application stack tracing for exceptions, then the handler should happen "in place" so that it can use the stack marks or the like at that location to determine the relevant stack info.

The aforementioned issue with compositionality by having the trap handler be specified per function rather than per module. Due to interop constraints (e.g. calling to imported functions), any trap-handler design/implementation will likely have to reasonably accommodate at least per-function granularity anyways. Finer grained than that would probably not be useful and would be challenging for engines (as trap handlers might do "meaningful" things that an optimizer has to account for). For that same reason, the trap handler should probably always be specified through lexical scope rather than dynamic scope; that is, a function's trap handler has no effect on how traps within functions it calls are handled.

@gkdn
Copy link

gkdn commented Apr 28, 2021

what downside do you see in adding new throwing instructions?

It solves the most problematic parts; array and property accesses but not the overall problem. Anything else that may result in trap need to be instrumented (casts, arithmetic, etc) or need a throwing version of the same instruction.

exception must be converted to an exception that the languages are expected

That should not be a problem for our case; we had the same issue in the JS land and on catch we create the proper exception type expected from Java.

@tlively
Copy link
Member

tlively commented Apr 28, 2021

Oh right, I hadn't thought about all the user-space setup a runtime would have to do before actually throwing an exception.

That being said, throwing instructions still seems like the right solution to me. The exception thrown from e.g. a divide by zero would be caught (probably nearby in the same function) by a handler that allocates whatever objects are necessary to construct the "real" exception then throws that exception. This is similar to the per-function trap handler idea, except not tied to function granularity and composed of concepts that already exist.

@gkdn
Copy link

gkdn commented Apr 28, 2021

If this wasn't a hook but there was a module level setting that let the module to choose to catch traps or not (i.e. not configurable after declaration nor configurable per trap), would that still have the same concerns of having hooks?

@tlively
Copy link
Member

tlively commented Apr 28, 2021

Yes, WebAssembly has so far (almost) avoided having module-level configuration bits and hooks like that to keep modules composable and decomposable. Whether the module-level configuration were a bit or a hook, it would still make it impossible to statically merge and optimize two modules that use different configurations.

@aheejin
Copy link
Member

aheejin commented Apr 28, 2021

@tlively

Adding new throwing instructions seems clearly simpler than adding hooks or new control flow constructs because it does not introduce anything fundamentally new to the spec and does not raise any composability or forward compatibility questions (at least so far!). For those who prefer a hook mechanism, what downside do you see in adding new throwing instructions?

My concern was primarily about scalability with existing instructions. But as @rossberg suggested if it can be done by setting a flag, I think this can be a viable option too.

@fgmccabe

If you are going to have an instruction throw an exception on trap, you must also solve the next two problems: what is the nature of that exception and (assuming you solve the first) how do I map that exception to C++ exceptions/Python exceptions/JS exceptions.

I think different languages should use different sets of instructions that make sense to them. For example, throwing version of trapping instructions wouldn't make sense in C++, so C++ will continue to use the original trapping version.

@RossTate
Copy link
Contributor

Yes, but it may be worth the advantages of keeping compilation simpler. It's not clear to me that there are many applications for inlining functions with different handlers (except where the inlined function's handler behaves the same on the traps the inlined function might incur).

@RossTate
Copy link
Contributor

That said, you could have a handle_traps_with $handler instr* end block instruction. But you'd want it to be something that can wrap the whole function body in the typical case. (There's also a corner case with stack-overflow traps.)

@titzer
Copy link
Contributor

titzer commented Apr 30, 2021

@RossTate A block-scoped handle_traps_with instruction is less general than what I proposed, which was a block scope to assign to a thread-local variable (when we have those) and restore it upon exit. Then if the trap handler is just per-thread state, i.e. a thread-local variable, it can be set and reset in a properly scoped fashion. We could even allow some thread-local variables to be declared that force them to only be assigned with the scoped assignment construct.

@titzer
Copy link
Contributor

titzer commented Apr 30, 2021

Throwing versions of instructions does not support the "catching traps from uncooperative modules for testing purposes" use case that I mentioned above.

@titzer
Copy link
Contributor

titzer commented Apr 30, 2021

Inlining a function with one handler into a function with another handler would become nontrivial.

It's really no worse than inlining a function that has try/catch. That said, I think engines can and will inline heavily in the future, so I don't think we should unnecessarily make things complicated for them when they do.

@RossTate
Copy link
Contributor

The tooling and engine should be able to statically determine the trap handler. Otherwise completely arbitrary code could be run during any trap, which can make it hard to optimize and compile. (For example, can two divide instructions be reordered?) For this same reason, we want modules to be able to specify very few (e.g. typically one) trap handler so that tooling and optimizers can analyze and summarize it (i.e. what effects do the various handlers have that the optimizer/compiler needs to be conscious of).

You also want to think about calls into other wasm modules compiled from other languages (possibly using the "default" trap handlers, i.e. trapping). Your trap handler should not affect how their traps are handled (except possibly for if the call itself traps, i.e. except for unhandled traps - which are then handled as the call instruction trapping rather than as whatever instruction inside the call trapping).

So for both implementation and semantic reasons, my current thinking is that any design that does not ensure trap handlers are statically determinable is problematic.

@titzer
Copy link
Contributor

titzer commented Apr 30, 2021

Traps are already catchable in JavaScript, so their precise ordering is observable. That's a good thing, because we don't want engines reordering traps for the same reason we don't want them (observably) reordering stores to memory or other side effects.

@RossTate
Copy link
Contributor

In the current spec, division instructions are reorderable/parallelizable with cast instructions (among many other examples). That would not be the case with arbitrary trap handlers. Also, non-locally-catchable traps are specific to the JS embedder, and my understanding is other embedders are interested in avoiding that partly because of the significant compilation constraints that imposes, instead making sure that a trap terminates all computation that has access to the intermediate state that would expose at which point the trap occurred (such as in the coarsest strategy where a trap simply terminates the whole process).

@RossTate
Copy link
Contributor

That reminds me, another thing to consider in this space is resumable trap handlers. This can be useful for two reasons.

The first is fault-tolerant programming and compilation. In this space, it's more important for a program to just keep going even if it's possibly going incorrectly. For example, rather than having i32.store effectively throw an exception when the index is out of bounds, a fault-tolerant resumable trap handler would just have it keep going. And similarly, i32.load would just return 0 when the access is out of bounds.

The second is optimizability. The above trap handlers for i32.store and i32.load are way more optimizable than the current (default) trap handlers because they are side effect free whereas the current one is very effectful.

But for resumable trap handlers to be manageable and to benefit from the above observation, it's very important that they be statically determinable. And we wouldn't need to add support for them right away; for now, we could require all trap handlers to have unreachable output type, and later we can enable resumable ones by relaxing this requirement for appropriate traps.

@Horcrux7
Copy link

Horcrux7 commented May 2, 2021

We should really wait until we get actual data about the importance of this.

@fgmccabe Which concrete data do you need?

@tlively
Copy link
Member

tlively commented May 2, 2021

It would be useful to to see how much having to instrument accesses and arithmetic to throw rather than trap under the current proposal increases code size and reduces performance compared to assuming no accesses or arithmetic would trap/throw. Once we have a better understanding of the current costs, we will be able to make more informed assessments of the various proposed solutions. A good next step might be to prototype some of the solutions.

@RossTate
Copy link
Contributor

RossTate commented May 3, 2021

The answer to that depends on whether we want WebAssembly to (eventually) support generators that would error provenance, whether for semantic or debugging purposes. If we do, then the cost comes down to the combination of frequency of relevant (surface-level) instructions and size increase of their lowerings.

For example, an array access in Java for an int[] (so ignoring casting issues) would translate to (with trap handlers) array.get whereas with, say, alternative labels the access would translate to:

(block ([(ref null (array (mut i32))) i32] -> [i32]) $no_trap ;; block types can be consolidated
  (block ([(ref null (array (mut i32))) i32] -> []) $null_pointer ;; block types can be consolidated
    (block ([(ref null (array (mut i32))) i32] -> []) $out_of_bounds ;; block types can be consolidated
      array.get_no_trap $null_pointer $out_of_bounds
      br $no_trap
    end)
    call $throw_IndexOutOfBoundsException ;; unreachable return type
  end)
  call $throw_NullPointerException ;; unreachable return type
end)

Similar for field access via struct.get (though with only one alternative label) and the like. So if someone has a representative wasm binary and a tool that can count the occurrences of these instructions, we could quickly compute a rough sense of the size increase without a prototype generator.

Of course, code size is only one part of the problem.

@tlively
Copy link
Member

tlively commented May 3, 2021

Another neat idea due to @sunfishcode: We could have versions of trapping instructions that take a function index immediate and call the function when the normal version of the instruction would otherwise trap. The main idea is that instead of each function having a code block for preparing and throwing an exception, this could be factored out into a single function referenced by all the non-trapping instructions. This is similar to having a global trap handler, but it also uses existing mechanisms and is composable.

The precise semantics regarding function arguments, results, whether or not to trap on non-exceptional return from the handler function, etc. can all be fleshed out and bikeshedded if the core idea sounds worth pursuing.

@RossTate
Copy link
Contributor

RossTate commented May 4, 2021

That still does not address the implementation and optimization concerns above about per-instruction-granularity trap handling.

At this point, I don't think the issue is about composability (a function-granularity has module composability, and block-granularity further has function composability). I also don't think this is so much about reusing existing concepts. Rather than making a new variant of every instruction specifying a function for each kind of trap, we could decide to define trap handlers to just specify, for each trapping instruction and each way the instruction could trap (that the handler cares about), a function to call of the appropriate type.

To me, the issue is about granularity. Enabling a module to have just a couple of trap handlers would make it reasonably possible (though still non-trivial) for engines to be optimize code with custom trap handlers. I don't believe that will be so easy if every instruction can have custom behavior (at the least optimization will be much more ad hoc). Furthermore, putting optimization aside, it will be easier for engines to implement custom trap handlers via code ranges, for reasons already given above.

I'm inferring that this is unappealing to some, and it would be helpful have reasons articulated.

@tlively
Copy link
Member

tlively commented May 4, 2021

For the optimization concerns you mentioned, are you thinking about optimizations on the engine side? On the producer side, I don't think there would be any extra optimization issues due to granularity. The important part on the producer side would be that the handler is statically determinable, as you mentioned.

I would be interested to hear from engine folks about the tradeoffs around handler granularity. I imagine that much of the existing infrastructure for detecting traps and turning them into JS exceptions could be reused to execute trap handlers with arbitrary granularity, but I may be missing some considerations.

@RossTate
Copy link
Contributor

RossTate commented May 4, 2021

More so on the engine side, though it's useful to know that on the producer side you don't think there would be a scaling issue.

I echo your question for engine folks. My concern is that the existing infrastructure you mention has some slackness to it that the semantic considerations of finer granularity would not permit. Also, my understanding is that not all engines turn traps into exceptions with debug information and so don't need to worry about generally preserving provenance information during compilation, which fine-grained trap handling would impose.

Thanks!

@conrad-watt
Copy link
Contributor

conrad-watt commented May 4, 2021

We could have versions of trapping instructions that take a function index immediate and call the function when the normal version of the instruction would otherwise trap.

I like this idea. It's a more flexible solution than function or block trap handlers that doesn't require us to go into the weeds of how to define the table that associates each kind of trap with its handler (e.g. NPE for struct.get, DBZ for i32.div). My concern is that we'd never make such a table fully future-proof and would need to keep on adding ad-hoc generalisations/work-arounds. For example, what if in the future we hit a case where the same instruction needs to throw different exceptions depending on the types of its operands?

I'm not saying that engines couldn't get some hypothetical benefit from a function-level handler approach, but I'd prefer to go with the most flexible solution as a default and wait for a case from engines that they'd prefer the alternative.

Also, my understanding is that not all engines turn traps into exceptions with debug information and so don't need to worry about generally preserving provenance information during compilation, which fine-grained trap handling would impose.

Could you go into more detail about why this is more of a problem for instruction-level trap handling compared to function-level trap handing? In my head, engines would have code generation and optimisation paths for "there is a trap handler associated with the trapping case of this instruction" which (in your scenario) would require provenance information to be preserved, irrespective of whether other code generation/optimisation paths do not preserve provenance, and irrespective of whether the trap handler was defined at an instruction or function granularity.

@jakobkummerow
Copy link
Contributor Author

From an engine point of view, I'm not concerned about (tail-?)calling a custom function instead of producing a trap: generating a trap is a function call too. Whether that custom function is specified per-module, per-function, per-block, or per-instruction shouldn't make much of a difference for the engine, or for the optimizations it can perform.

Example: if the engine is able to optimize away a null-check by propagating some non-null-ness information, then it doesn't matter whether the check that was optimized away would have called the internal "generate a 'null' trap" function or some custom function. Similarly, an engine that builds type propagation infrastructure can determine that a value is non-null after having passed a null-check, regardless of whether that null-check would have produced a trap or an exception or called a function upon failure.

@fgmccabe
Copy link

fgmccabe commented May 4, 2021

It seems to me that there are a lot of proposed 'solutions' to a problem that we are not sure is real. (I too am guilty as charged).
It also seems to me that adding special hooks with implicit function calls has a pretty strong 'code smell' in the design.
Realistically, the extra cost of an ref.is_null instruction is 2 bytes (for a local branch). Unless some 10-20% of the generated Java code consists of struct.get, it does not strike me as being particularly onerous.
In addition, there are other, more powerful, techniques that we can deploy to look at code compression.
So, unless and until we get some actual data, I suggest holding off on attempting to solve this problem.

@rossberg
Copy link
Member

rossberg commented May 4, 2021

Agreed with @fgmccabe. FWIW, this is a reoccurring kind of discussion. So far, Wasm's design has almost always erred on the side of not introducing "macro instructions". Like, it doesn't even have i32.neg, you need 4 bytes to express that.

@tlively
Copy link
Member

tlively commented May 4, 2021

I agree that code size alone is probably not enough to motivate a lot of work here. I guess I've been thinking that avoiding duplicated null checks in the engine and user space would be beneficial, but perhaps that would be trivial for engines to optimize anyway. We should gather data on that as well before committing to pursuing any of these ideas further.

That being said, I've found the brainstorming on this thread useful. A number of ideas have come up that I would not have initially considered. I think it's useful to continue sharing ideas in this space as long as we don't get bogged down bikeshedding or debating the details before we have data demonstrating that there is a problem that needs solving.

@Horcrux7
Copy link

Horcrux7 commented May 4, 2021

It is not only a duplicate null check. There are many more:

  • duplicate the value on stack
  • if not from a local variable then an extra local variable is needed with a tee and get operation
  • the null check
  • creating the exception and throwing it, can be a function call

For an array index access

  • we need 3 extra references of the object
  • the array index must also be copy 2 times on the stack
  • then the length of the array must be copied 2 times
  • then the 3 checks (null, lower index, higher index) must be do
  • the 2 different exceptions must be created and throwing

This are minimum 14 extra instructions for one array index access.

A good solution can be to move this all in an addition function call. But the WASM type check required a different function for every array type.

@lukewagner
Copy link
Member

While we have the general goal of not adding macro-instructions just for size benefits, wasm has thus far aimed to reliably eliminate runtime checks as a way to increase predictability of performance (reducing dependency on compiler magic) and to help out "baseline" compilers. So +1 to thinking about explicitly parameterizing trapping instructions to have non-trapping paths.


Separately, to the question of why we should or shouldn't feel uneasy allowing wasm to handle traps: while I agree there are use cases for allowing wasm to handle traps, and I think we need to eventually address these use cases, I think there is pretty serious ecosystem risk if we allow arbitrary core wasm code to handle traps. In general, a trap means a Bad thing happened, and if arbitrary wasm code can handle traps, I think we'll end up with swallowed traps leading to corrupt continued execution (which is a bad outcome). Rather, I think we should define some explicit "blast zone" unit that semantically contains a set of core instances and low-level mutable state, where a trap always takes down its entire containing blast zone, leaving instances outside the blast zone alive and able to observe the failure. Importantly, having an explicit blast zone unit gives developers a fighting chance to ensure that all state transitively corruptible by a trap is taken down with the trap.

(FWIW, how "blast zones" relate to components is an open question to me. Currently, I imagine a blast zone containing multiple component instances; the shared-nothingness of components being a necessary but not sufficient condition for bounding corruption.)

@fgmccabe
Copy link

fgmccabe commented May 5, 2021

@lukewagner I agree completely
I would say, in addition, that the semantics of Java allow recovery from some operations that Wasm considers trap scenarios.

@Horcrux7 the longer list of special operations confirms me in my preferred approach: support some form of macro/inline capability. The logic of this is pretty straightforward: without them Wasm is guaranteed to be less compact than JVM at representing JVM code. (The same is true for any language with a byte code.)

@tlively
Copy link
Member

tlively commented May 6, 2021

A macro system or other compression scheme is certainly valuable, but is out of scope for and orthogonal to the GC proposal. Let's move further discussion of macros to a separate thread. You may be interested in the prior proposal for compressing WebAssembly: https://github.com/WebAssembly/decompressor-prototype/blob/master/CompressionLayer1.md. Unfortunately that proposal has been inactive for the last five (!) years.

@RossTate
Copy link
Contributor

RossTate commented May 6, 2021

I think it has been good to have some higher-level discussion (e.g. should we solve this problem now), but now I'm wondering what the status of this discussion is. There were some lower-level questions asked that I'm happy to continue on, but only if the group is still interested in pursuing such questions. I myself am fine either way.

@tlively
Copy link
Member

tlively commented May 7, 2021

Personally, I would like to continue collecting high-level ideas about directions for possible solutions while we await data on the magnitude and character of the problem to guide more detailed and low-level discussions. Of course any new information about the problem space would be very welcome, too. If there is appetite to discuss specific potential solutions in detail right now, perhaps those conversations can be split into separate threads; this high-level thread is already quite long.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Post-MVP Ideas for Post-MVP extensions
Projects
None yet
Development

Successfully merging a pull request may close this issue.