This is an archive of the discontinued LLVM Phabricator instance.

Fix x86 return pattern detection
ClosedPublic

Authored by mortimer on Feb 7 2019, 2:40 PM.

Details

Summary

Replace 0xc9 (LEAVE) with 0xcb (RETF) in ret_pattern_p(). Also put 0xc3 first, since it is the most common form and will match first.

Diff Detail

Event Timeline

mortimer created this revision.Feb 7 2019, 2:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 7 2019, 2:40 PM
labath added a subscriber: labath.Feb 8 2019, 1:41 AM

Could you please write a test for this? You can take a look at unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp for other x86AssemblyInspectionEngine tests.

Also, I'm curious how you found this bug. (i.e. which functionality was broken with the old implementation).

For sure I can add a test.

I found this only because I was looking to fix something else (prologue detection on OpenBSD with -fret-protector) and noticed that the return pattern detect function was wrong. So nothing was broken that I found, though the inclusion of 0xc9 as a return pattern could conceivably cause problems under some circumstances. In any event, I figured it would be easy enough to submit a fix, since it’s a tiny change.

mortimer updated this revision to Diff 186144.Feb 9 2019, 9:26 PM

Added a test to verify that the unwind state is reset after each kind of return.

jasonmolenda accepted this revision.Feb 10 2019, 12:01 AM

Thanks for fixing this and adding a testcase. I don't know why 0xc9 leave was here; it's handled over in x86AssemblyInspectionEngine::leave_pattern_p. Do you have commit access?

This revision is now accepted and ready to land.Feb 10 2019, 12:01 AM

Happy to help out. :-) I do not have commit access.

This revision was automatically updated to reflect the committed changes.

Thanks for the fix and the test.