Messages in this thread Patch in this message |  | | Subject | Re: [PATCH v2 2/3] objtool: Handle secondary stack related instructions | | From | Tiezhu Yang <> | | Date | Mon, 5 Aug 2024 14:37:57 +0800 |
| |
On 08/05/2024 11:26 AM, Tiezhu Yang wrote: > After commit a0f7085f6a63 ("LoongArch: Add RANDOMIZE_KSTACK_OFFSET > support"), there is a new instruction "sub.d $sp, $sp, $t0" for the > secondary stack in do_syscall(), then there exists a objtool warning > "do_syscall+0x11c: return with modified stack frame" and there is no > handle_syscall() which is the previous frame of do_syscall() in the > call trace when executing the command "echo l > /proc/sysrq-trigger".
...
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c > index 01237d167223..c7b9942fee29 100644 > --- a/tools/objtool/check.c > +++ b/tools/objtool/check.c > @@ -2993,6 +2993,28 @@ static int update_cfi_state(struct instruction *insn, > break; > } > > + if (op->dest.reg == CFI_BP && op->src.reg == CFI_SP) { > + /* addi.d fp,sp,imm for the secondary stack on LoongArch */ > + if (cfa->base == CFI_SP && cfa->offset == op->src.offset) { > + if (insn->sym->secondary_stack) { > + cfa->base = CFI_BP; > + cfa->offset = 0; > + } > + } > + break; > + } > + > + if (op->dest.reg == CFI_SP && op->src.reg == CFI_BP) { > + /* addi.d sp,fp,imm for the secondary stack on LoongArch */ > + if (cfa->base == CFI_FP && cfa->offset == 0) {
Here should be CFI_BP instead of CFI_FP which is only defined for LoongArch.
> + if (insn->sym->secondary_stack) { > + cfa->base = CFI_SP; > + cfa->offset = -op->src.offset; > + } > + } > + break; > + } > + > if (op->dest.reg == CFI_SP && op->src.reg == CFI_BP) { > > /* lea disp(%rbp), %rsp */
Oh, sorry, I forgot to test this change on x86.
Here is the test info on x86: the cfa->base is CFI_BP or CFI_BP_INDIRECT and the cfa->offset is not 0 if (op->dest.reg == CFI_SP && op->src.reg == CFI_BP), thus it can check the following condition if(cfa->base == CFI_BP && cfa->offset == 0) to distinguish x86 and LoongArch.
So the correct change should be something like this to make sure it works well for both x86 and LoongArch:
diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 01237d167223..0832d20c95d2 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -2993,10 +2993,28 @@ static int update_cfi_state(struct instruction *insn, break; }
- if (op->dest.reg == CFI_SP && op->src.reg == CFI_BP) { + if (op->dest.reg == CFI_BP && op->src.reg == CFI_SP) { + /* addi.d fp,sp,imm for the secondary stack on LoongArch */ + if (cfa->base == CFI_SP && cfa->offset == op->src.offset) { + if (insn->sym->secondary_stack) { + cfa->base = CFI_BP; + cfa->offset = 0; + } + } + break; + }
- /* lea disp(%rbp), %rsp */ - cfi->stack_size = -(op->src.offset + regs[CFI_BP].offset); + if (op->dest.reg == CFI_SP && op->src.reg == CFI_BP) { + /* addi.d sp,fp,imm for the secondary stack on LoongArch */ + if (cfa->base == CFI_BP && cfa->offset == 0) { + if (insn->sym->secondary_stack) { + cfa->base = CFI_SP; + cfa->offset = -op->src.offset; + } + } else { + /* lea disp(%rbp), %rsp */ + cfi->stack_size = -(op->src.offset + regs[CFI_BP].offset); + } break; } I will wait for some days to get more review comments and then send v3 later.
Thanks, Tiezhu
|  |