[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Crash on Linux+Clang (NSTimer)
|
From: |
David Chisnall |
|
Subject: |
Re: Crash on Linux+Clang (NSTimer) |
|
Date: |
Wed, 11 Nov 2020 09:19:25 +0000 |
|
User-agent: |
Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.4.1 |
On 10/11/2020 23:33, Riccardo Mottola wrote:
Hello David,
thank you for the thorough analysis.
If this is the case, you'll be able to see by looking at the value of
the %rsp register and the current instruction. Please can you:
- Use `show registers rsp` to let me know your current stack pointer
value.
- Use `disas` and show me the instruction that it points to
I don't have that registerm it is for x86-64, right? esp should be the
equivalent IA-32, no?
Yes, sorry, I missed that this was 32-bit.
here my dump:
(gdb) info registers
eax 0xb7afa5e4 -1213225500
ecx 0xb7850510 -1216019184
edx 0x2 2
ebx 0xb7ad1000 -1213394944
esp 0xbfffeb84 0xbfffeb84
ebp 0x0 0x0
esi 0x834bde4 137674212
edi 0x833ef24 137621284
eip 0xb7850558 0xb7850558 <-[NSTimer
initWithFireDate:interval:target:selector:userInfo:repeats:]+72>
eflags 0x210246 [ PF ZF IF RF ID ]
cs 0x73 115
ss 0x7b 123
ds 0x7b 123
es 0x7b 123
fs 0x0 0
gs 0x33 51
The SysV x86 ABI is a bit weird with stack alignment (for both 32- and
64-bit), it mandates that the alignment is 16 bytes *before* a call
instruction. The all then subtracts one pointer from the stack. This
means that, on a 32-bit platform, on function entry (meaning on the
first instruction - often the function entry breakpoint is after the
prelude, which spills ebp and a few other things) $esp + 4 % 16 == 0
disas:
Dump of assembler code for function -[NSTimer
initWithFireDate:interval:target:selector:userInfo:repeats:]:
0xb7850510 <+0>: push %ebp
0xb7850511 <+1>: push %ebx
0xb7850512 <+2>: push %edi
0xb7850513 <+3>: push %esi
0xb7850514 <+4>: sub $0x1c,%esp
0xb7850517 <+7>: call 0xb785051c <-[NSTimer
initWithFireDate:interval:target:selector:userInfo:repeats:]+12>
0xb785051c <+12>: pop %ebx
0xb785051d <+13>: movsd 0x3c(%esp),%xmm0
0xb7850523 <+19>: mov 0x38(%esp),%ebp
0xb7850527 <+23>: mov 0x44(%esp),%edi
0xb785052b <+27>: mov 0x30(%esp),%esi
0xb785052f <+31>: xorpd %xmm1,%xmm1
0xb7850533 <+35>: add $0x280ae4,%ebx
0xb7850539 <+41>: movsd -0x10d648(%ebx),%xmm3
0xb7850541 <+49>: movapd %xmm0,%xmm2
0xb7850545 <+53>: test %ebp,%ebp
0xb7850547 <+55>: cmpnlesd %xmm1,%xmm2
0xb785054c <+60>: andpd %xmm2,%xmm0
0xb7850550 <+64>: andnpd %xmm3,%xmm2
0xb7850554 <+68>: orpd %xmm0,%xmm2
Unfortunately, this is not the dump at the point where it crashed. The
faulting instruction is the one immediately after this. If you run
disas at the point where it breaks, you will get a => indicator next to
the faulting instruction.
For extra confirmation, put an breakpoint on the first instruction of
`NSRunLoop`'s `+initialize` and let me know what the value of %rsp is
on function entry?
That would be breaking:
Breakpoint 1, +[NSRunLoop initialize] (
self=0xb7af2944 <._OBJC_CLASS_NSRunLoop>, _cmd=0x8085db0)
at NSRunLoop.m:746
746 if (self == [NSRunLoop class])
(gdb) info registers esp
esp 0xbfffec44 0xbfffec44
You need to put it on the first instruction. The gdb syntax for this is
`b *0x{whatever the address of the function is`.
Looking at the code, I think we are spilling 24 8-byte words to the
stack, but the weirdness related to the x86 call instruction means
that we will be doing the wrong thing for the ABI.
Before any fix, libobjc2 test suite:
76% tests passed, 44 tests failed out of 186
Notably:
125 - objc_msgSend (Child aborted)
126 - objc_msgSend_optimised (Child aborted)
127 - objc_msgSend_legacy (Child aborted)
128 - objc_msgSend_legacy_optimised (Child aborted)
That's interesting. We run those tests in CI, I wonder what is
different on your platform.
If you want to try a fix, I believe changing the 0x98 to 0xa0 on these
two lines should work:
https://github.com/gnustep/libobjc2/blob/41808111aa0a58708daf66b2322940d4353e37d8/objc_msgSend.x86-64.S#L216
https://github.com/gnustep/libobjc2/blob/41808111aa0a58708daf66b2322940d4353e37d8/objc_msgSend.x86-64.S#L257
I believe doing that should cause the objc_msgSend test in the
runtime's test suite to fail, but then changing the 0xD8 to 0xE0 on
this line should fix it:
https://github.com/gnustep/libobjc2/blob/41808111aa0a58708daf66b2322940d4353e37d8/objc_msgSend.x86-64.S#L241
If your theory about alignment still applies, maybe you can hint me the
32bit version of it?
The 32bit method in slowLookUp is much shorter :)
Yup, in the 32-bit ABI all arguments are stored on the stack so we don't
need to store any of them in the trampoline.
On function entry we'll have an esp that is 4-bytes off 16-byte alignment.
5: # slowSend:
mov \sel(%esp), %ecx
lea \receiver(%esp), %eax
push %ecx # _cmd
push %eax # &self
.cfi_def_cfa_offset 12
We then do two pushes, taking it to 12-byte.
call CDECL(slowMsgLookup)@PLT
But here the ABI requires it to be 16 byte aligned, so that looks wrong.
add $8, %esp # restore the stack
I see "12" delcared here as frame offset
This one is slightly more complicated because we're passing the address
of `self` in the argument frame into slowMsgLookup, so our stack looks
something like this:
{ other args }
_cmd
self
return address from objc_msgSend
_cmd
&self (the stored version a little way up the stack)
To fix this, we need to add 4 bytes of padding before the second copy of
_cmd.
I think the simplest way to do that is duplicate the push %ecx. We then
also need to fix the CFI directive and the add that does the stack
restore to compensate for the extra 4 bytes:
```
mov \sel(%esp), %ecx
lea \receiver(%esp), %eax
push %ecx # Unused, stack alignment
push %ecx # _cmd
push %eax # &self
.cfi_def_cfa_offset 16
call CDECL(slowMsgLookup)@PLT
add $12, %esp # restore the stack
```
Does that fix it for you? If so, please can you raise a PR with that
change in it?
Thanks,
David