Add proper CFI and other inline assembly cleanups #38
Conversation
| # Call the provided function. | ||
| call *8(%esp) | ||
| call *%ebp |
There was a problem hiding this comment.
This makes the data flow even more complicated with no benefit that I can see.
|
@Amanieu Also, looks like you broke OS X. This is likely related to the DWARF encoding, I suggest looking with |
Amanieu
force-pushed
the
cfi
branch
5 times, most recently
from
4993612
to
2416111
Compare
|
I've addressed all the comments. I tested both x86 and x86_64 on Linux, but I haven't been able to test on OS X apart from the tests run by travis. |
Amanieu
force-pushed
the
cfi
branch
3 times, most recently
from
5bc3dcf
to
662872b
Compare
| @@ -98,7 +140,8 @@ pub unsafe fn init(stack: &Stack, f: unsafe extern "C" fn(usize) -> !) -> StackP | |||
| let mut sp = StackPointer(stack.base() as *mut usize); | |||
| push(&mut sp, 0xdead0cfa); // CFA slot | |||
| push(&mut sp, f as usize); // function | |||
| push(&mut sp, trampoline_1 as usize); | |||
| push(&mut sp, trampoline_1 as usize + 2); // skip the 2 nops | |||
There was a problem hiding this comment.
Please add a comment here explaining that we set up the stack as-if the first trampoline has already execited.
Amanieu
force-pushed
the
cfi
branch
2 times, most recently
from
04358e1
to
eface94
Compare
| # never actually executed. | ||
| nop | ||
|
|
||
| # Stack unwinding in OSX doesn't seem to like 1-byte symbols, so we add |
There was a problem hiding this comment.
s/OSX/libunwind/, since I believe this will affect anyone using libunwind (like ARTIQ)
|
LGTM pending inline comments and or1k port. |
|
I've fixed the comments. I think the or1k port is best left to a separate PR for now. |
|
@Amanieu is there a problem with it? |
|
Not really, it's just that it's hard to make sure I don't accidentally break it if I have no way of testing it. |
|
Just tell me once you have something and I'll test (and fix if something is broken). |
Amanieu
force-pushed
the
cfi
branch
2 times, most recently
from
ad7e03a
to
deb303d
Compare
Amanieu
force-pushed
the
cfi
branch
3 times, most recently
from
0cd3dbc
to
0033a1a
Compare
Amanieu
force-pushed
the
cfi
branch
4 times, most recently
from
4e95ad7
to
705f931
Compare
|
I reworked the trampoline code so that it is compatible with frame pointer-based unwinding. I personally feel that this approach is superior to #44 since it preserves the guarantee that a backtrace can be correctly generated at all instruction boundaries. |
Amanieu
force-pushed
the
cfi
branch
3 times, most recently
from
33ccfa6
to
ea65404
Compare
|
This is now covered by #49. |
|
I'm reopening this since it seems that the approach in #44 is broken. You actually do need 2 trampolines. |
Amanieu
force-pushed
the
cfi
branch
3 times, most recently
from
7906fb4
to
addda97
Compare
This ensures that gdb is able to generate a correct backtrace when stopped at any instruction in the inline asm. It also makes backtraces accessible to tools that only track frame pointer chains, like perf or dtrace.
The most significant change is that all of the inline asm for x86 and x86_64 now has proper CFI annotations, which allows gdb to produce a correct backtrace even if the program is interrupted in the middle of a swap operations.
I didn't do this for the OR1K code because I am not familiar with the instruction set and don't have any way of testing the resulting code.
I've also added a few cleanups for the inline asm clobbers.