WebAssembly / gc

Branch of the spec repo scoped to discussion of GC integration in WebAssembly
https://webassembly.github.io/gc/
Other
1k stars 72 forks source link

Mapping Java null-pointer exceptions to WasmGC #208

Closed jakobkummerow closed 1 year ago

jakobkummerow commented 3 years ago

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:

(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 commented 3 years ago

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

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

Horcrux7 commented 3 years ago

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:

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

tlively commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

FYI, this is a summary of the rationale on why we decided not to catch traps, after long discussions: https://github.com/WebAssembly/exception-handling/issues/1#issuecomment-546568318

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.
  2. 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?
  3. 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.
  4. 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?
  5. 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?
  6. Do nothing, hoping the compressor will do a good job 🤷🏻

Horcrux7 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

It is a trap handler hook, so if the handler didn't explicitly throw or otherwise set a resumption point, the runtime should trap.

On Wed, Apr 28, 2021 at 1:59 AM Andreas Rossberg @.***> wrote:

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.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/WebAssembly/gc/issues/208#issuecomment-828199208, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC46VVBCEN3TGBE72N73NHDTK6W5JANCNFSM43SXNGGQ .

fgmccabe commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

Many traps are detectable by hardware (e.g. the DE error for division by zero in x86) and handled by a trap gate. The trap gate needs to consider the current PC to determine what to do. With function-granularity trap handlers, the engine only needs to be able to tell which function the PC is within. With throwing instructions, every division instruction could throw a different exception, which requires the engine to keep track of how code is emitted at a rather fine granularity, including optimizers.

So, in addition to being able to execute "in place", I suspect function-granularity trap handlers would be easier to implement efficiently.

gkdn commented 3 years ago

@tlively I think I get the concern now. Thanks.

If it is not the module but the 'try/catch' makes the distinction, then there wouldn't be an implication on module composition/decomposition, right?

titzer commented 3 years ago

There is a use case for being able to catch arbitrary traps: testing frameworks. In this scenario, a driver module wants to call functions from modules under test and catch any (fatal) traps that they generate, collecting them as failures and moving on. Having traps be uncatchable forever would not allow that, and having exception-throwing versions of instructions that currently trap would not help, because modules under test would be forced to cooperate.

I would summarize that as: some modules do want to catch traps from uncooperative modules.

Horcrux7 commented 3 years ago

Some question to the new throwing instructions concept:

fgmccabe commented 3 years ago

An alternative to having a throwing variant of struct.get, one could have a branching version: branch out of an identified block when failing.

fgmccabe commented 3 years ago

Generally, a machine like Wasm is lower-level than the JVM (say); or even ANY language specific VM. This will negatively impact code size: the wasm instruction sequence is going to be inevitably longer than the JVM byte code. This will have an impact on code size that goes beyond checking for null references. One route to a solution here is to embrace macros (compile-time functions). But, that is a pretty big thing to chew on.

tlively commented 3 years ago
  • What would receive a try/catch handler with this new throwing instructions? I am unclear how the conversion to the languages exception should work.

If language exceptions require nontrivial setup, then the code generator would want to insert a separate chunk of code to set up and throw the language exceptions. With the new throwing instructions idea, the new instructions would throw a trivial Wasm exception that is caught in the same function to enter the language exception setup code.

  • Should it be possible to catch null access traps from another module with a language that does not support it? This would not be possible with new throwing instructions. This concept required a change on the use and on the catch code.

Correct. As the CG decided, traps should never be able to be caught (using mechanisms from the EH proposal) by any module. I don't think this is a problem for this design.


An alternative to having a throwing variant of struct.get, one could have a branching version: branch out of an identified block when failing.

I quite like this idea. It makes the language exception setup I described above much simpler, since there is no new trivial exception to throw or catch, but just a normal branch to the setup code instead. The new throwing instruction idea would be more attractive for languages that can directly use Wasm exceptions with no setup, but that does not seem to be the common case.

aardappel commented 3 years ago

@fgmccabe new branch instructions are a relatively "heavy" addition to Wasm though, since any tools need to typically process them specially.

And they only add a mild amount of code compression compared to just using an existing branch instruction.

fgmccabe commented 3 years ago

So, I kind of agree with that. We are talking about ref.is_null followed by struct.get which is 1 byte more than struct.get_branch We should really wait until we get actual data about the importance of this. IMO, macros are the way to go if you want to be serious about code size. And that would have many more applications. But, it is also a much bigger lift to get macros adopted.

kripken commented 3 years ago

Agreed 100% we need to wait on actual data here. But to add something we should measure on that data, there is a point @titzer made earlier:

compression only helps module size, not the size of the resulting machine code

If there is a way to get null checks in all the places Java needs that avoids adding machine code in each possible location (using a signal handler / hook), and if those wins are big enough, then I think it's worth considering all of our options to achieve that, including compromising on composability.

RossTate commented 3 years ago

Having the handler be specified per function rather than per module maintains composability while not increasing code size.

tlively commented 3 years ago

@RossTate that would make functions less composable, wouldn't it? Inlining a function with one handler into a function with another handler would become nontrivial.

RossTate commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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

titzer commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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.