Skip to content

[EH] Use _UA_SEARCH_PHASE in personality function (NFC) #17991

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 5, 2022
Merged

[EH] Use _UA_SEARCH_PHASE in personality function (NFC) #17991

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3ed63fe
[EH] Use _UA_SEARCH_PHASE in personality function (NFC)
aheejin Oct 5, 2022
1832b60
comment
aheejin Oct 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 7 additions & 10 deletions system/lib/libcxxabi/src/cxa_personality.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,8 @@ static void scan_eh_tab(scan_results &results, _Unwind_Action actions,
{
// Native exception caught by exception
// specification.
assert(actions & _UA_SEARCH_PHASE);
results.ttypeIndex = ttypeIndex;
Comment on lines +801 to +802
Copy link
Member Author

@aheejin aheejin Oct 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This is not related to this PR, but I compared our cxa_personality.cpp with llvm 14.0.0 version (on which our current libc++abi is based on), this part is missing. I guess it was dropped when we upgraded the library. I don't think Wasm is using this part of the code though, reducing difference from the upstream code seems better.

results.actionRecord = actionRecord;
results.adjustedPtr = adjustedPtr;
results.reason = _URC_HANDLER_FOUND;
Expand Down Expand Up @@ -964,6 +966,11 @@ __gxx_personality_v0
exc->languageSpecificData = results.languageSpecificData;
exc->catchTemp = reinterpret_cast<void*>(results.landingPad);
exc->adjustedPtr = results.adjustedPtr;
#ifdef __USING_WASM_EXCEPTIONS__
// Wasm only uses a single phase (_UA_SEARCH_PHASE), so save the
// results here.
set_registers(unwind_exception, context, results);
#endif
Comment on lines +969 to +973
Copy link
Member Author

Choose a reason for hiding this comment

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

This is to save our selector computation result.

In the upstream version, the search phase searches for a matching catch handler and caches the result, and the second cleanup phase retrieves the result and sets registers with the final selector value. (In Wasm's libunwind, they are not registers but just a field in a struct.). They do it in the second cleanup phase:

emscripten/system/lib/libcxxabi/src/cxa_personality.cpp

Line 973 in 40fb7d2

set_registers(unwind_exception, context, results);

But now we only use the search phase, we do it here. Instead, we are able to remove our duplicated caching code in line 984-993 below.

}
return _URC_HANDLER_FOUND;
}
Expand All @@ -981,16 +988,6 @@ __gxx_personality_v0
exception_header->catchTemp = 0;
#endif
}
#ifdef __USING_WASM_EXCEPTIONS__
// Wasm uses only one phase in _UA_CLEANUP_PHASE, so we should set
// these here.
__cxa_exception* exception_header = (__cxa_exception*)(unwind_exception+1) - 1;
exception_header->handlerSwitchValue = static_cast<int>(results.ttypeIndex);
exception_header->actionRecord = results.actionRecord;
exception_header->languageSpecificData = results.languageSpecificData;
exception_header->catchTemp = reinterpret_cast<void*>(results.landingPad);
exception_header->adjustedPtr = results.adjustedPtr;
#endif
Comment on lines -984 to -993
Copy link
Member Author

Choose a reason for hiding this comment

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

This is what the upstream code does in the first search phase, but we used only the cleanup phase, we duplicated the code here. Now we can remove it.

return _URC_INSTALL_CONTEXT;
}

Expand Down
2 changes: 1 addition & 1 deletion system/lib/libunwind/src/Unwind-wasm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ _Unwind_Reason_Code _Unwind_CallPersonality(void *exception_ptr) {
// Call personality function. Wasm does not have two-phase unwinding, so we
// only do the cleanup phase.
_Unwind_Reason_Code ret = __gxx_personality_wasm0(
1, _UA_CLEANUP_PHASE, exception_object->exception_class, exception_object,
1, _UA_SEARCH_PHASE, exception_object->exception_class, exception_object,
(struct _Unwind_Context *)&__wasm_lpad_context);
return ret;
}
Expand Down