This repository has been archived by the owner on Mar 15, 2022. It is now read-only.
EH RI #897
Merged
EH RI #897
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
This flag allows compiling in a bring-up mode where EH clauses aren't reported to the runtime.
This is a temporary measure during EH bring-up; methods with landing pads will have their IR shown (if IR dumps are enabled), but then will be discarded so we'll fall back to the other jit rather than choking on the landing pads downstream.
Expand the FlowGraphInfoMap to record the innermost containing EHRegion for each BasicBlock. Remove the explicit Region parameter to makeFlowGraphNode since it is now assumed to be the CurrentRegion. Remove the overload of fgNodeGetRegion that takes a Function (and rewrite its only callsite to get the tree directly).
Allow recording catch class tokens on region objects. Use a union to overlay data used only for finally regions from data used only for catch regions.
Group pointers for better packing.
In preparation for making their logic a little more complex.
This will facilitate consulting the FlowGraphNodeInfoMap to identify handlers for nominal edges.
Add a hook, fgEnterRegion, used during first-pass flow-graph construction to perform any necessary client actions for setting up regions. Implement fgEnterRegion in GenIR to create a landing pad for each protected region. Clauses are added for catch handlers in the landing pad; other handler types are stubbed out. Update FlowGraphSuccessorEdgeList to report a nominal edge from any handler-protected block to its handler block (even if no throwing instructions have been inserted into the source block yet); this conservative model is what the base Reader class is expecting in terms of EH flow (and in particular, this ensures that handlers are visited when determining reachable blocks to decide which blocks to run pass 2 over). Remove setupBlockForEH now that its functionality has been incorporated into fgEnterRegion. Update leave processing to generate a call to llvm.eh.endcatch whenever a catch handler is exited. Closes #66
Calls in protected regions that may throw must use invoke instructions annotated with the appropriate unwind label. Invoke is a terminator, so it splits its block; update some code in genConditionalPointBlock to allow that there may be multiple point blocks once the invoke has been inserted.
Implement reader support for catch handlers and may-throw calls
Conflicts: lib/Jit/LLILCJit.cpp
Conflicts: lib/Reader/reader.cpp
The argument is the entire struct defined by the LandingPad, not the extracted exception pointer field of it.
As currently written, the exception dispatch from the inner landingpad would jump straight to the outer landingpad if the exception type doesn't match. This is incorrect; that path out of the inner dispatch should jump to a common point that the outer landingpad also jumps to (only one `landingpad` instruction [the inner one] should be executed in these cases). Just punt on these cases since the dispatch code will need to look very different once the new representation lands.
- Change the name of the personality function to the correct one for CoreCLR. - Pass the catch class token value directly as the clause argument on the landingpad instruction.
Add a jit layer before the ObjectLinkingLayer to inspect the object and: - Call reserveUnwindInfo for each funclet (before calling allocMem) - Call allocUnwindInfo separately for each funclet Also, add code to: - Call setEHcount and setEHinfo to report protected regions and their handlers The individual functions' contributes in .xdata are determined by parsing the unwind info headers in the .xdata and markers placed between the functions' contributions by the CoreCLR EH table generation in LLVM.
The handler types supported in the reader are now also supported in lowering/emission.
Emit catch clauses
Conflicts: include/Jit/LLILCJit.h lib/Jit/EEMemoryManager.cpp lib/Jit/LLILCJit.cpp
This layer just needs to call the memory manager's reserveUnwindSpace method on each object as it passes through; use the orc::ObjectTransformLayer utility rather than a custom object layer.
Use ObjectTransformLayer to reserve unwind space
Conflicts: lib/Jit/LLILCJit.cpp
This will be necessary when we outline filter functions.
Filters are outlined during initial IR generation (which includes explicitly escaping any locals referenced in filters), to simplify modeling their effects in the IR (invoking the outlined function must be included as a possible side-effect of any invoke that may throw an exception, since this must bottom out in a call to the throw helper [which is external code] and the filter function has external linkage). The ReaderBase class is unaware of the outlining. GenIR generates the filter code inline in the main function (except for an initial point block) during its main passes, and then moves filter blocks to the correct function in the readerPostVisit pass that this change adds between reading MSIL and removing unreachable code. The CurrentRegion tracked by the reader is used by GenIR to check whether code currently being generated is destined for an outlined filter or not, in the places where that is needed. References to parameters and locals check if the use is in a filter and insert escape code in the parent function and recover code in the filter as necessary. Any invoke in a filter-protected region needs a nominal edge to the filter as well as to its landingpad (the landingpad doesn't have explicit flow to the filter); this is achieved by having FlowGraphSuccessorEdgeList track down and iterate through the entry blocks of any filters referenced in the landingpad. There's also some juggling in fgNodeGetNext to ensure that the point blocks for filter entries are visited in their correct place in MSIL order. This change renames GenIR's Function field to RootFunction as reminder that some methods will have multiple functions. A few places are updated to iterate through all functions in the module where previously they were just operating on the one Function, including marking functions with unmanaged call frames and removing unreachable code.
Add support for EH filters
Skip relocation for personality routine in xdata
The object gets a function symbol for each funclet, so we need to sum their sizes rather than trying to report each in turn.
Update debug info emission to handle funclets
When insertConditionalPointBlock inserts a point block that has been split, it needs to find the ultimate tail point block, not keep checking the head point block's terminator repeatedly.
The helper call may be an invoke, so the predecessor of the PHI block may not be the block holding the call, but rather it's invoke's normal successor.
Make sure that CurrentInstrOffset and CurrentRegion have reasonable values when inserting prolog code after parsing all instructions (previously they were inadvertently getting the end-of-function values).
Conflicts: lib/Reader/readerir.cpp
Conflicts: lib/GcInfo/GcInfo.cpp
This will make it easier to triage failures due to unsupported EH behavior. Also add a setting that overrides this behavior to facilitate EH testing.
Insert failfast calls at exception handler entries
|
The individual changes were all PRed when they went into the EH branch. @AndyAyersMS PTAL. |
|
Forgot to mention:
|
|
Can you please add an exclusion for b28158 under the |
This test times out; opened #898 to investigate.
Sure; updated. |
|
I'm quite happy to see this merge. Looked through the code a bit but not in detail. Am willing to trust that suitable rigor was applied along the way. Do you think there is value in preserving branch history here, or should you rebase and squash? |
|
I'd prefer to have the history as-is;
For Microsoft:LLVM, on the other hand, while the EH and MS branches now have identical sources, I considered doing a merge there just to keep the history from the EH branch, but after looking at the specific commits that would preserve, decided there wasn't anything in there worth preserving. |
|
I don't feel strongly either way, just wanted your perspective. LGTM. |
| @@ -103,6 +104,7 @@ def main(argv): | |||
| parser.add_argument('-n', '--ngen', help='use ngened mscorlib', default=False, action="store_true") | |||
| parser.add_argument('-p', '--precise-gc', help='test with precise gc', default=False, action="store_true") | |||
| parser.add_argument('-v', '--verbose', help='echo commands', default=False, action="store_true") | |||
| parser.add_argument('-e', '--eh', help='enable exception handlers to run (as opposed to failfast)', default=False, action="store_true") | |||
There was a problem hiding this comment.
Shouldn't the llilc_runtest.py have the --eh option as well?
There was a problem hiding this comment.
Oops. Yeah, good catch, thanks. Will PR an update.
This was referenced Nov 4, 2015
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Merge the EH branch back into master.
At a high level, the EH status is still "code with EH constructs compiles cleanly, but attempts to catch exceptions may not work correctly at run-time". The differences are: