Merge master containing exception stacks into kp/partr #29791
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The logic for this was repeated in three quite different ways, in JL_CATCH, interpreter and codegen but putting it into the runtime in jl_eh_restore_state should be sufficient. Also move jl_eh_restore_state out of main header - seems unnecessary to expose such implementation detail, and inconsistent with other eh functions.
* A new lowered expression head :pop_exc is introduced and emitted in any location where a catch block exits normally (either by stepping out, or using return, break, goto). The semantics of :pop_exc are to pop the exception stack back to the state of the associated enter. * Make Expr(:enter) return a token which may be consumed by :pop_exc, thereby allowing the interpreter and codegen to know which :enter state should be used to pop the exception stack. I tried various alternatives for this association, but this was by far the nicest in terms of non-disruptive integration into the SSAIR processing code, and supporting both the interpreter and codegen.
Runtime ------- * An exception stack type, `jl_exc_stack_t` and associated manipulation functions has been added. Conceptually this stores a stack of (exception,backtrace) pairs. It's stored in a contiguous buffer so we can avoid reallocating when throwing and catching the same exception or set of exceptions repeatedly. Space for the exception and backtrace is allocated on demand inside `throw_internal`, after changing GC mode. Several variations were tried for allocating this sgorage, including allocating up front with malloc on the thread local state and copying during task switching. Keeping everything on the task seemed the simplest as it involves the least copying and keeps track of the exception stack buffers in a unified way. * The exception in transit is no longer a single pointer in the thread local storage. It's stored in `ptls->current_task->exc_stack` instead, along with associated backtrace. * `jl_current_exception()` is now the method to retreive the current exception from within a `JL_CATCH` dynamic context. * Several places where manual restoration of the exception and backtrace was done are no longer necessary because the stack system handles these automatically. This code is removed, including `jl_apply_with_saved_exception_state`. * `jl_eh_restore_state` has become non-inline. It seemed good to get this out of the public header. * `jl_sig_throw` is now used with `jl_throw_in_ctx` from signal handlers, to make the special circumstances clear and to avoid conflation with rethrow which now simply rethrows the existing exception stack. * Make rethrow outside a catch block into an error. * Use `ptls->previous_exception` to support `jl_exception_occurred` in embedding API. * finally lowering no longer includes a `foreigncall`, so expressions using finally can now be interpreted. Interpreter / Codegen --------------------- Mostly small changes here, to track the `:enter` and `:pop_exc` association with the SSAValue token. The token SSAValue slot is ideal here for storing the state of the exception stack at the `:enter`. GC -- Integrate exception and raw backtrace scanning as a special case into GC mark loop. This is necessary now that Task objects can refer to exception and backtrace data in raw form.
Add missing GC static analyzer annotations to exception stack functions and fix two problems found by the GC static analyser: * An early return with missing GC_POP * Missing root for the exception across a safe point in throw_internal
gcc-5 appears to miscompile jl_type_infer under special circumstances, leading to segfaults but gcc-7 and clang do fine on this. The "special circumstances" are that assertions are enabled when preprocessing jl_svecref for use within jl_type_infer specifically. There are various ways to remove the segfaults and force a correct compilation: * The given rearrangement * Replacing jl_svecref in the last part of jl_type_infer with an equivalent function, but without the assertions. * Removing the JL_TRY / JL_CATCH further up. It's suspicious that the error only occurs when both the double-return __sigsetjmp() and the no-return __assert_fail() are present inside the function in sufficient quantities. Perhaps the compiler is failing to analyze the interaction between these.
* Use functions rather than macros for exception stack access * Use standard type check macro * More clearly document gc rooting of `jl_current_exception()`
Better be explicit here for something that's not commonly encountered.
* Test try-catch-finally form * Add a few tests to do with exiting try blocks. * More explanatory comments
cd src ag -l exc_stack | xargs sed -i -e 's/exc_stack/excstack/g' ag -l excstk_raw | xargs sed -i -e 's/excstk_raw/excstack_raw/g' sed -i -e 's/jlcurrent_exception_func/jl_current_exception_func/g' codegen.cpp cd ../base ag -l exc_stack | xargs sed -i -e 's/exc_stack/excstack/g'
fix two punctuation errors
A couple more testsets for operators
The information on how to run doctests in `doc/README.md` wasn't correct. I replaced it with what I found in `CONTRIBUTING.md`.
* Support repeat at any dimension * Use a more friendly way to do getindex * rollback to original commit * use `1` instead of `OneTo(1)` * Change 1 to OneTo(1) * Change `length` to `ndims`, add test * Fix a test error * trigger rebuild
Resolve some trivial conflicts and convert exception/task interaction in partr.c to use exception stack system.
|
After this merge the tests got most of the way locally, but eventually got stuck in channels. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As promised over at #28878, here's a resolution to the conflicts between latest master (after merging #28878) and the PARTR branch #22631.
There's not an awful lot of overlap between these changes, so I think I caught the cases which needed catching and the
exceptionstests work locally. Having said that, I'm not particularly familiar with the PARTR internals.@kpamnany @JeffBezanson