[PATCH v3 0/4] Fix objtool issues about do_syscall() and Clang on LoongArch

[PATCH v3 0/4] Fix objtool issues about do_syscall() and Clang on LoongArch

[Tiezhu Yang]

With this series, there is no objtool warning about do_syscall() and
there is handle_syscall() which is the previous frame of do_syscall()
in the call trace when running "echo l > /proc/sysrq-trigger".

Compiled with GCC and Clang, 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

Tiezhu Yang (4):
  objtool: Handle frame pointer related instructions
  LoongArch: Remove STACK_FRAME_NON_STANDARD(do_syscall)
  LoongArch: Set AS_HAS_THIN_ADD_SUB as y if AS_IS_LLVM
  LoongArch: Enable objtool for Clang

 arch/loongarch/Kconfig                |  4 ++--
 arch/loongarch/kernel/syscall.c       |  4 ----
 tools/objtool/arch/loongarch/decode.c | 11 ++++++++++-
 tools/objtool/check.c                 | 23 ++++++++++++++++++++---
 tools/objtool/include/objtool/elf.h   |  1 +
 5 files changed, 33 insertions(+), 10 deletions(-)

-- 
2.42.0

[PATCH v3 1/4] objtool: Handle frame pointer related instructions

[Tiezhu Yang]

After commit a0f7085f6a63 ("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 rather than
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,
so the beginning thought is not a good way.

Essentially, there are two special frame pointer instructions
"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 not add a arch specified
update_cfi_state(), 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: stable@vger.kernel.org # 6.9+
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 tools/objtool/arch/loongarch/decode.c | 11 ++++++++++-
 tools/objtool/check.c                 | 23 ++++++++++++++++++++---
 tools/objtool/include/objtool/elf.h   |  1 +
 3 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/tools/objtool/arch/loongarch/decode.c b/tools/objtool/arch/loongarch/decode.c
index aee479d2191c..69b66994f2a1 100644
--- a/tools/objtool/arch/loongarch/decode.c
+++ b/tools/objtool/arch/loongarch/decode.c
@@ -122,7 +122,7 @@ 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 */
+			/* addi.d sp,sp,si12 or addi.d fp,sp,si12 or addi.d sp,fp,si12 */
 			insn->immediate = sign_extend64(inst.reg2i12_format.immediate, 11);
 			ADD_OP(op) {
 				op->src.type = OP_SRC_ADD;
@@ -132,6 +132,15 @@ static bool decode_insn_reg2i12_fomat(union loongarch_instruction inst,
 				op->dest.reg = inst.reg2i12_format.rd;
 			}
 		}
+		if ((inst.reg2i12_format.rd == CFI_SP) && (inst.reg2i12_format.rj == CFI_FP)) {
+			/* addi.d sp,fp,si12 */
+			struct symbol *func = find_func_containing(insn->sec, insn->offset);
+
+			if (!func)
+				return false;
+
+			func->frame_pointer = true;
+		}
 		break;
 	case ldd_op:
 		if (inst.reg2i12_format.rj == CFI_SP) {
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 01237d167223..af9cfed7f4ec 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2993,10 +2993,27 @@ 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 &&
+			    insn->sym->frame_pointer) {
+				/* addi.d fp,sp,imm on LoongArch */
+				if (cfa->base == CFI_SP && cfa->offset == op->src.offset) {
+					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 on LoongArch */
+				if (cfa->base == CFI_BP && cfa->offset == 0) {
+					if (insn->sym->frame_pointer) {
+						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;
 			}
 
diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
index 2b8a69de4db8..d7e815c2fd15 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -68,6 +68,7 @@ struct symbol {
 	u8 warned	     : 1;
 	u8 embedded_insn     : 1;
 	u8 local_label       : 1;
+	u8 frame_pointer     : 1;
 	struct list_head pv_target;
 	struct reloc *relocs;
 };
-- 
2.42.0

[PATCH v3 2/4] LoongArch: Remove STACK_FRAME_NON_STANDARD(do_syscall)

[Tiezhu Yang]

For now, it can remove STACK_FRAME_NON_STANDARD(do_syscall) because
there is no objtool warning "do_syscall+0x11c: return with modified
stack frame", then there is handle_syscall() which is the previous
frame of do_syscall() in the call trace when executing the command
"echo l > /proc/sysrq-trigger".

Fixes: a0f7085f6a63 ("LoongArch: Add RANDOMIZE_KSTACK_OFFSET support")
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 arch/loongarch/kernel/syscall.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/loongarch/kernel/syscall.c b/arch/loongarch/kernel/syscall.c
index ba5d0930a74f..168bd97540f8 100644
--- a/arch/loongarch/kernel/syscall.c
+++ b/arch/loongarch/kernel/syscall.c
@@ -79,7 +79,3 @@ void noinstr __no_stack_protector do_syscall(struct pt_regs *regs)
 
 	syscall_exit_to_user_mode(regs);
 }
-
-#ifdef CONFIG_RANDOMIZE_KSTACK_OFFSET
-STACK_FRAME_NON_STANDARD(do_syscall);
-#endif
-- 
2.42.0

[PATCH v3 3/4] LoongArch: Set AS_HAS_THIN_ADD_SUB as y if AS_IS_LLVM

[Tiezhu Yang]

When building kernel with "make CC=clang defconfig", LLVM Assembler
is used due to LLVM_IAS=0 is not specified, then AS_HAS_THIN_ADD_SUB
is not set, thus objtool can not be built after enable it for Clang.

config AS_HAS_THIN_ADD_SUB is to check whether -mthin-add-sub option is
available to know R_LARCH_{32,64}_PCREL are supported for GNU Assembler,
there is no this option for LLVM Assembler. The minimal version of Clang
is 18 for building LoongArch kernel, and Clang >= 17 already supports
R_LARCH_{32,64}_PCREL, that is to say, there is no need to depend on
AS_HAS_THIN_ADD_SUB for Clang, so just set AS_HAS_THIN_ADD_SUB as y if
AS_IS_LLVM.

Fixes: 120dd4118e58 ("LoongArch: Only allow OBJTOOL & ORC unwinder if toolchain supports -mthin-add-sub")
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 arch/loongarch/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index 70f169210b52..e1b6cb306d4d 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -265,7 +265,7 @@ config AS_HAS_FCSR_CLASS
 	def_bool $(as-instr,movfcsr2gr \$t0$(comma)\$fcsr0)
 
 config AS_HAS_THIN_ADD_SUB
-	def_bool $(cc-option,-Wa$(comma)-mthin-add-sub)
+	def_bool $(cc-option,-Wa$(comma)-mthin-add-sub) || AS_IS_LLVM
 
 config AS_HAS_LSX_EXTENSION
 	def_bool $(as-instr,vld \$vr0$(comma)\$a0$(comma)0)
-- 
2.42.0

[PATCH v3 4/4] LoongArch: Enable objtool for Clang

[Tiezhu Yang]

For now, it can enable objtool for Clang, just remove !CC_IS_CLANG
for HAVE_OBJTOOL in arch/loongarch/Kconfig.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 arch/loongarch/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index e1b6cb306d4d..c53f8953d46c 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -146,7 +146,7 @@ config LOONGARCH
 	select HAVE_LIVEPATCH
 	select HAVE_MOD_ARCH_SPECIFIC
 	select HAVE_NMI
-	select HAVE_OBJTOOL if AS_HAS_EXPLICIT_RELOCS && AS_HAS_THIN_ADD_SUB && !CC_IS_CLANG
+	select HAVE_OBJTOOL if AS_HAS_EXPLICIT_RELOCS && AS_HAS_THIN_ADD_SUB
 	select HAVE_PCI
 	select HAVE_PERF_EVENTS
 	select HAVE_PERF_REGS
-- 
2.42.0

Re: [PATCH v3 0/4] Fix objtool issues about do_syscall() and Clang on LoongArch

[Huacai Chen]

Hi, Ruoyao and Jinyang,

On Wed, Aug 7, 2024 at 4:59 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> With this series, there is no objtool warning about do_syscall() and
> there is handle_syscall() which is the previous frame of do_syscall()
> in the call trace when running "echo l > /proc/sysrq-trigger".
>
> Compiled with GCC and Clang, 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
Please review this series. If you have no objections, I will apply to
loongarch-next.

Huacai

>
> Tiezhu Yang (4):
>   objtool: Handle frame pointer related instructions
>   LoongArch: Remove STACK_FRAME_NON_STANDARD(do_syscall)
>   LoongArch: Set AS_HAS_THIN_ADD_SUB as y if AS_IS_LLVM
>   LoongArch: Enable objtool for Clang
>
>  arch/loongarch/Kconfig                |  4 ++--
>  arch/loongarch/kernel/syscall.c       |  4 ----
>  tools/objtool/arch/loongarch/decode.c | 11 ++++++++++-
>  tools/objtool/check.c                 | 23 ++++++++++++++++++++---
>  tools/objtool/include/objtool/elf.h   |  1 +
>  5 files changed, 33 insertions(+), 10 deletions(-)
>
> --
> 2.42.0
>

Re: [PATCH v3 3/4] LoongArch: Set AS_HAS_THIN_ADD_SUB as y if AS_IS_LLVM

[Xi Ruoyao]

On Wed, 2024-08-07 at 16:59 +0800, Tiezhu Yang wrote:
> When building kernel with "make CC=clang defconfig", LLVM Assembler
> is used due to LLVM_IAS=0 is not specified, then AS_HAS_THIN_ADD_SUB
> is not set, thus objtool can not be built after enable it for Clang.
> 
> config AS_HAS_THIN_ADD_SUB is to check whether -mthin-add-sub option is
> available to know R_LARCH_{32,64}_PCREL are supported for GNU Assembler,
> there is no this option for LLVM Assembler. The minimal version of Clang
> is 18 for building LoongArch kernel, and Clang >= 17 already supports
> R_LARCH_{32,64}_PCREL, that is to say, there is no need to depend on
> AS_HAS_THIN_ADD_SUB for Clang, so just set AS_HAS_THIN_ADD_SUB as y if
> AS_IS_LLVM.
> 
> Fixes: 120dd4118e58 ("LoongArch: Only allow OBJTOOL & ORC unwinder if toolchain supports -mthin-add-sub")
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>

This is what I wanted in
https://lore.kernel.org/all/20240604150741.30252-1-xry111@xry111.site/.

Reviewed-by: Xi Ruoyao <xry111@xry111.site>

> ---
>  arch/loongarch/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> index 70f169210b52..e1b6cb306d4d 100644
> --- a/arch/loongarch/Kconfig
> +++ b/arch/loongarch/Kconfig
> @@ -265,7 +265,7 @@ config AS_HAS_FCSR_CLASS
>  	def_bool $(as-instr,movfcsr2gr \$t0$(comma)\$fcsr0)
>  
>  config AS_HAS_THIN_ADD_SUB
> -	def_bool $(cc-option,-Wa$(comma)-mthin-add-sub)
> +	def_bool $(cc-option,-Wa$(comma)-mthin-add-sub) || AS_IS_LLVM
>  
>  config AS_HAS_LSX_EXTENSION
>  	def_bool $(as-instr,vld \$vr0$(comma)\$a0$(comma)0)

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH v3 3/4] LoongArch: Set AS_HAS_THIN_ADD_SUB as y if AS_IS_LLVM

[Huacai Chen]

On Sun, Sep 8, 2024 at 10:43 PM Xi Ruoyao <xry111@xry111.site> wrote:
>
> On Wed, 2024-08-07 at 16:59 +0800, Tiezhu Yang wrote:
> > When building kernel with "make CC=clang defconfig", LLVM Assembler
> > is used due to LLVM_IAS=0 is not specified, then AS_HAS_THIN_ADD_SUB
> > is not set, thus objtool can not be built after enable it for Clang.
> >
> > config AS_HAS_THIN_ADD_SUB is to check whether -mthin-add-sub option is
> > available to know R_LARCH_{32,64}_PCREL are supported for GNU Assembler,
> > there is no this option for LLVM Assembler. The minimal version of Clang
> > is 18 for building LoongArch kernel, and Clang >= 17 already supports
> > R_LARCH_{32,64}_PCREL, that is to say, there is no need to depend on
> > AS_HAS_THIN_ADD_SUB for Clang, so just set AS_HAS_THIN_ADD_SUB as y if
> > AS_IS_LLVM.
> >
> > Fixes: 120dd4118e58 ("LoongArch: Only allow OBJTOOL & ORC unwinder if toolchain supports -mthin-add-sub")
> > Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
>
> This is what I wanted in
> https://lore.kernel.org/all/20240604150741.30252-1-xry111@xry111.site/.
Then which one is better?  AS_IS_LLVM or CC_IS_CLANG?

Huacai

>
> Reviewed-by: Xi Ruoyao <xry111@xry111.site>
>
> > ---
> >  arch/loongarch/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> > index 70f169210b52..e1b6cb306d4d 100644
> > --- a/arch/loongarch/Kconfig
> > +++ b/arch/loongarch/Kconfig
> > @@ -265,7 +265,7 @@ config AS_HAS_FCSR_CLASS
> >       def_bool $(as-instr,movfcsr2gr \$t0$(comma)\$fcsr0)
> >
> >  config AS_HAS_THIN_ADD_SUB
> > -     def_bool $(cc-option,-Wa$(comma)-mthin-add-sub)
> > +     def_bool $(cc-option,-Wa$(comma)-mthin-add-sub) || AS_IS_LLVM
> >
> >  config AS_HAS_LSX_EXTENSION
> >       def_bool $(as-instr,vld \$vr0$(comma)\$a0$(comma)0)
>
> --
> Xi Ruoyao <xry111@xry111.site>
> School of Aerospace Science and Technology, Xidian University

Re: [PATCH v3 1/4] objtool: Handle frame pointer related instructions

[Jinyang He]

On 2024-08-07 16:59, Tiezhu Yang wrote:

> After commit a0f7085f6a63 ("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 rather than
> 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,
> so the beginning thought is not a good way.
>
> Essentially, there are two special frame pointer instructions
> "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 not add a arch specified
> update_cfi_state(), 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: stable@vger.kernel.org # 6.9+
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>   tools/objtool/arch/loongarch/decode.c | 11 ++++++++++-
>   tools/objtool/check.c                 | 23 ++++++++++++++++++++---
>   tools/objtool/include/objtool/elf.h   |  1 +
>   3 files changed, 31 insertions(+), 4 deletions(-)
LGTM.
Thanks!

Jinyang

> diff --git a/tools/objtool/arch/loongarch/decode.c b/tools/objtool/arch/loongarch/decode.c
> index aee479d2191c..69b66994f2a1 100644
> --- a/tools/objtool/arch/loongarch/decode.c
> +++ b/tools/objtool/arch/loongarch/decode.c
> @@ -122,7 +122,7 @@ 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 */
> +			/* addi.d sp,sp,si12 or addi.d fp,sp,si12 or addi.d sp,fp,si12 */
>   			insn->immediate = sign_extend64(inst.reg2i12_format.immediate, 11);
>   			ADD_OP(op) {
>   				op->src.type = OP_SRC_ADD;
> @@ -132,6 +132,15 @@ static bool decode_insn_reg2i12_fomat(union loongarch_instruction inst,
>   				op->dest.reg = inst.reg2i12_format.rd;
>   			}
>   		}
> +		if ((inst.reg2i12_format.rd == CFI_SP) && (inst.reg2i12_format.rj == CFI_FP)) {
> +			/* addi.d sp,fp,si12 */
> +			struct symbol *func = find_func_containing(insn->sec, insn->offset);
> +
> +			if (!func)
> +				return false;
> +
> +			func->frame_pointer = true;
> +		}
>   		break;
>   	case ldd_op:
>   		if (inst.reg2i12_format.rj == CFI_SP) {
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 01237d167223..af9cfed7f4ec 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -2993,10 +2993,27 @@ 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 &&
> +			    insn->sym->frame_pointer) {
> +				/* addi.d fp,sp,imm on LoongArch */
> +				if (cfa->base == CFI_SP && cfa->offset == op->src.offset) {
> +					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 on LoongArch */
> +				if (cfa->base == CFI_BP && cfa->offset == 0) {
> +					if (insn->sym->frame_pointer) {
> +						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;
>   			}
>   
> diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
> index 2b8a69de4db8..d7e815c2fd15 100644
> --- a/tools/objtool/include/objtool/elf.h
> +++ b/tools/objtool/include/objtool/elf.h
> @@ -68,6 +68,7 @@ struct symbol {
>   	u8 warned	     : 1;
>   	u8 embedded_insn     : 1;
>   	u8 local_label       : 1;
> +	u8 frame_pointer     : 1;
>   	struct list_head pv_target;
>   	struct reloc *relocs;
>   };

Re: [PATCH v3 3/4] LoongArch: Set AS_HAS_THIN_ADD_SUB as y if AS_IS_LLVM

[Xi Ruoyao]

On Mon, 2024-09-09 at 09:04 +0800, Huacai Chen wrote:
> On Sun, Sep 8, 2024 at 10:43 PM Xi Ruoyao <xry111@xry111.site> wrote:
> > 
> > On Wed, 2024-08-07 at 16:59 +0800, Tiezhu Yang wrote:
> > > When building kernel with "make CC=clang defconfig", LLVM Assembler
> > > is used due to LLVM_IAS=0 is not specified, then AS_HAS_THIN_ADD_SUB
> > > is not set, thus objtool can not be built after enable it for Clang.
> > > 
> > > config AS_HAS_THIN_ADD_SUB is to check whether -mthin-add-sub option is
> > > available to know R_LARCH_{32,64}_PCREL are supported for GNU Assembler,
> > > there is no this option for LLVM Assembler. The minimal version of Clang
> > > is 18 for building LoongArch kernel, and Clang >= 17 already supports
> > > R_LARCH_{32,64}_PCREL, that is to say, there is no need to depend on
> > > AS_HAS_THIN_ADD_SUB for Clang, so just set AS_HAS_THIN_ADD_SUB as y if
> > > AS_IS_LLVM.
> > > 
> > > Fixes: 120dd4118e58 ("LoongArch: Only allow OBJTOOL & ORC unwinder if toolchain supports -mthin-add-sub")
> > > Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> > 
> > This is what I wanted in
> > https://lore.kernel.org/all/20240604150741.30252-1-xry111@xry111.site/.
> Then which one is better?  AS_IS_LLVM or CC_IS_CLANG?

AS_IS_LLVM should be better because when "make LLVM=1 LLVM_IAS=0" we
still need to check against gas from binutils.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University