Skip to content

Commit 6a00ef4

Browse files
changbindupalmer-dabbelt
authored andcommitted
riscv: eliminate unreliable __builtin_frame_address(1)
I tried different pieces of code which uses __builtin_frame_address(1) (with both gcc version 7.5.0 and 10.3.0) to verify whether it works as expected on riscv64. The result is negative. What the compiler had generated is as below: 31 fp = (unsigned long)__builtin_frame_address(1); 0xffffffff80006024 <+200>: ld s1,0(s0) It takes '0(s0)' as the address of frame 1 (caller), but the actual address should be '-16(s0)'. | ... | <-+ +-----------------+ | | return address | | | previous fp | | | saved registers | | | local variables | | $fp --> | ... | | +-----------------+ | | return address | | | previous fp --------+ | saved registers | $sp --> | local variables | +-----------------+ This leads the kernel can not dump the full stack trace on riscv. [ 7.222126][ T1] Call Trace: [ 7.222804][ T1] [<ffffffff80006058>] dump_backtrace+0x2c/0x3a This problem is not exposed on most riscv builds just because the '0(s0)' occasionally is the address frame 2 (caller's caller), if only ra and fp are stored in frame 1 (caller). | ... | <-+ +-----------------+ | | return address | | $fp --> | previous fp | | +-----------------+ | | return address | | | previous fp --------+ | saved registers | $sp --> | local variables | +-----------------+ This could be a *bug* of gcc that should be fixed. But as noted in gcc manual "Calling this function with a nonzero argument can have unpredictable effects, including crashing the calling program.", let's remove the '__builtin_frame_address(1)' in backtrace code. With this fix now it can show full stack trace: [ 10.444838][ T1] Call Trace: [ 10.446199][ T1] [<ffffffff8000606c>] dump_backtrace+0x2c/0x3a [ 10.447711][ T1] [<ffffffff800060ac>] show_stack+0x32/0x3e [ 10.448710][ T1] [<ffffffff80a005c0>] dump_stack_lvl+0x58/0x7a [ 10.449941][ T1] [<ffffffff80a005f6>] dump_stack+0x14/0x1c [ 10.450929][ T1] [<ffffffff804c04ee>] ubsan_epilogue+0x10/0x5a [ 10.451869][ T1] [<ffffffff804c092e>] __ubsan_handle_load_invalid_value+0x6c/0x78 [ 10.453049][ T1] [<ffffffff8018f834>] __pagevec_release+0x62/0x64 [ 10.455476][ T1] [<ffffffff80190830>] truncate_inode_pages_range+0x132/0x5be [ 10.456798][ T1] [<ffffffff80190ce0>] truncate_inode_pages+0x24/0x30 [ 10.457853][ T1] [<ffffffff8045bb04>] kill_bdev+0x32/0x3c ... Signed-off-by: Changbin Du <[email protected]> Fixes: eac2f30 ("riscv: stacktrace: fix the riscv stacktrace when CONFIG_FRAME_POINTER enabled") Cc: [email protected] Signed-off-by: Palmer Dabbelt <[email protected]>
1 parent e783362 commit 6a00ef4

File tree

1 file changed

+5
-4
lines changed

1 file changed

+5
-4
lines changed

arch/riscv/kernel/stacktrace.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,16 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
2222
bool (*fn)(void *, unsigned long), void *arg)
2323
{
2424
unsigned long fp, sp, pc;
25+
int level = 0;
2526

2627
if (regs) {
2728
fp = frame_pointer(regs);
2829
sp = user_stack_pointer(regs);
2930
pc = instruction_pointer(regs);
3031
} else if (task == NULL || task == current) {
31-
fp = (unsigned long)__builtin_frame_address(1);
32-
sp = (unsigned long)__builtin_frame_address(0);
33-
pc = (unsigned long)__builtin_return_address(0);
32+
fp = (unsigned long)__builtin_frame_address(0);
33+
sp = sp_in_global;
34+
pc = (unsigned long)walk_stackframe;
3435
} else {
3536
/* task blocked in __switch_to */
3637
fp = task->thread.s[0];
@@ -42,7 +43,7 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
4243
unsigned long low, high;
4344
struct stackframe *frame;
4445

45-
if (unlikely(!__kernel_text_address(pc) || !fn(arg, pc)))
46+
if (unlikely(!__kernel_text_address(pc) || (level++ >= 1 && !fn(arg, pc))))
4647
break;
4748

4849
/* Validate frame pointer */

0 commit comments

Comments
 (0)