Opened 14 years ago
Closed 14 years ago
#6139 closed enhancement (fixed)
[PATCH] ppc: Implement arch_debug_get_caller
Reported by: | andreasf | Owned by: | axeld |
---|---|---|---|
Priority: | normal | Milestone: | R1 |
Component: | System/Kernel | Version: | R1/Development |
Keywords: | Cc: | ||
Blocked By: | Blocking: | ||
Platform: | PowerPC |
Description
Some low hanging fruit: Resolve a TODO, code is adapted from x86.
Priority: low
Attachments (3)
Change History (9)
by , 14 years ago
Attachment: | 0001-Implement-arch_debug_get_caller.patch added |
---|
comment:1 by , 14 years ago
patch: | 0 → 1 |
---|
follow-up: 5 comment:2 by , 14 years ago
I don't think that this returns the correct result. Unlike on x86 the return address is not stored in the own stack frame, but in the caller's stack frame.
Note that the same issue might ail the stack trace code.
comment:3 by , 14 years ago
patch: | 1 → 0 |
---|
comment:4 by , 14 years ago
patch: | 0 → 1 |
---|
by , 14 years ago
Attachment: | 0002-Exclude-stack_trace-function-from-stack-trace.patch added |
---|
proposed patch: exclude stack_trace from stack trace
follow-up: 6 comment:5 by , 14 years ago
Right, it would've returned the requesting function as caller.
As can be seen in #6129, stack_trace
itself is included in the stack trace. This is remedied accordingly in patch 2.
Ideally I would like to see everything including panic
vanish from the regular stack trace, but that would seem to require major kernel debugger API refactoring, pushing the callsite of debug_call_with_fault_handler
to arch-specific code to strip known-preceding frames.
Inside ppc it might be feasible to split stack_trace
in one function with stack_frame*
argument and a wrapper with the present int argc, char** argv
args to handle the evaluate_debug_command
case.
comment:6 by , 14 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Replying to andreasf:
Right, it would've returned the requesting function as caller.
As can be seen in #6129,
stack_trace
itself is included in the stack trace. This is remedied accordingly in patch 2.
Patches applied in hrev37032 and hrev37033.
Ideally I would like to see everything including
panic
vanish from the regular stack trace, but that would seem to require major kernel debugger API refactoring, pushing the callsite ofdebug_call_with_fault_handler
to arch-specific code to strip known-preceding frames.
Actually it can be done significantly easier. If you look at enter_kernel_debugger() and debug_trap_cpu_in_kdl(), you'll see that they call arch_debug_save_registers(), which stores the registers state at that point (not implemented for PPC yet). The x86 debug code already uses that for stack traces on other CPUs. I haven't enabled it for the current CPU yet mostly out of concern that any problem in that code would totally break stack traces, though I guess that's really a non-issue. That would already simplify stack traces somewhat. The remaining superfluous stack frames could be skipped, e.g. by passing the number of those stack traces to arch_debug_save_registers() or saving it directly in the structure. Alternatively the outmost kernel debugger function (panic() or kernel_debugger()) could get its caller, which would be stored and serve as a marker for skipping stack frames. Or finally the complete range of kernel debugger code could be skipped at the beginning of the stack trace.
Inside ppc it might be feasible to split
stack_trace
in one function withstack_frame*
argument and a wrapper with the presentint argc, char** argv
args to handle theevaluate_debug_command
case.
The same goes for x86, too. OTOH, arch_debug_stack_trace() is a kernel debugger function. Printing the same stack trace as the bt
should be the desired behavior anyway. ATM the stack trace printed automatically on a panic differs from the one bt
prints, which is seriously annoying, because it also doesn't match the call
stack frame numbering.
proposed patch