Page MenuHomePhabricator

[llvm-readobj] [ARMWinEH] Fix printing of exception handlers with packed epilogues
ClosedPublic

Authored by mstorsjo on Sep 9 2020, 4:39 AM.

Details

Summary

If there's a packed epilogue (indicated by the flag E), the EpilogueCount() field actually should be interpreted as EpilogueOffset.

Diff Detail

Event Timeline

mstorsjo created this revision.Sep 9 2020, 4:39 AM
mstorsjo requested review of this revision.Sep 9 2020, 4:39 AM
mstorsjo retitled this revision from [llvm-readobj] [ARMWinEH] Fix printing of exception handlers with packed epiloguus to [llvm-readobj] [ARMWinEH] Fix printing of exception handlers with packed epilogues.Sep 9 2020, 5:21 AM
efriedma accepted this revision.Sep 9 2020, 1:35 PM

LGTM

This revision is now accepted and ready to land.Sep 9 2020, 1:35 PM
MaskRay added a subscriber: grimar.Sep 9 2020, 2:07 PM
MaskRay added inline comments.
llvm/test/tools/llvm-readobj/COFF/arm64-packed-epilog.s
3

If the format is important:

# RUN: llvm-mc -filetype=obj -triple aarch64-windows %s -o - |
# RUN:   llvm-readobj --unwind - | FileCheck %s --match-full-lines --strict-whitespace

#      CHECK:ExceptionData {
# CHECK-NEXT:  FunctionLength: 4

@grimar may prefer keeping the temporary file as %t

grimar added inline comments.Sep 10 2020, 1:06 AM
llvm/test/tools/llvm-readobj/COFF/arm64-packed-epilog.s
3

Yes, I find it is much more convenient for debugging broken tests to have them.
It helps to save time - instead of rewriting (and then reverting) such constructions you can just debug an existent temp file.

mstorsjo added inline comments.Sep 10 2020, 1:13 AM
llvm/test/tools/llvm-readobj/COFF/arm64-packed-epilog.s
3

Sure, I can change that.

A number of the existing related testcases also use the temp-less form, but I can change to using temp files for the newly added tests at least, and cases where I otherwise touch the RUN lines.

grimar added inline comments.Sep 10 2020, 1:16 AM
llvm/test/tools/llvm-readobj/COFF/arm64-packed-epilog.s
3

Sounds good, thanks!