This is an archive of the discontinued LLVM Phabricator instance.

Fix typo in ArmUnwindInfo::GetUnwindPlan
ClosedPublic

Authored by teemperor on Apr 13 2019, 12:37 PM.

Details

Summary

As reported in LLVM bug 41486, the check (byte1 & 0xf8) == 0xc0 is wrong. We want to check for 11010nnn,
so the proper value we want to compare against is 0xd0 (0xc0 would check for the value 11000nnn which we
already checked for above as described in the bug report).

Diff Detail

Event Timeline

teemperor created this revision.Apr 13 2019, 12:37 PM

Don't have the test for this and probably can't write one for it (as I don't have access to an ARM device for testing and don't really know the code so well). If anyone feels like providing a test, feel free to link it to this revision.

jasonmolenda accepted this revision.Apr 15 2019, 2:34 PM
jasonmolenda added a subscriber: jasonmolenda.

Agreed, this change is correct. I double checked this in the http://infocenter.arm.com/help/topic/com.arm.doc.ihi0038b/IHI0038B_ehabi.pdf doc (mod 24 Nov 2015) and the only difference in Section 9.3 "Frame unwinding instructions" Table 4 and this code is in the comment - they now refer to this as 'Pop VFP double-precision registers D[8]-D[8+nnn] saved (as if) by VPUSH (see remark d)'. But there's no benefit in updating this one comment while the others are using the older comments (this code was originally written against the 30 Nov 2012 version of the doc).

We don't have any tests for the ARM.exidx exception handling information today, it would be take some work to add that. It's a format like darwin's compact unwind or less correctly, eh_frame, only defined for 32-bit arm. I would commit this obvious fix without a test.

This revision is now accepted and ready to land.Apr 15 2019, 2:34 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 16 2019, 1:08 AM
teemperor edited the summary of this revision. (Show Details)Apr 16 2019, 1:09 AM