Re: [RFC][PATCH] x86: don't destroy %rbp on kernel-mode faults

[Vegard Nossum]

On Mon, May 19, 2008 at 11:16 PM, Andi Kleen <andi@xxxxxxxxxxxxxx> wrote:
> Vegard Nossum wrote:
>> Hi,
>>
>> The RFC part of this patch is: Does anybody see why touching %rcx would
>> be bad? It certainly looks like %ecx is free. This fixes the stacktrace
>> problem I was seeing, and Pekka tested a bootup to userspace. (Pekka also
>> did half of the debugging. When will git allow multiple authors for a
>> patch? :-))
>
> The patch is ok, but I'm sure there's lots of other assembler code that
> destroys %rbp when it was saved elsewhere.

Thanks, The real intention of this code (you might have guessed it)
was to fix kmemcheck on 64-bit, and it did, so I'm happy. If we (or
others) hit another similar case, I'm sure we'll be able to fix those
too.

The problem seems to be that %rbp was never restored before it was
used again, and that's what I consider the real error in this case. I
changed it to use a different register for the temporary computation,
but restoring %rbp from wherever it was stored would also have been a
valid, albeit less efficient, solution.

> When I wrote all the assembler the assumption was always that a real
> unwinder would be used for backtraces, not frame pointer.

Hm, I am not sure exactly what a "real unwinder" would be. But I do
think it's fair to say that it is the assembly code in this case that
is violating the binary interface, and not the stack tracer code.


Vegard

-- 
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
	-- E. W. Dijkstra, EWD1036
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/