Skip to content

Commit da5b2ad

Browse files
seehearfeelchenhuacai
authored andcommitted
objtool: Handle frame pointer related instructions
After commit a0f7085 ("LoongArch: Add RANDOMIZE_KSTACK_OFFSET support"), there are three new instructions "addi.d $fp, $sp, 32", "sub.d $sp, $sp, $t0" and "addi.d $sp, $fp, -32" for the secondary stack in do_syscall(), then there is a objtool warning "return with modified stack frame" and no handle_syscall() which is the previous frame of do_syscall() in the call trace when executing the command "echo l > /proc/sysrq-trigger". objdump shows something like this: 0000000000000000 <do_syscall>: 0: 02ff8063 addi.d $sp, $sp, -32 4: 29c04076 st.d $fp, $sp, 16 8: 29c02077 st.d $s0, $sp, 8 c: 29c06061 st.d $ra, $sp, 24 10: 02c08076 addi.d $fp, $sp, 32 ... 74: 0011b063 sub.d $sp, $sp, $t0 ... a8: 4c000181 jirl $ra, $t0, 0 ... dc: 02ff82c3 addi.d $sp, $fp, -32 e0: 28c06061 ld.d $ra, $sp, 24 e4: 28c04076 ld.d $fp, $sp, 16 e8: 28c02077 ld.d $s0, $sp, 8 ec: 02c08063 addi.d $sp, $sp, 32 f0: 4c000020 jirl $zero, $ra, 0 The instruction "sub.d $sp, $sp, $t0" changes the stack bottom and the new stack size is a random value, in order to find the return address of do_syscall() which is stored in the original stack frame after executing "jirl $ra, $t0, 0", it should use fp which points to the original stack top. At the beginning, the thought is tended to decode the secondary stack instruction "sub.d $sp, $sp, $t0" and set it as a label, then check this label for the two frame pointer instructions to change the cfa base and cfa offset during the period of secondary stack in update_cfi_state(). This is valid for GCC but invalid for Clang due to there are different secondary stack instructions for ClangBuiltLinux on LoongArch, something like this: 0000000000000000 <do_syscall>: ... 88: 00119064 sub.d $a0, $sp, $a0 8c: 00150083 or $sp, $a0, $zero ... Actually, it equals to a single instruction "sub.d $sp, $sp, $a0", but there is no proper condition to check it as a label like GCC, and so the beginning thought is not a good way. Essentially, there are two special frame pointer instructions which are "addi.d $fp, $sp, imm" and "addi.d $sp, $fp, imm", the first one points fp to the original stack top and the second one restores the original stack bottom from fp. Based on the above analysis, in order to avoid adding an arch-specific update_cfi_state(), we just add a member "frame_pointer" in the "struct symbol" as a label to avoid affecting the current normal case, then set it as true only if there is "addi.d $sp, $fp, imm". The last is to check this label for the two frame pointer instructions to change the cfa base and cfa offset in update_cfi_state(). Tested with the following two configs: (1) CONFIG_RANDOMIZE_KSTACK_OFFSET=y && CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT=n (2) CONFIG_RANDOMIZE_KSTACK_OFFSET=y && CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT=y By the way, there is no effect for x86 with this patch, tested on the x86 machine with Fedora 40 system. Cc: [email protected] # 6.9+ Signed-off-by: Tiezhu Yang <[email protected]> Signed-off-by: Huacai Chen <[email protected]>
1 parent 987cbaf commit da5b2ad

File tree

3 files changed

+31
-4
lines changed

3 files changed

+31
-4
lines changed

tools/objtool/arch/loongarch/decode.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ static bool decode_insn_reg2i12_fomat(union loongarch_instruction inst,
122122
switch (inst.reg2i12_format.opcode) {
123123
case addid_op:
124124
if ((inst.reg2i12_format.rd == CFI_SP) || (inst.reg2i12_format.rj == CFI_SP)) {
125-
/* addi.d sp,sp,si12 or addi.d fp,sp,si12 */
125+
/* addi.d sp,sp,si12 or addi.d fp,sp,si12 or addi.d sp,fp,si12 */
126126
insn->immediate = sign_extend64(inst.reg2i12_format.immediate, 11);
127127
ADD_OP(op) {
128128
op->src.type = OP_SRC_ADD;
@@ -132,6 +132,15 @@ static bool decode_insn_reg2i12_fomat(union loongarch_instruction inst,
132132
op->dest.reg = inst.reg2i12_format.rd;
133133
}
134134
}
135+
if ((inst.reg2i12_format.rd == CFI_SP) && (inst.reg2i12_format.rj == CFI_FP)) {
136+
/* addi.d sp,fp,si12 */
137+
struct symbol *func = find_func_containing(insn->sec, insn->offset);
138+
139+
if (!func)
140+
return false;
141+
142+
func->frame_pointer = true;
143+
}
135144
break;
136145
case ldd_op:
137146
if (inst.reg2i12_format.rj == CFI_SP) {

tools/objtool/check.c

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2993,10 +2993,27 @@ static int update_cfi_state(struct instruction *insn,
29932993
break;
29942994
}
29952995

2996-
if (op->dest.reg == CFI_SP && op->src.reg == CFI_BP) {
2996+
if (op->dest.reg == CFI_BP && op->src.reg == CFI_SP &&
2997+
insn->sym->frame_pointer) {
2998+
/* addi.d fp,sp,imm on LoongArch */
2999+
if (cfa->base == CFI_SP && cfa->offset == op->src.offset) {
3000+
cfa->base = CFI_BP;
3001+
cfa->offset = 0;
3002+
}
3003+
break;
3004+
}
29973005

2998-
/* lea disp(%rbp), %rsp */
2999-
cfi->stack_size = -(op->src.offset + regs[CFI_BP].offset);
3006+
if (op->dest.reg == CFI_SP && op->src.reg == CFI_BP) {
3007+
/* addi.d sp,fp,imm on LoongArch */
3008+
if (cfa->base == CFI_BP && cfa->offset == 0) {
3009+
if (insn->sym->frame_pointer) {
3010+
cfa->base = CFI_SP;
3011+
cfa->offset = -op->src.offset;
3012+
}
3013+
} else {
3014+
/* lea disp(%rbp), %rsp */
3015+
cfi->stack_size = -(op->src.offset + regs[CFI_BP].offset);
3016+
}
30003017
break;
30013018
}
30023019

tools/objtool/include/objtool/elf.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ struct symbol {
6868
u8 warned : 1;
6969
u8 embedded_insn : 1;
7070
u8 local_label : 1;
71+
u8 frame_pointer : 1;
7172
struct list_head pv_target;
7273
struct reloc *relocs;
7374
};

0 commit comments

Comments
 (0)