Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid compact unwind info generated for a function without frame pointers on OSX #21174

Closed
ramosian-glider opened this issue Aug 29, 2014 · 8 comments
Assignees
Labels
backend:X86 bugzilla Issues migrated from bugzilla

Comments

@ramosian-glider
Copy link
Contributor

Bugzilla Link 20800
Resolution FIXED
Resolved on Sep 09, 2014 09:51
Version trunk
OS MacOS X
Attachments test case for which the incorrect debug info is generated
CC @isanbard,@kcc,@rotateright

Extended Description

The attached file contains the __asan_report_error function for which the compact unwind info appears to be incorrect if the frame pointers are omitted:

$ bin/clang++ -O3 -fomit-frame-pointer asan_report.ii -c
$ bin/llvm-objdump -unwind-info -d asan_report.o
...
__Z19__asan_report_errormmmm:
0: 55 pushq %rbp
1: 41 57 pushq %r15
3: 41 56 pushq %r14
5: 41 55 pushq %r13
7: 41 54 pushq %r12
9: 53 pushq %rbx
a: 48 81 ec 18 08 00 00 subq $2072, %rsp
...
Contents of __compact_unwind section:
Entry at offset 0x0:
start: 0x0 __Z19__asan_report_errormmmm
length: 0xda
compact encoding: 0x0309f800

According to /usr/include/mach-o/compact_unwind_encoding.h, the compact encoding uses the frameless stack index mode (0x03000000 is UNWIND_X86_64_MODE_STACK_IND), so the second byte of the compact encoding must be the offset of the immediate constant denoting the stack size in the subq instruction. This byte has value 0x9 instead of 0xc, which makes the unwinder think the function stack size is around 3Gb.

This test case contains a reduced version of __asan_report_error in the ASan runtime. I'm seeing actual crashes because of this bug when calling _Unwind_Backtrace from the real __asan_report_error on OSX. GDB also has problems unwinding through this function on OSX.
A number of other functions in the ASan runtime have the same incorrect compact unwind info.

@ramosian-glider
Copy link
Contributor Author

assigned to @ramosian-glider

@ramosian-glider
Copy link
Contributor Author

Looks like Target/X86/MCTargetDesc/X86AsmBackend.cpp assumes that the size of a push instruction is always constant on X86.
However it may vary: in the objdump above the push instruction sizes for RBP and RBX are equal to 1, while the sizes for R12-R15 are 2.
As a result, the offset is off by 4.

@ramosian-glider
Copy link
Contributor Author

@ramosian-glider
Copy link
Contributor Author

I also think it's reasonable to add a check to CompactUnwinder.hpp in libcxxabi so that it doesn't crash on unexpectedly big stack sizes. Nick, what do you think?

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 2, 2014

If we added a check to the unwinder, all it could do is assert() which is itself a form of crashing. We need to catch this way earlier.

When the compact unwind creation code lived in the linker, for UNWIND_MOD_STACK_IND it would check the actual bytes in the function instruction to make sure the stack size value needed was at the offset being computed. Is there a way to pass the function bytes down into generateCompactUnwindEncoding() so that check could be done?

@ramosian-glider
Copy link
Contributor Author

If we added a check to the unwinder, all it could do is assert() which is itself a form of crashing. We need to catch this way earlier.
We could just bail out from the unwinding procedure returning some reason code, but not crashing the whole program.

This can be just a stopgap measure that'll fix the situation for libcxxabi users until the bug is fixed in the system compiler.

@ramosian-glider
Copy link
Contributor Author

Fixed in r217020 and r217021

@rotateright
Copy link
Contributor

Hi Alexander -

Your switch doesn't include cases for r8 - r11 (which would be 2 byte push instructions). I assume that's because they are not callee saved registers? A comment in the code to make that obvious would be good (or just handle those cases anyway).

Is there no general-purpose way to determine an instruction's size at this level?

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 9, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 bugzilla Issues migrated from bugzilla
Projects
None yet
Development

No branches or pull requests

3 participants