Is this related to the general panic discussed in freebsd-current?

[Tim Kientzle]

On May 5, 2013, at 4:20 PM, Tim Kientzle wrote:
> 
> On May 5, 2013, at 3:37 PM, Andrew Turner wrote:
> 
>> On Sun, 5 May 2013 09:37:48 -0700
>> Tim Kientzle <tim at kientzle.com> wrote:
>>> On May 5, 2013, at 6:00 AM, Andrew Turner wrote:
>>> 
>>>> On Sat, 4 May 2013 15:44:37 -0700
>>>> Tim Kientzle <tim at kientzle.com> wrote:
>>>>> I'm baffled.  If I insert a printf into the loop in stack_capture,
>>>>> the kernel boots. But the generated assembly looks perfectly
>>>>> correct to me in either case.  So inserting the printf must have
>>>>> some side-effect.
>>>>> 
>>>>> The stack does end up aligned differently:  The failing version
>>>>> puts 16 bytes on the stack, the working version puts 24 bytes.
>>>>> But I can't figure out how that would explain what I'm seeing...
>>>> 
>>>> It feels like an alignment issue but those stack sizes should both
>>>> be valid. Are you able to send me the asm for the working and broken
>>>> versions of the function?
>>>> 
>>>> Also which ABI are you using? I have not been able to reproduce it
>>>> with EABI, but that may have been because I have a patched clang
>>>> I've been using to track down another issue.
>>> 
>>> I'm using whatever the default is in FreeBSD-CURRENT.  I've seen
>>> this consistently with both RaspberryPi and BeagleBone kernels
>>> for the last few weeks.
>> Ok, it's the old ABI. I note this function may be broken with EABI as
>> it make assumptions on the layout of each frame.
> 
> Thought so.
> 
>>> /* Broken version */
>>> c0519cec <stack_save>:
>>> void
>>> stack_save(struct stack *st)
>>> {
>>> c0519cec:       e92d4830        push    {r4, r5, fp, lr}
>> 
>> This stack layout is incorrect. It should store (from a low address to
>> high address) r4, r5, fp, ip, lr and pc.
> 
> If I understand right, you're claiming that Clang is generating
> a wrong preamble for OABI functions which is manifesting
> as crashes in the stack-walking code.
> 
> I'm not sure I understand the frame layout you're saying it
> should use, though.  Pushing PC seems a very strange thing
> to do on ARM.  (Though it would seem to match sys/arm/include/stack.h.)
> 
> It doesn't look like Clang/OABI is using the layout you suggest
> anywhere in the kernel code:  I grepped through the kernel
> disassembly and found only a single instance of "fp, ip, lr, pc"
> and that was from assembly.
> 
> It also looks like sys/arm/include/stack.h needs to be taught
> about the difference between EABI and OABI.
> 
>> The unwind code following is
>> incorrect for this stack layout.
> 
> Ah.  I'll take another look.  I hadn't tried to match up the offsets
> to see if they made sense for the stack layout.
> 
> I could probably change this stack-walking code to
> match the frame layout being used by Clang here,
> but I'm not sure whether that's the "right" fix.

Here's a version of stack_capture that allows a Clang-built
OABI kernel with WITNESS enabled to boot:

/* In sys/arm/arm/stack_machdep.c */
static void
stack_capture(struct stack *st, u_int32_t *frame)
{
        vm_offset_t callpc;

        stack_zero(st);
        while (INKERNEL(frame)) {
                callpc = frame[1];
                if (stack_put(st, callpc) == -1)
                        break;
                frame = (u_int32_t *)(frame[0]);
        }
}


From the above, it sounds like this should not be committed;
rather, we should fix Clang's OABI support to emit the right
frame layout.  I've not yet started to look through Clang to
try to figure out how to do that….  Any pointers?  ;-)

Tim