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

[NativeAOT-LLVM] Implement two-pass exception handling #2284

Merged
merged 6 commits into from
May 23, 2023

Conversation

SingleAccretion
Copy link

@SingleAccretion SingleAccretion commented May 20, 2023

Implements the two-pass system which #2145 laid the groundwork for.

The general idea is that we use native exception handling for the first pass, caching second-pass handlers in a linked list, to be invoked in the second pass. With this, our EH semantics should align fully with .NET proper modulo bugs.

One area where a two-pass scheme like this causes problems is dynamic stack allocation. We can no longer use the native stack in cases where the underlying data may escape into handlers, and so much of this change is focused on adding the implementation and tests for another kind of "stack" - "the dynamic stack", a specialized allocator, used only for locallocs in methods with EH. This allocator tracks the shadow stack to know how much state to release and is invoked by codegen on method return (handling normal flow) and by the dispatchers at the very end of the second pass (handling exceptional flow).

Contributes to #2169.

@SingleAccretion SingleAccretion force-pushed the EH-Two-Pass branch 3 times, most recently from 353b548 to b501ae4 Compare May 21, 2023 11:37
@SingleAccretion SingleAccretion marked this pull request as ready for review May 21, 2023 12:48
@SingleAccretion
Copy link
Author

@dotnet/nativeaot-llvm

@yowl
Copy link
Contributor

yowl commented May 22, 2023

We can no longer use the native stack in cases where the underlying data may escape into handlers, and so much of this change is focused on adding the implementation and tests for another kind of "stack" - "the dynamic stack"

I understand why we can't use the native stack for stackalloc but I'm not clear on why a stackalloc variable cannot be accessed from an EH the same as any other variable on the shadow stack that escapes to a EH?

@SingleAccretion
Copy link
Author

SingleAccretion commented May 22, 2023

It is a very good question - why not use the shadow stack for such stackallocs? This is certainly a tradeoff, the considerations were:

  1. Pro: the allocation path is fast. Faster than native in fact, as we don't do stack probing (and if/when we start to, it'll be relatively cheap).
  2. Pro: no need to write a specialized allocator; more robust.
  3. Con: this introduces dynamic shadow stack allocation. Dynamic stack allocation in general is a qualitative leap in complexity (see, e. g., #85548) and may make implementing some future features more difficult and/or make them more expensive. One such feature I had in mind is precise GC. More immediately, this would complicate the compiler.
  4. Con: large native allocations on the shadow stack that are actually "not needed" (realized escape into a handler should be very rare, after all) slow down GC scanning.
  5. Con: Having an efficient stack allocator like may be interesting for other purposes (like, say, allocating FaultNodes in the dispatch path...). This was a minor point.

I also considered allocating a pinned managed array, this would have been the simplest and most robust solution of all, but also the least performant (so I rejected it).

Copy link
Contributor

@yowl yowl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jkotas jkotas merged commit 408c069 into dotnet:feature/NativeAOT-LLVM May 23, 2023
@SingleAccretion SingleAccretion deleted the EH-Two-Pass branch May 24, 2023 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants