This is an archive of the discontinued LLVM Phabricator instance.

[ARM64-BE] Fix byte order of CIE and FDE frames for exception handling
ClosedPublic

Authored by cpirker on May 13 2014, 3:32 AM.

Details

Reviewers
jmolloy
Summary

Hi all,

The .eh_frame section is a sequence of records.
Each record is either a CIE (Common Information Entry) or an FDE (Frame Description Entry).
Each of these records contain length and offset fields, that need to be stored in proper byte order.
ARM big endian mode requires the .eh_frame section in bit endian order, although instructions are always in le order.

This patch stores fixups in .eh_frame section in bit endian order (for be mode).

Please review.

Thanks,
Christian

Diff Detail

Event Timeline

cpirker updated this revision to Diff 9341.May 13 2014, 3:32 AM
cpirker retitled this revision from to [ARM64-BE] Fix byte order of CIE and FDE frames for exception handling.
cpirker updated this object.
cpirker edited the test plan for this revision. (Show Details)
cpirker added subscribers: Unknown Object (MLST), Konrad, jmolloy.

Hi Christian,

This looks reasonable - I only have one comment. Can I assume there'll be an AArch32 patch coming to fix the same thing too?

Cheers,

James

lib/Target/ARM64/MCTargetDesc/ARM64AsmBackend.cpp
540 ↗(On Diff #9341)

This can be greatly simplified by using ByteSwap_32 from Support/MathExtras.h.

Hi James,

For the AArch32 you can find a similar path in D3704.

Thanks,
Christian

jmolloy added inline comments.May 13 2014, 3:55 AM
lib/Target/ARM64/MCTargetDesc/ARM64AsmBackend.cpp
536 ↗(On Diff #9341)

Another nit: please put the * next to the variable.

cpirker updated this revision to Diff 9394.May 14 2014, 9:43 AM

Hi James,

I updated as you requested.
Can you please review.

Thanks,
Christian

cpirker updated this revision to Diff 9395.May 14 2014, 9:47 AM

Hi James,

Please ignore "Diff 2", I used the wrong diff file.
Can you please review "Diff 3".

Thanks,
Christian

jmolloy accepted this revision.May 14 2014, 9:50 AM
jmolloy added a reviewer: jmolloy.

LGTM!

This revision is now accepted and ready to land.May 14 2014, 9:50 AM
cpirker closed this revision.May 14 2014, 10:00 AM

I committed this patch as rL208792.