Re: [PATCH 1/2] objtool/LoongArch: Restrict stack operation instruction

[Huacai Chen]

Hi, Tiezhu,

Can this patch also fix the warnings of ClangBuiltLinux?

Huacai

On Tue, Jul 30, 2024 at 2:19 PM Tiezhu Yang <yangtiezhu@xxxxxxxxxxx> wrote:
>
> After commit a0f7085f6a63 ("LoongArch: Add RANDOMIZE_KSTACK_OFFSET
> support"), the code flow of do_syscall() was changed when compiled
> with GCC due to the secondary stack of add_random_kstack_offset(),
> something like this:
>
>   addi.d          $sp, $sp, -32
>   st.d            $fp, $sp, 16
>   st.d            $ra, $sp, 24
>   addi.d          $fp, $sp, 32
>   ...
>   sub.d           $sp, $sp, $t1
>   ...
>   addi.d          $sp, $fp, -32
>   ld.d            $ra, $sp, 24
>   ld.d            $fp, $sp, 16
>   addi.d          $sp, $sp, 32
>
> fp points to the stack top, it is only used to save and restore the
> original sp and is not used as cfa base for arch_callee_saved_reg().
> In the case OP_SRC_ADD of update_cfi_state(), the above rare case is
> not handled so that lead to a wrong stack size, then there exists a
> objtool warning "do_syscall+0x11c: return with modified stack frame".
>
> Because the fp related instructions do not modify the stack frame,
> no need to decode them, just restrict stack operation instruction
> only with the single case "addi.d sp,sp,si12".
>
> By the way, if fp is used as cfa base for arch_callee_saved_reg()
> (there is no this behavior on LoongArch at present), then it needs
> to decode the related instructions and modify update_cfi_state().
>
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> Closes: https://lore.kernel.org/oe-kbuild-all/202407201336.mW8dj1VB-lkp@xxxxxxxxx/
> Fixes: b2d23158e6c8 ("objtool/LoongArch: Implement instruction decoder")
> Signed-off-by: Tiezhu Yang <yangtiezhu@xxxxxxxxxxx>
> ---
>  tools/objtool/arch/loongarch/decode.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/objtool/arch/loongarch/decode.c b/tools/objtool/arch/loongarch/decode.c
> index aee479d2191c..6a34af675cee 100644
> --- a/tools/objtool/arch/loongarch/decode.c
> +++ b/tools/objtool/arch/loongarch/decode.c
> @@ -121,8 +121,8 @@ static bool decode_insn_reg2i12_fomat(union loongarch_instruction inst,
>  {
>         switch (inst.reg2i12_format.opcode) {
>         case addid_op:
> -               if ((inst.reg2i12_format.rd == CFI_SP) || (inst.reg2i12_format.rj == CFI_SP)) {
> -                       /* addi.d sp,sp,si12 or addi.d fp,sp,si12 */
> +               if ((inst.reg2i12_format.rd == CFI_SP) && (inst.reg2i12_format.rj == CFI_SP)) {
> +                       /* addi.d sp,sp,si12 */
>                         insn->immediate = sign_extend64(inst.reg2i12_format.immediate, 11);
>                         ADD_OP(op) {
>                                 op->src.type = OP_SRC_ADD;
> --
> 2.42.0
>
>