This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Make sure we don't emit packed unwind for .seh_save_any_reg_p
ClosedPublic

Authored by efriedma on Nov 29 2022, 3:04 PM.

Details

Diff Detail

Event Timeline

efriedma created this revision.Nov 29 2022, 3:04 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 29 2022, 3:04 PM
efriedma requested review of this revision.Nov 29 2022, 3:04 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 29 2022, 3:04 PM
mstorsjo accepted this revision.Nov 29 2022, 11:06 PM

LGTM

In many similar switches, we've got a default: llvm_unreachable("Unsupported ARM64 unwind code"); - I wonder if we should have that here too, or at least default: return false;?

This revision is now accepted and ready to land.Nov 29 2022, 11:06 PM

Looking a bit more closely, it looks like the following opcodes don't show up in the switch:

UOP_AllocLarge
UOP_AddFP
UOP_TrapFrame
UOP_Context
UOP_ClearUnwoundToCall

Not sure if this actually leads to issues in practice, but I'll follow up to address.

Looking a bit more closely, it looks like the following opcodes don't show up in the switch:

UOP_AllocLarge
UOP_AddFP
UOP_TrapFrame
UOP_Context
UOP_ClearUnwoundToCall

Not sure if this actually leads to issues in practice, but I'll follow up to address.

Ok, so then we can't make it an unreachable unless we add those as well. In that case, I guess default: return false; would be simplest/safest?

This revision was landed with ongoing or failed builds.Nov 30 2022, 1:49 PM
This revision was automatically updated to reflect the committed changes.