[PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Xi Ruoyao]

GAS <= 2.41 does not support generating R_LARCH_{32,64}_PCREL for
"label - ." and it generates R_LARCH_{ADD,SUB}{32,64} pairs instead.
objtool cannot handle R_LARCH_{ADD,SUB}{32,64} pair in __jump_table
(static key implementation) and etc. so it will produce some warnings.
This is causing the kernel CI systems to complain everywhere.

For GAS we can check if -mthin-add-sub option is available to know if
R_LARCH_{32,64}_PCREL are supported.

For Clang, we require Clang >= 18 and Clang >= 17 already supports
R_LARCH_{32,64}_PCREL, so we can always assume Clang is fine for
objtool.

Note that __jump_table is not generated by the compiler so
-fno-jump-table is completely irrelevant for this issue.

Fixes: cb8a2ef0848c ("LoongArch: Add ORC stack unwinder support")
Closes: https://lore.kernel.org/loongarch/Zl5m1ZlVmGKitAof@yujie-X299/
Closes: https://lore.kernel.org/loongarch/ZlY1gDDPi_mNrwJ1@slm.duckdns.org/
Closes: https://lore.kernel.org/loongarch/1717478006.038663-1-hengqi@linux.alibaba.com/
Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=816029e06768
Link: https://github.com/llvm/llvm-project/commit/42cb3c6346fc
Signed-off-by: Xi Ruoyao <xry111@xry111.site>
---
 arch/loongarch/Kconfig       | 5 ++++-
 arch/loongarch/Kconfig.debug | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index e38139c576ee..e461a6c59f15 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -143,7 +143,7 @@ config LOONGARCH
 	select HAVE_LIVEPATCH
 	select HAVE_MOD_ARCH_SPECIFIC
 	select HAVE_NMI
-	select HAVE_OBJTOOL if AS_HAS_EXPLICIT_RELOCS
+	select HAVE_OBJTOOL if AS_HAS_EXPLICIT_RELOCS && AS_HAS_THIN_ADD_SUB
 	select HAVE_PCI
 	select HAVE_PERF_EVENTS
 	select HAVE_PERF_REGS
@@ -273,6 +273,9 @@ config AS_HAS_LBT_EXTENSION
 config AS_HAS_LVZ_EXTENSION
 	def_bool $(as-instr,hvcl 0)
 
+config AS_HAS_THIN_ADD_SUB
+	def_bool $(cc-option,-Wa$(comma)-mthin-add-sub) || CC_IS_CLANG
+
 menu "Kernel type and options"
 
 source "kernel/Kconfig.hz"
diff --git a/arch/loongarch/Kconfig.debug b/arch/loongarch/Kconfig.debug
index 98d60630c3d4..8b2ce5b5d43e 100644
--- a/arch/loongarch/Kconfig.debug
+++ b/arch/loongarch/Kconfig.debug
@@ -28,6 +28,7 @@ config UNWINDER_PROLOGUE
 
 config UNWINDER_ORC
 	bool "ORC unwinder"
+	depends on HAVE_OBJTOOL
 	select OBJTOOL
 	help
 	  This option enables the ORC (Oops Rewind Capability) unwinder for
-- 
2.45.2

Re: [PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Huacai Chen]

Hi, Ruoyao,

On Tue, Jun 4, 2024 at 11:08 PM Xi Ruoyao <xry111@xry111.site> wrote:
>
> GAS <= 2.41 does not support generating R_LARCH_{32,64}_PCREL for
> "label - ." and it generates R_LARCH_{ADD,SUB}{32,64} pairs instead.
> objtool cannot handle R_LARCH_{ADD,SUB}{32,64} pair in __jump_table
> (static key implementation) and etc. so it will produce some warnings.
> This is causing the kernel CI systems to complain everywhere.
>
> For GAS we can check if -mthin-add-sub option is available to know if
> R_LARCH_{32,64}_PCREL are supported.
I think we should give a chance to toolchains without the
-mthin-add-sub option, so I hope Tiezhu can resubmit this patch to
solve this problem.
https://lore.kernel.org/loongarch/1690272910-11869-6-git-send-email-yangtiezhu@loongson.cn/

Huacai

>
> For Clang, we require Clang >= 18 and Clang >= 17 already supports
> R_LARCH_{32,64}_PCREL, so we can always assume Clang is fine for
> objtool.
>
> Note that __jump_table is not generated by the compiler so
> -fno-jump-table is completely irrelevant for this issue.
>
> Fixes: cb8a2ef0848c ("LoongArch: Add ORC stack unwinder support")
> Closes: https://lore.kernel.org/loongarch/Zl5m1ZlVmGKitAof@yujie-X299/
> Closes: https://lore.kernel.org/loongarch/ZlY1gDDPi_mNrwJ1@slm.duckdns.org/
> Closes: https://lore.kernel.org/loongarch/1717478006.038663-1-hengqi@linux.alibaba.com/
> Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=816029e06768
> Link: https://github.com/llvm/llvm-project/commit/42cb3c6346fc
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> ---
>  arch/loongarch/Kconfig       | 5 ++++-
>  arch/loongarch/Kconfig.debug | 1 +
>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> index e38139c576ee..e461a6c59f15 100644
> --- a/arch/loongarch/Kconfig
> +++ b/arch/loongarch/Kconfig
> @@ -143,7 +143,7 @@ config LOONGARCH
>         select HAVE_LIVEPATCH
>         select HAVE_MOD_ARCH_SPECIFIC
>         select HAVE_NMI
> -       select HAVE_OBJTOOL if AS_HAS_EXPLICIT_RELOCS
> +       select HAVE_OBJTOOL if AS_HAS_EXPLICIT_RELOCS && AS_HAS_THIN_ADD_SUB
>         select HAVE_PCI
>         select HAVE_PERF_EVENTS
>         select HAVE_PERF_REGS
> @@ -273,6 +273,9 @@ config AS_HAS_LBT_EXTENSION
>  config AS_HAS_LVZ_EXTENSION
>         def_bool $(as-instr,hvcl 0)
>
> +config AS_HAS_THIN_ADD_SUB
> +       def_bool $(cc-option,-Wa$(comma)-mthin-add-sub) || CC_IS_CLANG
> +
>  menu "Kernel type and options"
>
>  source "kernel/Kconfig.hz"
> diff --git a/arch/loongarch/Kconfig.debug b/arch/loongarch/Kconfig.debug
> index 98d60630c3d4..8b2ce5b5d43e 100644
> --- a/arch/loongarch/Kconfig.debug
> +++ b/arch/loongarch/Kconfig.debug
> @@ -28,6 +28,7 @@ config UNWINDER_PROLOGUE
>
>  config UNWINDER_ORC
>         bool "ORC unwinder"
> +       depends on HAVE_OBJTOOL
>         select OBJTOOL
>         help
>           This option enables the ORC (Oops Rewind Capability) unwinder for
> --
> 2.45.2
>

Re: [PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Xi Ruoyao]

On Wed, 2024-06-05 at 09:52 +0800, Huacai Chen wrote:
> Hi, Ruoyao,
> 
> On Tue, Jun 4, 2024 at 11:08 PM Xi Ruoyao <xry111@xry111.site> wrote:
> > 
> > GAS <= 2.41 does not support generating R_LARCH_{32,64}_PCREL for
> > "label - ." and it generates R_LARCH_{ADD,SUB}{32,64} pairs instead.
> > objtool cannot handle R_LARCH_{ADD,SUB}{32,64} pair in __jump_table
> > (static key implementation) and etc. so it will produce some warnings.
> > This is causing the kernel CI systems to complain everywhere.
> > 
> > For GAS we can check if -mthin-add-sub option is available to know if
> > R_LARCH_{32,64}_PCREL are supported.
> I think we should give a chance to toolchains without the
> -mthin-add-sub option, so I hope Tiezhu can resubmit this patch to
> solve this problem.
> https://lore.kernel.org/loongarch/1690272910-11869-6-git-send-email-yangtiezhu@loongson.cn/

It only handles .discard.{un,}reachable but not __jump_table. 
__jump_table is our main issue here.  And I don't see a quick way to
make -mno-thin-add-sub work for __jump_table because we really need to
resolve the relocation for __jump_table instead of simply skipping it.

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

Re: [PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Huacai Chen]

On Wed, Jun 5, 2024 at 12:38 PM Xi Ruoyao <xry111@xry111.site> wrote:
>
> On Wed, 2024-06-05 at 09:52 +0800, Huacai Chen wrote:
> > Hi, Ruoyao,
> >
> > On Tue, Jun 4, 2024 at 11:08 PM Xi Ruoyao <xry111@xry111.site> wrote:
> > >
> > > GAS <= 2.41 does not support generating R_LARCH_{32,64}_PCREL for
> > > "label - ." and it generates R_LARCH_{ADD,SUB}{32,64} pairs instead.
> > > objtool cannot handle R_LARCH_{ADD,SUB}{32,64} pair in __jump_table
> > > (static key implementation) and etc. so it will produce some warnings.
> > > This is causing the kernel CI systems to complain everywhere.
> > >
> > > For GAS we can check if -mthin-add-sub option is available to know if
> > > R_LARCH_{32,64}_PCREL are supported.
> > I think we should give a chance to toolchains without the
> > -mthin-add-sub option, so I hope Tiezhu can resubmit this patch to
> > solve this problem.
> > https://lore.kernel.org/loongarch/1690272910-11869-6-git-send-email-yangtiezhu@loongson.cn/
>
> It only handles .discard.{un,}reachable but not __jump_table.
> __jump_table is our main issue here.  And I don't see a quick way to
> make -mno-thin-add-sub work for __jump_table because we really need to
> resolve the relocation for __jump_table instead of simply skipping it.
 __jump_table is disabled now, so we can only solve  -mno-thin-add-sub
at present, and the next step is __jump_table.

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

Re: [PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Xi Ruoyao]

On Wed, 2024-06-05 at 13:21 +0800, Huacai Chen wrote:
> On Wed, Jun 5, 2024 at 12:38 PM Xi Ruoyao <xry111@xry111.site> wrote:
> > 
> > On Wed, 2024-06-05 at 09:52 +0800, Huacai Chen wrote:
> > > Hi, Ruoyao,
> > > 
> > > On Tue, Jun 4, 2024 at 11:08 PM Xi Ruoyao <xry111@xry111.site> wrote:
> > > > 
> > > > GAS <= 2.41 does not support generating R_LARCH_{32,64}_PCREL for
> > > > "label - ." and it generates R_LARCH_{ADD,SUB}{32,64} pairs instead.
> > > > objtool cannot handle R_LARCH_{ADD,SUB}{32,64} pair in __jump_table
> > > > (static key implementation) and etc. so it will produce some warnings.
> > > > This is causing the kernel CI systems to complain everywhere.
> > > > 
> > > > For GAS we can check if -mthin-add-sub option is available to know if
> > > > R_LARCH_{32,64}_PCREL are supported.
> > > I think we should give a chance to toolchains without the
> > > -mthin-add-sub option, so I hope Tiezhu can resubmit this patch to
> > > solve this problem.
> > > https://lore.kernel.org/loongarch/1690272910-11869-6-git-send-email-yangtiezhu@loongson.cn/
> > 
> > It only handles .discard.{un,}reachable but not __jump_table.
> > __jump_table is our main issue here.  And I don't see a quick way to
> > make -mno-thin-add-sub work for __jump_table because we really need to
> > resolve the relocation for __jump_table instead of simply skipping it.
>  __jump_table is disabled now, so we can only solve  -mno-thin-add-sub
> at present, and the next step is __jump_table.

No, -fno-jump-table does not disable __jump_table.  __jump_table is from
static key implementation in arch/loongarch/include/asm/jump_label.h,
not generated by the compiler.

And the problem of __jump_table is exact with -mno-thin-add-sub.  It
reads:

#define JUMP_TABLE_ENTRY                \
     ".pushsection  __jump_table, \"aw\"    \n\t"   \
     ".align    3           \n\t"   \
     ".long     1b - ., %l[l_yes] - .   \n\t"   \
     ".quad     %0 - .          \n\t"   \
     ".popsection               \n\t"

The "1b - ." and "%0 - ." things produce different relocs with -mno-
thin-add-sub and -mthin-add-sub.  The objtool can only handle the relocs
from -mthin-add-sub here.

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

Re: [PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Heng Qi]

On Tue,  4 Jun 2024 23:07:41 +0800, Xi Ruoyao <xry111@xry111.site> wrote:
> GAS <= 2.41 does not support generating R_LARCH_{32,64}_PCREL for
> "label - ." and it generates R_LARCH_{ADD,SUB}{32,64} pairs instead.
> objtool cannot handle R_LARCH_{ADD,SUB}{32,64} pair in __jump_table
> (static key implementation) and etc. so it will produce some warnings.
> This is causing the kernel CI systems to complain everywhere.
> 
> For GAS we can check if -mthin-add-sub option is available to know if
> R_LARCH_{32,64}_PCREL are supported.
> 
> For Clang, we require Clang >= 18 and Clang >= 17 already supports
> R_LARCH_{32,64}_PCREL, so we can always assume Clang is fine for
> objtool.
> 
> Note that __jump_table is not generated by the compiler so
> -fno-jump-table is completely irrelevant for this issue.
> 

Hi Xi Ruoyao,

I want to know does this patch fix the following warning:
https://lore.kernel.org/all/202406040645.6z95FW1f-lkp@intel.com/ ?

Thanks.

> Fixes: cb8a2ef0848c ("LoongArch: Add ORC stack unwinder support")
> Closes: https://lore.kernel.org/loongarch/Zl5m1ZlVmGKitAof@yujie-X299/
> Closes: https://lore.kernel.org/loongarch/ZlY1gDDPi_mNrwJ1@slm.duckdns.org/
> Closes: https://lore.kernel.org/loongarch/1717478006.038663-1-hengqi@linux.alibaba.com/
> Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=816029e06768
> Link: https://github.com/llvm/llvm-project/commit/42cb3c6346fc
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> ---
>  arch/loongarch/Kconfig       | 5 ++++-
>  arch/loongarch/Kconfig.debug | 1 +
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> index e38139c576ee..e461a6c59f15 100644
> --- a/arch/loongarch/Kconfig
> +++ b/arch/loongarch/Kconfig
> @@ -143,7 +143,7 @@ config LOONGARCH
>  	select HAVE_LIVEPATCH
>  	select HAVE_MOD_ARCH_SPECIFIC
>  	select HAVE_NMI
> -	select HAVE_OBJTOOL if AS_HAS_EXPLICIT_RELOCS
> +	select HAVE_OBJTOOL if AS_HAS_EXPLICIT_RELOCS && AS_HAS_THIN_ADD_SUB
>  	select HAVE_PCI
>  	select HAVE_PERF_EVENTS
>  	select HAVE_PERF_REGS
> @@ -273,6 +273,9 @@ config AS_HAS_LBT_EXTENSION
>  config AS_HAS_LVZ_EXTENSION
>  	def_bool $(as-instr,hvcl 0)
>  
> +config AS_HAS_THIN_ADD_SUB
> +	def_bool $(cc-option,-Wa$(comma)-mthin-add-sub) || CC_IS_CLANG
> +
>  menu "Kernel type and options"
>  
>  source "kernel/Kconfig.hz"
> diff --git a/arch/loongarch/Kconfig.debug b/arch/loongarch/Kconfig.debug
> index 98d60630c3d4..8b2ce5b5d43e 100644
> --- a/arch/loongarch/Kconfig.debug
> +++ b/arch/loongarch/Kconfig.debug
> @@ -28,6 +28,7 @@ config UNWINDER_PROLOGUE
>  
>  config UNWINDER_ORC
>  	bool "ORC unwinder"
> +	depends on HAVE_OBJTOOL
>  	select OBJTOOL
>  	help
>  	  This option enables the ORC (Oops Rewind Capability) unwinder for
> -- 
> 2.45.2
> 

Re: [PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Nathan Chancellor]

On Tue, Jun 04, 2024 at 11:07:41PM +0800, Xi Ruoyao wrote:
> GAS <= 2.41 does not support generating R_LARCH_{32,64}_PCREL for
> "label - ." and it generates R_LARCH_{ADD,SUB}{32,64} pairs instead.
> objtool cannot handle R_LARCH_{ADD,SUB}{32,64} pair in __jump_table
> (static key implementation) and etc. so it will produce some warnings.
> This is causing the kernel CI systems to complain everywhere.
> 
> For GAS we can check if -mthin-add-sub option is available to know if
> R_LARCH_{32,64}_PCREL are supported.
> 
> For Clang, we require Clang >= 18 and Clang >= 17 already supports
> R_LARCH_{32,64}_PCREL, so we can always assume Clang is fine for
> objtool.

For what it's worth, I have noticed some warnings with clang that I
don't see with GCC but I only filed an issue on our GitHub and never
followed up on the mailing list, so sorry about that.

https://github.com/ClangBuiltLinux/linux/issues/2024

Might be tangential to this patch though but I felt it was worth
mentioning.

Cheers,
Nathan

Re: [PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Xi Ruoyao]

On Tue, 2024-06-04 at 22:43 -0700, Nathan Chancellor wrote:
> On Tue, Jun 04, 2024 at 11:07:41PM +0800, Xi Ruoyao wrote:
> > GAS <= 2.41 does not support generating R_LARCH_{32,64}_PCREL for
> > "label - ." and it generates R_LARCH_{ADD,SUB}{32,64} pairs instead.
> > objtool cannot handle R_LARCH_{ADD,SUB}{32,64} pair in __jump_table
> > (static key implementation) and etc. so it will produce some warnings.
> > This is causing the kernel CI systems to complain everywhere.
> > 
> > For GAS we can check if -mthin-add-sub option is available to know if
> > R_LARCH_{32,64}_PCREL are supported.
> > 
> > For Clang, we require Clang >= 18 and Clang >= 17 already supports
> > R_LARCH_{32,64}_PCREL, so we can always assume Clang is fine for
> > objtool.
> 
> For what it's worth, I have noticed some warnings with clang that I
> don't see with GCC but I only filed an issue on our GitHub and never
> followed up on the mailing list, so sorry about that.
> 
> https://github.com/ClangBuiltLinux/linux/issues/2024
> 
> Might be tangential to this patch though but I felt it was worth
> mentioning.

The warnings in GCC build is definitely the issue handled by this patch.
But the warnings in Clang build should be a different issue.  Can you
attach the kernel/events/core.o file from the Clang build for analysis?
I guess we need to disable more optimization...

I personally hate "pessimizing the code generation just for debugging"
with a passion so I never enabled objtool on LoongArch, thus I didn't
notice the issue.

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

Re: [PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Nathan Chancellor]

[-- Attachment #1: Type: text/plain, Size: 836 bytes --]

On Wed, Jun 05, 2024 at 01:54:24PM +0800, Xi Ruoyao wrote:
> On Tue, 2024-06-04 at 22:43 -0700, Nathan Chancellor wrote:
> > For what it's worth, I have noticed some warnings with clang that I
> > don't see with GCC but I only filed an issue on our GitHub and never
> > followed up on the mailing list, so sorry about that.
> > 
> > https://github.com/ClangBuiltLinux/linux/issues/2024
> > 
> > Might be tangential to this patch though but I felt it was worth
> > mentioning.
> 
> The warnings in GCC build is definitely the issue handled by this patch.
> But the warnings in Clang build should be a different issue.  Can you
> attach the kernel/events/core.o file from the Clang build for analysis?
> I guess we need to disable more optimization...

Sure thing. Let me know if there are any issues with the attachment.

Cheers,
Nathan

[-- Attachment #2: core.o --]
[-- Type: application/octet-stream, Size: 270384 bytes --]

Re: [PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Xi Ruoyao]

On Tue, 2024-06-04 at 23:25 -0700, Nathan Chancellor wrote:
> On Wed, Jun 05, 2024 at 01:54:24PM +0800, Xi Ruoyao wrote:
> > On Tue, 2024-06-04 at 22:43 -0700, Nathan Chancellor wrote:
> > > For what it's worth, I have noticed some warnings with clang that I
> > > don't see with GCC but I only filed an issue on our GitHub and never
> > > followed up on the mailing list, so sorry about that.
> > > 
> > > https://github.com/ClangBuiltLinux/linux/issues/2024
> > > 
> > > Might be tangential to this patch though but I felt it was worth
> > > mentioning.
> > 
> > The warnings in GCC build is definitely the issue handled by this patch.
> > But the warnings in Clang build should be a different issue.  Can you
> > attach the kernel/events/core.o file from the Clang build for analysis?
> > I guess we need to disable more optimization...
> 
> Sure thing. Let me know if there are any issues with the attachment.

Thanks!  I've simplified it and now even...

.global test
.type test,@function
test:

addi.d	$sp,$sp,-448
st.d	$ra,$sp,440
st.d	$fp,$sp,432
addi.d	$fp,$sp,448

# do something

addi.d	$sp,$fp,-448
ld.d	$fp,$sp,432
ld.d	$ra,$sp,440
addi.d	$sp,$sp,448
ret

.size test,.-test

is enough to trigger a objtool warning:

/home/xry111/t1.o: warning: objtool: test+0x20: return with modified stack frame

And to me this warning is bogus?

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

Re: [PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Xi Ruoyao]

On Wed, 2024-06-05 at 18:57 +0800, Xi Ruoyao wrote:
> On Tue, 2024-06-04 at 23:25 -0700, Nathan Chancellor wrote:
> > On Wed, Jun 05, 2024 at 01:54:24PM +0800, Xi Ruoyao wrote:
> > > On Tue, 2024-06-04 at 22:43 -0700, Nathan Chancellor wrote:
> > > > For what it's worth, I have noticed some warnings with clang that I
> > > > don't see with GCC but I only filed an issue on our GitHub and never
> > > > followed up on the mailing list, so sorry about that.
> > > > 
> > > > https://github.com/ClangBuiltLinux/linux/issues/2024
> > > > 
> > > > Might be tangential to this patch though but I felt it was worth
> > > > mentioning.
> > > 
> > > The warnings in GCC build is definitely the issue handled by this patch.
> > > But the warnings in Clang build should be a different issue.  Can you
> > > attach the kernel/events/core.o file from the Clang build for analysis?
> > > I guess we need to disable more optimization...
> > 
> > Sure thing. Let me know if there are any issues with the attachment.
> 
> Thanks!  I've simplified it and now even...
> 
> .global test
> .type test,@function
> test:
> 
> addi.d	$sp,$sp,-448
> st.d	$ra,$sp,440
> st.d	$fp,$sp,432
> addi.d	$fp,$sp,448
> 
> # do something
> 
> addi.d	$sp,$fp,-448
> ld.d	$fp,$sp,432
> ld.d	$ra,$sp,440
> addi.d	$sp,$sp,448
> ret
> 
> .size test,.-test
> 
> is enough to trigger a objtool warning:
> 
> /home/xry111/t1.o: warning: objtool: test+0x20: return with modified stack frame
> 
> And to me this warning is bogus?

Minimal C reproducer:

struct x { _Alignas(64) char buf[128]; };

void f(struct x *p);
void g()
{
	struct x x = { .buf = "1145141919810" };
	f(&x);
}

Then objtool is unhappy to the object file produced with "clang -c -O2"
from this translation unit:

/home/xry111/t2.o: warning: objtool: g+0x50: return with modified stack frame

It seems CFI_BP has a very specific semantic in objtool and Clang does
not operates $fp in the expected way.  I'm not sure about my conclusion
though.  Maybe Peter can explain it better.

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

Re: [PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Xi Ruoyao]

On Wed, 2024-06-05 at 21:18 +0800, Xi Ruoyao wrote:
> On Wed, 2024-06-05 at 18:57 +0800, Xi Ruoyao wrote:
> > On Tue, 2024-06-04 at 23:25 -0700, Nathan Chancellor wrote:
> > > On Wed, Jun 05, 2024 at 01:54:24PM +0800, Xi Ruoyao wrote:
> > > > On Tue, 2024-06-04 at 22:43 -0700, Nathan Chancellor wrote:
> > > > > For what it's worth, I have noticed some warnings with clang that I
> > > > > don't see with GCC but I only filed an issue on our GitHub and never
> > > > > followed up on the mailing list, so sorry about that.
> > > > > 
> > > > > https://github.com/ClangBuiltLinux/linux/issues/2024
> > > > > 
> > > > > Might be tangential to this patch though but I felt it was worth
> > > > > mentioning.
> > > > 
> > > > The warnings in GCC build is definitely the issue handled by this patch.
> > > > But the warnings in Clang build should be a different issue.  Can you
> > > > attach the kernel/events/core.o file from the Clang build for analysis?
> > > > I guess we need to disable more optimization...
> > > 
> > > Sure thing. Let me know if there are any issues with the attachment.
> > 
> > Thanks!  I've simplified it and now even...
> > 
> > .global test
> > .type test,@function
> > test:
> > 
> > addi.d	$sp,$sp,-448
> > st.d	$ra,$sp,440
> > st.d	$fp,$sp,432
> > addi.d	$fp,$sp,448
> > 
> > # do something
> > 
> > addi.d	$sp,$fp,-448
> > ld.d	$fp,$sp,432
> > ld.d	$ra,$sp,440
> > addi.d	$sp,$sp,448
> > ret
> > 
> > .size test,.-test
> > 
> > is enough to trigger a objtool warning:
> > 
> > /home/xry111/t1.o: warning: objtool: test+0x20: return with modified stack frame
> > 
> > And to me this warning is bogus?
> 
> Minimal C reproducer:
> 
> struct x { _Alignas(64) char buf[128]; };
> 
> void f(struct x *p);
> void g()
> {
> 	struct x x = { .buf = "1145141919810" };
> 	f(&x);
> }
> 
> Then objtool is unhappy to the object file produced with "clang -c -O2"
> from this translation unit:
> 
> /home/xry111/t2.o: warning: objtool: g+0x50: return with modified stack frame
> 
> It seems CFI_BP has a very specific semantic in objtool and Clang does
> not operates $fp in the expected way.  I'm not sure about my conclusion
> though.  Maybe Peter can explain it better.

Another example: some simple rust code:

extern { fn f(x: &i64) -> i64; }

#[no_mangle]
fn g() -> i64 {
    let x = 114514;
    unsafe {f(&x)}
}

It's just lucky GCC doesn't use $fp as the frame pointer unless there's
some stupid code (VLA etc) thus the issue was not detected.


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

Re: [PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Jinyang He]

[-- Attachment #1: Type: text/plain, Size: 4618 bytes --]

On 2024-06-05 23:13, Xi Ruoyao wrote:

> On Wed, 2024-06-05 at 21:18 +0800, Xi Ruoyao wrote:
>> On Wed, 2024-06-05 at 18:57 +0800, Xi Ruoyao wrote:
>>> On Tue, 2024-06-04 at 23:25 -0700, Nathan Chancellor wrote:
>>>> On Wed, Jun 05, 2024 at 01:54:24PM +0800, Xi Ruoyao wrote:
>>>>> On Tue, 2024-06-04 at 22:43 -0700, Nathan Chancellor wrote:
>>>>>> For what it's worth, I have noticed some warnings with clang that I
>>>>>> don't see with GCC but I only filed an issue on our GitHub and never
>>>>>> followed up on the mailing list, so sorry about that.
>>>>>>
>>>>>> https://github.com/ClangBuiltLinux/linux/issues/2024
>>>>>>
>>>>>> Might be tangential to this patch though but I felt it was worth
>>>>>> mentioning.
>>>>> The warnings in GCC build is definitely the issue handled by this patch.
>>>>> But the warnings in Clang build should be a different issue.  Can you
>>>>> attach the kernel/events/core.o file from the Clang build for analysis?
>>>>> I guess we need to disable more optimization...
>>>> Sure thing. Let me know if there are any issues with the attachment.
>>> Thanks!  I've simplified it and now even...
>>>
>>> .global test
>>> .type test,@function
>>> test:
>>>
>>> addi.d	$sp,$sp,-448
>>> st.d	$ra,$sp,440
>>> st.d	$fp,$sp,432
>>> addi.d	$fp,$sp,448
>>>
>>> # do something
>>>
>>> addi.d	$sp,$fp,-448
>>> ld.d	$fp,$sp,432
>>> ld.d	$ra,$sp,440
>>> addi.d	$sp,$sp,448
>>> ret
>>>
>>> .size test,.-test
>>>
>>> is enough to trigger a objtool warning:
>>>
>>> /home/xry111/t1.o: warning: objtool: test+0x20: return with modified stack frame
>>>
>>> And to me this warning is bogus?
>> Minimal C reproducer:
>>
>> struct x { _Alignas(64) char buf[128]; };
>>
>> void f(struct x *p);
>> void g()
>> {
>> 	struct x x = { .buf = "1145141919810" };
>> 	f(&x);
>> }
>>
>> Then objtool is unhappy to the object file produced with "clang -c -O2"
>> from this translation unit:
>>
>> /home/xry111/t2.o: warning: objtool: g+0x50: return with modified stack frame
>>
>> It seems CFI_BP has a very specific semantic in objtool and Clang does
>> not operates $fp in the expected way.  I'm not sure about my conclusion
>> though.  Maybe Peter can explain it better.
> Another example: some simple rust code:
>
> extern { fn f(x: &i64) -> i64; }
>
> #[no_mangle]
> fn g() -> i64 {
>      let x = 114514;
>      unsafe {f(&x)}
> }
>
> It's just lucky GCC doesn't use $fp as the frame pointer unless there's
> some stupid code (VLA etc) thus the issue was not detected.
>
>
I think this because we did not add arch special handle in 
update_cfi_state().
In our 419 repo this func has been renamed arch_update_insn_state (, now it
should be arch_update_cfi_state) and give some actions to deal with the
frame pointer case. If we support it we may deal with some case but for 
clang...

.global test
.type test,@function
test:

addi.d  $sp,$sp,-448
st.d    $ra,$sp,440
st.d    $fp,$sp,432
addi.d  $fp,$sp,448

# do something  <- not change $sp

# addi.d        $sp,$fp,-448  <- not restore sp from cfa(fp)
ld.d    $fp,$sp,432
ld.d    $ra,$sp,440
addi.d  $sp,$sp,448
ret

.size test,.-test

This will cause warning, too. (Gcc should be ok, I don't test.)

source.c: (clang 19, based 0c1c0d53931, -g)
void *foo() { return __builtin_frame_address(0); }

asm.s:
         .cfi_sections .debug_frame
         .cfi_startproc
# %bb.0:
         addi.d  $sp, $sp, -16
         .cfi_def_cfa_offset 16
         st.d    $ra, $sp, 8                     # 8-byte Folded Spill
         st.d    $fp, $sp, 0                     # 8-byte Folded Spill
         .cfi_offset 1, -8
         .cfi_offset 22, -16
         addi.d  $fp, $sp, 16
         .cfi_def_cfa 22, 0                    <- define cfa is $r22
.Ltmp0:
         .loc    0 1 22 prologue_end             # hello.c:1:22
         move    $a0, $fp
         ld.d    $fp, $sp, 0                     # 8-byte Folded Reload  
<- restore regs from non-cfa ?
         ld.d    $ra, $sp, 8                     # 8-byte Folded Reload  
<- restore regs from non-cfa ?
         .loc    0 1 15 epilogue_begin is_stmt 0 # hello.c:1:15
         addi.d  $sp, $sp, 16                                            
<- restore prev-sp from non-cfa ?
         ret

Maybe Clang have anything wrong?


Attach: tmp fix by support arch_update_insn_state.


Thanks,

Jinyang


[-- Attachment #2: 0001-tmp-fix.patch --]
[-- Type: text/x-patch, Size: 30644 bytes --]

From 98d31f806c018a0c5f5fe72e13bf5d107d94d93b Mon Sep 17 00:00:00 2001
From: Jinyang He <hejinyang@loongson.cn>
Date: Wed, 5 Jun 2024 21:26:36 +0800
Subject: [PATCH] tmp fix

---
 tools/objtool/arch/loongarch/decode.c | 111 ++++++
 tools/objtool/arch/x86/decode.c       | 507 ++++++++++++++++++++++++++
 tools/objtool/check.c                 | 505 +------------------------
 tools/objtool/include/objtool/arch.h  |   4 +
 4 files changed, 623 insertions(+), 504 deletions(-)

diff --git a/tools/objtool/arch/loongarch/decode.c b/tools/objtool/arch/loongarch/decode.c
index aee479d2191c..ce4f1c140308 100644
--- a/tools/objtool/arch/loongarch/decode.c
+++ b/tools/objtool/arch/loongarch/decode.c
@@ -354,3 +354,114 @@ void arch_initial_func_cfi_state(struct cfi_init_state *state)
 	state->cfa.base = CFI_SP;
 	state->cfa.offset = 0;
 }
+
+static int update_cfi_state_regs(struct instruction *insn,
+				 struct cfi_state *cfi,
+				 struct stack_op *op)
+{
+	struct cfi_reg *cfa = &cfi->cfa;
+
+	if (cfa->base != CFI_SP && cfa->base != CFI_SP_INDIRECT)
+		return 0;
+
+	/* addi.d sp, sp, imm */
+	if (op->dest.type == OP_DEST_REG && op->src.type == OP_SRC_ADD &&
+	    op->dest.reg == CFI_SP && op->src.reg == CFI_SP)
+		cfa->offset -= op->src.offset;
+
+	return 0;
+}
+
+static void save_reg(struct cfi_state *cfi, unsigned char reg, int base,
+		     int offset)
+{
+	if (arch_callee_saved_reg(reg) &&
+	    cfi->regs[reg].base == CFI_UNDEFINED) {
+		cfi->regs[reg].base = base;
+		cfi->regs[reg].offset = offset;
+	}
+}
+
+static void restore_reg(struct cfi_state *cfi, unsigned char reg)
+{
+	cfi->regs[reg].base = CFI_UNDEFINED;
+	cfi->regs[reg].offset = 0;
+}
+
+int arch_update_cfi_state(struct instruction *insn,
+			  struct instruction *next_insn,
+			  struct cfi_state *cfi, struct stack_op *op)
+{
+	struct cfi_reg *cfa = &cfi->cfa;
+	struct cfi_reg *regs = cfi->regs;
+
+	/* ignore UNWIND_HINT_UNDEFINED regions */
+	if (cfi->force_undefined)
+		return 0;
+
+	if (cfa->base == CFI_UNDEFINED) {
+		if (insn_func(insn)) {
+			WARN_INSN(insn, "undefined stack state");
+			return -1;
+		}
+		return 0;
+	}
+
+	if (cfi->type == UNWIND_HINT_TYPE_REGS)
+		return update_cfi_state_regs(insn, cfi, op);
+
+	switch (op->dest.type) {
+	case OP_DEST_REG:
+		switch (op->src.type) {
+		case OP_SRC_ADD:
+			if (op->dest.reg == CFI_SP && op->src.reg == CFI_SP) {
+				/* addi.d sp, sp, imm */
+				cfi->stack_size -= op->src.offset;
+				if (cfa->base == CFI_SP)
+				/* addi.d sp, sp, imm */
+				cfi->stack_size -= op->src.offset;
+				if (cfa->base == CFI_SP)
+					cfa->offset -= op->src.offset;
+			} else if (op->dest.reg == CFI_FP && op->src.reg == CFI_SP) {
+				/* addi.d fp, sp, imm */
+				if (cfa->base == CFI_SP && cfa->offset == op->src.offset) {
+					cfa->base = CFI_FP;
+					cfa->offset = 0;
+				}
+			} else if (op->dest.reg == CFI_SP && op->src.reg == CFI_FP) {
+				/* addi.d sp, fp, imm */
+				if (cfa->base == CFI_FP && cfa->offset == 0) {
+					cfa->base = CFI_SP;
+					cfa->offset = -op->src.offset;
+				}
+			}
+			break;
+		case OP_SRC_REG_INDIRECT:
+			/* ld.d _reg, sp, imm */
+			if (op->src.reg == CFI_SP &&
+				op->src.offset == (regs[op->dest.reg].offset + cfi->stack_size)) {
+				restore_reg(cfi, op->dest.reg);
+				/* Gcc may not restore sp, we adjust it directly. */
+				if (cfa->base == CFI_FP && cfa->offset == 0) {
+					cfa->base = CFI_SP;
+					cfa->offset = cfi->stack_size;
+				}
+			}
+			break;
+		default:
+			break;
+		}
+		break;
+	case OP_DEST_REG_INDIRECT:
+		if (op->src.type == OP_SRC_REG) {
+			/* st.d _reg, sp, imm */
+			save_reg(cfi, op->src.reg, CFI_CFA, op->dest.offset - cfi->stack_size);
+		}
+		break;
+	default:
+		WARN_FUNC("unknown stack-related instruction", insn->sec, insn->offset);
+		return -1;
+	}
+
+	return 0;
+}
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index 3a1d80a7878d..848d890a3cbf 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -841,3 +841,510 @@ bool arch_is_embedded_insn(struct symbol *sym)
 	return !strcmp(sym->name, "retbleed_return_thunk") ||
 	       !strcmp(sym->name, "srso_safe_ret");
 }
+
+static int update_cfi_state_regs(struct instruction *insn,
+				  struct cfi_state *cfi,
+				  struct stack_op *op)
+{
+	struct cfi_reg *cfa = &cfi->cfa;
+
+	if (cfa->base != CFI_SP && cfa->base != CFI_SP_INDIRECT)
+		return 0;
+
+	/* push */
+	if (op->dest.type == OP_DEST_PUSH || op->dest.type == OP_DEST_PUSHF)
+		cfa->offset += 8;
+
+	/* pop */
+	if (op->src.type == OP_SRC_POP || op->src.type == OP_SRC_POPF)
+		cfa->offset -= 8;
+
+	/* add immediate to sp */
+	if (op->dest.type == OP_DEST_REG && op->src.type == OP_SRC_ADD &&
+	    op->dest.reg == CFI_SP && op->src.reg == CFI_SP)
+		cfa->offset -= op->src.offset;
+
+	return 0;
+}
+
+static void save_reg(struct cfi_state *cfi, unsigned char reg, int base, int offset)
+{
+	if (arch_callee_saved_reg(reg) &&
+	    cfi->regs[reg].base == CFI_UNDEFINED) {
+		cfi->regs[reg].base = base;
+		cfi->regs[reg].offset = offset;
+	}
+}
+
+static void restore_reg(struct cfi_state *cfi, unsigned char reg)
+{
+	cfi->regs[reg].base = initial_func_cfi.regs[reg].base;
+	cfi->regs[reg].offset = initial_func_cfi.regs[reg].offset;
+}
+
+/*
+ * A note about DRAP stack alignment:
+ *
+ * GCC has the concept of a DRAP register, which is used to help keep track of
+ * the stack pointer when aligning the stack.  r10 or r13 is used as the DRAP
+ * register.  The typical DRAP pattern is:
+ *
+ *   4c 8d 54 24 08		lea    0x8(%rsp),%r10
+ *   48 83 e4 c0		and    $0xffffffffffffffc0,%rsp
+ *   41 ff 72 f8		pushq  -0x8(%r10)
+ *   55				push   %rbp
+ *   48 89 e5			mov    %rsp,%rbp
+ *				(more pushes)
+ *   41 52			push   %r10
+ *				...
+ *   41 5a			pop    %r10
+ *				(more pops)
+ *   5d				pop    %rbp
+ *   49 8d 62 f8		lea    -0x8(%r10),%rsp
+ *   c3				retq
+ *
+ * There are some variations in the epilogues, like:
+ *
+ *   5b				pop    %rbx
+ *   41 5a			pop    %r10
+ *   41 5c			pop    %r12
+ *   41 5d			pop    %r13
+ *   41 5e			pop    %r14
+ *   c9				leaveq
+ *   49 8d 62 f8		lea    -0x8(%r10),%rsp
+ *   c3				retq
+ *
+ * and:
+ *
+ *   4c 8b 55 e8		mov    -0x18(%rbp),%r10
+ *   48 8b 5d e0		mov    -0x20(%rbp),%rbx
+ *   4c 8b 65 f0		mov    -0x10(%rbp),%r12
+ *   4c 8b 6d f8		mov    -0x8(%rbp),%r13
+ *   c9				leaveq
+ *   49 8d 62 f8		lea    -0x8(%r10),%rsp
+ *   c3				retq
+ *
+ * Sometimes r13 is used as the DRAP register, in which case it's saved and
+ * restored beforehand:
+ *
+ *   41 55			push   %r13
+ *   4c 8d 6c 24 10		lea    0x10(%rsp),%r13
+ *   48 83 e4 f0		and    $0xfffffffffffffff0,%rsp
+ *				...
+ *   49 8d 65 f0		lea    -0x10(%r13),%rsp
+ *   41 5d			pop    %r13
+ *   c3				retq
+ */
+int arch_update_cfi_state(struct instruction *insn,
+                            struct instruction *next_insn,
+                            struct cfi_state *cfi, struct stack_op *op)
+{
+	struct cfi_reg *cfa = &cfi->cfa;
+	struct cfi_reg *regs = cfi->regs;
+
+	/* ignore UNWIND_HINT_UNDEFINED regions */
+	if (cfi->force_undefined)
+		return 0;
+
+	/* stack operations don't make sense with an undefined CFA */
+	if (cfa->base == CFI_UNDEFINED) {
+		if (insn_func(insn)) {
+			WARN_INSN(insn, "undefined stack state");
+			return -1;
+		}
+		return 0;
+	}
+
+	if (cfi->type == UNWIND_HINT_TYPE_REGS ||
+	    cfi->type == UNWIND_HINT_TYPE_REGS_PARTIAL)
+		return update_cfi_state_regs(insn, cfi, op);
+
+	switch (op->dest.type) {
+
+	case OP_DEST_REG:
+		switch (op->src.type) {
+
+		case OP_SRC_REG:
+			if (op->src.reg == CFI_SP && op->dest.reg == CFI_BP &&
+			    cfa->base == CFI_SP &&
+			    check_reg_frame_pos(&regs[CFI_BP], -cfa->offset)) {
+
+				/* mov %rsp, %rbp */
+				cfa->base = op->dest.reg;
+				cfi->bp_scratch = false;
+			}
+
+			else if (op->src.reg == CFI_SP &&
+				 op->dest.reg == CFI_BP && cfi->drap) {
+
+				/* drap: mov %rsp, %rbp */
+				regs[CFI_BP].base = CFI_BP;
+				regs[CFI_BP].offset = -cfi->stack_size;
+				cfi->bp_scratch = false;
+			}
+
+			else if (op->src.reg == CFI_SP && cfa->base == CFI_SP) {
+
+				/*
+				 * mov %rsp, %reg
+				 *
+				 * This is needed for the rare case where GCC
+				 * does:
+				 *
+				 *   mov    %rsp, %rax
+				 *   ...
+				 *   mov    %rax, %rsp
+				 */
+				cfi->vals[op->dest.reg].base = CFI_CFA;
+				cfi->vals[op->dest.reg].offset = -cfi->stack_size;
+			}
+
+			else if (op->src.reg == CFI_BP && op->dest.reg == CFI_SP &&
+				 (cfa->base == CFI_BP || cfa->base == cfi->drap_reg)) {
+
+				/*
+				 * mov %rbp, %rsp
+				 *
+				 * Restore the original stack pointer (Clang).
+				 */
+				cfi->stack_size = -cfi->regs[CFI_BP].offset;
+			}
+
+			else if (op->dest.reg == cfa->base) {
+
+				/* mov %reg, %rsp */
+				if (cfa->base == CFI_SP &&
+				    cfi->vals[op->src.reg].base == CFI_CFA) {
+
+					/*
+					 * This is needed for the rare case
+					 * where GCC does something dumb like:
+					 *
+					 *   lea    0x8(%rsp), %rcx
+					 *   ...
+					 *   mov    %rcx, %rsp
+					 */
+					cfa->offset = -cfi->vals[op->src.reg].offset;
+					cfi->stack_size = cfa->offset;
+
+				} else if (cfa->base == CFI_SP &&
+					   cfi->vals[op->src.reg].base == CFI_SP_INDIRECT &&
+					   cfi->vals[op->src.reg].offset == cfa->offset) {
+
+					/*
+					 * Stack swizzle:
+					 *
+					 * 1: mov %rsp, (%[tos])
+					 * 2: mov %[tos], %rsp
+					 *    ...
+					 * 3: pop %rsp
+					 *
+					 * Where:
+					 *
+					 * 1 - places a pointer to the previous
+					 *     stack at the Top-of-Stack of the
+					 *     new stack.
+					 *
+					 * 2 - switches to the new stack.
+					 *
+					 * 3 - pops the Top-of-Stack to restore
+					 *     the original stack.
+					 *
+					 * Note: we set base to SP_INDIRECT
+					 * here and preserve offset. Therefore
+					 * when the unwinder reaches ToS it
+					 * will dereference SP and then add the
+					 * offset to find the next frame, IOW:
+					 * (%rsp) + offset.
+					 */
+					cfa->base = CFI_SP_INDIRECT;
+
+				} else {
+					cfa->base = CFI_UNDEFINED;
+					cfa->offset = 0;
+				}
+			}
+
+			else if (op->dest.reg == CFI_SP &&
+				 cfi->vals[op->src.reg].base == CFI_SP_INDIRECT &&
+				 cfi->vals[op->src.reg].offset == cfa->offset) {
+
+				/*
+				 * The same stack swizzle case 2) as above. But
+				 * because we can't change cfa->base, case 3)
+				 * will become a regular POP. Pretend we're a
+				 * PUSH so things don't go unbalanced.
+				 */
+				cfi->stack_size += 8;
+			}
+
+
+			break;
+
+		case OP_SRC_ADD:
+			if (op->dest.reg == CFI_SP && op->src.reg == CFI_SP) {
+
+				/* add imm, %rsp */
+				cfi->stack_size -= op->src.offset;
+				if (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 */
+				cfi->stack_size = -(op->src.offset + regs[CFI_BP].offset);
+				break;
+			}
+
+			if (op->src.reg == CFI_SP && cfa->base == CFI_SP) {
+
+				/* drap: lea disp(%rsp), %drap */
+				cfi->drap_reg = op->dest.reg;
+
+				/*
+				 * lea disp(%rsp), %reg
+				 *
+				 * This is needed for the rare case where GCC
+				 * does something dumb like:
+				 *
+				 *   lea    0x8(%rsp), %rcx
+				 *   ...
+				 *   mov    %rcx, %rsp
+				 */
+				cfi->vals[op->dest.reg].base = CFI_CFA;
+				cfi->vals[op->dest.reg].offset = \
+					-cfi->stack_size + op->src.offset;
+
+				break;
+			}
+
+			if (cfi->drap && op->dest.reg == CFI_SP &&
+			    op->src.reg == cfi->drap_reg) {
+
+				 /* drap: lea disp(%drap), %rsp */
+				cfa->base = CFI_SP;
+				cfa->offset = cfi->stack_size = -op->src.offset;
+				cfi->drap_reg = CFI_UNDEFINED;
+				cfi->drap = false;
+				break;
+			}
+
+			if (op->dest.reg == cfi->cfa.base && !(next_insn && next_insn->hint)) {
+				WARN_INSN(insn, "unsupported stack register modification");
+				return -1;
+			}
+
+			break;
+
+		case OP_SRC_AND:
+			if (op->dest.reg != CFI_SP ||
+			    (cfi->drap_reg != CFI_UNDEFINED && cfa->base != CFI_SP) ||
+			    (cfi->drap_reg == CFI_UNDEFINED && cfa->base != CFI_BP)) {
+				WARN_INSN(insn, "unsupported stack pointer realignment");
+				return -1;
+			}
+
+			if (cfi->drap_reg != CFI_UNDEFINED) {
+				/* drap: and imm, %rsp */
+				cfa->base = cfi->drap_reg;
+				cfa->offset = cfi->stack_size = 0;
+				cfi->drap = true;
+			}
+
+			/*
+			 * Older versions of GCC (4.8ish) realign the stack
+			 * without DRAP, with a frame pointer.
+			 */
+
+			break;
+
+		case OP_SRC_POP:
+		case OP_SRC_POPF:
+			if (op->dest.reg == CFI_SP && cfa->base == CFI_SP_INDIRECT) {
+
+				/* pop %rsp; # restore from a stack swizzle */
+				cfa->base = CFI_SP;
+				break;
+			}
+
+			if (!cfi->drap && op->dest.reg == cfa->base) {
+
+				/* pop %rbp */
+				cfa->base = CFI_SP;
+			}
+
+			if (cfi->drap && cfa->base == CFI_BP_INDIRECT &&
+			    op->dest.reg == cfi->drap_reg &&
+			    cfi->drap_offset == -cfi->stack_size) {
+
+				/* drap: pop %drap */
+				cfa->base = cfi->drap_reg;
+				cfa->offset = 0;
+				cfi->drap_offset = -1;
+
+			} else if (cfi->stack_size == -regs[op->dest.reg].offset) {
+
+				/* pop %reg */
+				restore_reg(cfi, op->dest.reg);
+			}
+
+			cfi->stack_size -= 8;
+			if (cfa->base == CFI_SP)
+				cfa->offset -= 8;
+
+			break;
+
+		case OP_SRC_REG_INDIRECT:
+			if (!cfi->drap && op->dest.reg == cfa->base &&
+			    op->dest.reg == CFI_BP) {
+
+				/* mov disp(%rsp), %rbp */
+				cfa->base = CFI_SP;
+				cfa->offset = cfi->stack_size;
+			}
+
+			if (cfi->drap && op->src.reg == CFI_BP &&
+			    op->src.offset == cfi->drap_offset) {
+
+				/* drap: mov disp(%rbp), %drap */
+				cfa->base = cfi->drap_reg;
+				cfa->offset = 0;
+				cfi->drap_offset = -1;
+			}
+
+			if (cfi->drap && op->src.reg == CFI_BP &&
+			    op->src.offset == regs[op->dest.reg].offset) {
+
+				/* drap: mov disp(%rbp), %reg */
+				restore_reg(cfi, op->dest.reg);
+
+			} else if (op->src.reg == cfa->base &&
+			    op->src.offset == regs[op->dest.reg].offset + cfa->offset) {
+
+				/* mov disp(%rbp), %reg */
+				/* mov disp(%rsp), %reg */
+				restore_reg(cfi, op->dest.reg);
+
+			} else if (op->src.reg == CFI_SP &&
+				   op->src.offset == regs[op->dest.reg].offset + cfi->stack_size) {
+
+				/* mov disp(%rsp), %reg */
+				restore_reg(cfi, op->dest.reg);
+			}
+
+			break;
+
+		default:
+			WARN_INSN(insn, "unknown stack-related instruction");
+			return -1;
+		}
+
+		break;
+
+	case OP_DEST_PUSH:
+	case OP_DEST_PUSHF:
+		cfi->stack_size += 8;
+		if (cfa->base == CFI_SP)
+			cfa->offset += 8;
+
+		if (op->src.type != OP_SRC_REG)
+			break;
+
+		if (cfi->drap) {
+			if (op->src.reg == cfa->base && op->src.reg == cfi->drap_reg) {
+
+				/* drap: push %drap */
+				cfa->base = CFI_BP_INDIRECT;
+				cfa->offset = -cfi->stack_size;
+
+				/* save drap so we know when to restore it */
+				cfi->drap_offset = -cfi->stack_size;
+
+			} else if (op->src.reg == CFI_BP && cfa->base == cfi->drap_reg) {
+
+				/* drap: push %rbp */
+				cfi->stack_size = 0;
+
+			} else {
+
+				/* drap: push %reg */
+				save_reg(cfi, op->src.reg, CFI_BP, -cfi->stack_size);
+			}
+
+		} else {
+
+			/* push %reg */
+			save_reg(cfi, op->src.reg, CFI_CFA, -cfi->stack_size);
+		}
+
+		/* detect when asm code uses rbp as a scratch register */
+		if (opts.stackval && insn_func(insn) && op->src.reg == CFI_BP &&
+		    cfa->base != CFI_BP)
+			cfi->bp_scratch = true;
+		break;
+
+	case OP_DEST_REG_INDIRECT:
+
+		if (cfi->drap) {
+			if (op->src.reg == cfa->base && op->src.reg == cfi->drap_reg) {
+
+				/* drap: mov %drap, disp(%rbp) */
+				cfa->base = CFI_BP_INDIRECT;
+				cfa->offset = op->dest.offset;
+
+				/* save drap offset so we know when to restore it */
+				cfi->drap_offset = op->dest.offset;
+			} else {
+
+				/* drap: mov reg, disp(%rbp) */
+				save_reg(cfi, op->src.reg, CFI_BP, op->dest.offset);
+			}
+
+		} else if (op->dest.reg == cfa->base) {
+
+			/* mov reg, disp(%rbp) */
+			/* mov reg, disp(%rsp) */
+			save_reg(cfi, op->src.reg, CFI_CFA,
+				 op->dest.offset - cfi->cfa.offset);
+
+		} else if (op->dest.reg == CFI_SP) {
+
+			/* mov reg, disp(%rsp) */
+			save_reg(cfi, op->src.reg, CFI_CFA,
+				 op->dest.offset - cfi->stack_size);
+
+		} else if (op->src.reg == CFI_SP && op->dest.offset == 0) {
+
+			/* mov %rsp, (%reg); # setup a stack swizzle. */
+			cfi->vals[op->dest.reg].base = CFI_SP_INDIRECT;
+			cfi->vals[op->dest.reg].offset = cfa->offset;
+		}
+
+		break;
+
+	case OP_DEST_MEM:
+		if (op->src.type != OP_SRC_POP && op->src.type != OP_SRC_POPF) {
+			WARN_INSN(insn, "unknown stack-related memory operation");
+			return -1;
+		}
+
+		/* pop mem */
+		cfi->stack_size -= 8;
+		if (cfa->base == CFI_SP)
+			cfa->offset -= 8;
+
+		break;
+
+	default:
+		WARN_INSN(insn, "unknown stack-related instruction");
+		return -1;
+	}
+
+	return 0;
+}
+
+
+
+
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 703f99c5a80d..1fe7e21a75be 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2753,509 +2753,6 @@ static bool has_valid_stack_frame(struct insn_state *state)
 	return false;
 }
 
-static int update_cfi_state_regs(struct instruction *insn,
-				  struct cfi_state *cfi,
-				  struct stack_op *op)
-{
-	struct cfi_reg *cfa = &cfi->cfa;
-
-	if (cfa->base != CFI_SP && cfa->base != CFI_SP_INDIRECT)
-		return 0;
-
-	/* push */
-	if (op->dest.type == OP_DEST_PUSH || op->dest.type == OP_DEST_PUSHF)
-		cfa->offset += 8;
-
-	/* pop */
-	if (op->src.type == OP_SRC_POP || op->src.type == OP_SRC_POPF)
-		cfa->offset -= 8;
-
-	/* add immediate to sp */
-	if (op->dest.type == OP_DEST_REG && op->src.type == OP_SRC_ADD &&
-	    op->dest.reg == CFI_SP && op->src.reg == CFI_SP)
-		cfa->offset -= op->src.offset;
-
-	return 0;
-}
-
-static void save_reg(struct cfi_state *cfi, unsigned char reg, int base, int offset)
-{
-	if (arch_callee_saved_reg(reg) &&
-	    cfi->regs[reg].base == CFI_UNDEFINED) {
-		cfi->regs[reg].base = base;
-		cfi->regs[reg].offset = offset;
-	}
-}
-
-static void restore_reg(struct cfi_state *cfi, unsigned char reg)
-{
-	cfi->regs[reg].base = initial_func_cfi.regs[reg].base;
-	cfi->regs[reg].offset = initial_func_cfi.regs[reg].offset;
-}
-
-/*
- * A note about DRAP stack alignment:
- *
- * GCC has the concept of a DRAP register, which is used to help keep track of
- * the stack pointer when aligning the stack.  r10 or r13 is used as the DRAP
- * register.  The typical DRAP pattern is:
- *
- *   4c 8d 54 24 08		lea    0x8(%rsp),%r10
- *   48 83 e4 c0		and    $0xffffffffffffffc0,%rsp
- *   41 ff 72 f8		pushq  -0x8(%r10)
- *   55				push   %rbp
- *   48 89 e5			mov    %rsp,%rbp
- *				(more pushes)
- *   41 52			push   %r10
- *				...
- *   41 5a			pop    %r10
- *				(more pops)
- *   5d				pop    %rbp
- *   49 8d 62 f8		lea    -0x8(%r10),%rsp
- *   c3				retq
- *
- * There are some variations in the epilogues, like:
- *
- *   5b				pop    %rbx
- *   41 5a			pop    %r10
- *   41 5c			pop    %r12
- *   41 5d			pop    %r13
- *   41 5e			pop    %r14
- *   c9				leaveq
- *   49 8d 62 f8		lea    -0x8(%r10),%rsp
- *   c3				retq
- *
- * and:
- *
- *   4c 8b 55 e8		mov    -0x18(%rbp),%r10
- *   48 8b 5d e0		mov    -0x20(%rbp),%rbx
- *   4c 8b 65 f0		mov    -0x10(%rbp),%r12
- *   4c 8b 6d f8		mov    -0x8(%rbp),%r13
- *   c9				leaveq
- *   49 8d 62 f8		lea    -0x8(%r10),%rsp
- *   c3				retq
- *
- * Sometimes r13 is used as the DRAP register, in which case it's saved and
- * restored beforehand:
- *
- *   41 55			push   %r13
- *   4c 8d 6c 24 10		lea    0x10(%rsp),%r13
- *   48 83 e4 f0		and    $0xfffffffffffffff0,%rsp
- *				...
- *   49 8d 65 f0		lea    -0x10(%r13),%rsp
- *   41 5d			pop    %r13
- *   c3				retq
- */
-static int update_cfi_state(struct instruction *insn,
-			    struct instruction *next_insn,
-			    struct cfi_state *cfi, struct stack_op *op)
-{
-	struct cfi_reg *cfa = &cfi->cfa;
-	struct cfi_reg *regs = cfi->regs;
-
-	/* ignore UNWIND_HINT_UNDEFINED regions */
-	if (cfi->force_undefined)
-		return 0;
-
-	/* stack operations don't make sense with an undefined CFA */
-	if (cfa->base == CFI_UNDEFINED) {
-		if (insn_func(insn)) {
-			WARN_INSN(insn, "undefined stack state");
-			return -1;
-		}
-		return 0;
-	}
-
-	if (cfi->type == UNWIND_HINT_TYPE_REGS ||
-	    cfi->type == UNWIND_HINT_TYPE_REGS_PARTIAL)
-		return update_cfi_state_regs(insn, cfi, op);
-
-	switch (op->dest.type) {
-
-	case OP_DEST_REG:
-		switch (op->src.type) {
-
-		case OP_SRC_REG:
-			if (op->src.reg == CFI_SP && op->dest.reg == CFI_BP &&
-			    cfa->base == CFI_SP &&
-			    check_reg_frame_pos(&regs[CFI_BP], -cfa->offset)) {
-
-				/* mov %rsp, %rbp */
-				cfa->base = op->dest.reg;
-				cfi->bp_scratch = false;
-			}
-
-			else if (op->src.reg == CFI_SP &&
-				 op->dest.reg == CFI_BP && cfi->drap) {
-
-				/* drap: mov %rsp, %rbp */
-				regs[CFI_BP].base = CFI_BP;
-				regs[CFI_BP].offset = -cfi->stack_size;
-				cfi->bp_scratch = false;
-			}
-
-			else if (op->src.reg == CFI_SP && cfa->base == CFI_SP) {
-
-				/*
-				 * mov %rsp, %reg
-				 *
-				 * This is needed for the rare case where GCC
-				 * does:
-				 *
-				 *   mov    %rsp, %rax
-				 *   ...
-				 *   mov    %rax, %rsp
-				 */
-				cfi->vals[op->dest.reg].base = CFI_CFA;
-				cfi->vals[op->dest.reg].offset = -cfi->stack_size;
-			}
-
-			else if (op->src.reg == CFI_BP && op->dest.reg == CFI_SP &&
-				 (cfa->base == CFI_BP || cfa->base == cfi->drap_reg)) {
-
-				/*
-				 * mov %rbp, %rsp
-				 *
-				 * Restore the original stack pointer (Clang).
-				 */
-				cfi->stack_size = -cfi->regs[CFI_BP].offset;
-			}
-
-			else if (op->dest.reg == cfa->base) {
-
-				/* mov %reg, %rsp */
-				if (cfa->base == CFI_SP &&
-				    cfi->vals[op->src.reg].base == CFI_CFA) {
-
-					/*
-					 * This is needed for the rare case
-					 * where GCC does something dumb like:
-					 *
-					 *   lea    0x8(%rsp), %rcx
-					 *   ...
-					 *   mov    %rcx, %rsp
-					 */
-					cfa->offset = -cfi->vals[op->src.reg].offset;
-					cfi->stack_size = cfa->offset;
-
-				} else if (cfa->base == CFI_SP &&
-					   cfi->vals[op->src.reg].base == CFI_SP_INDIRECT &&
-					   cfi->vals[op->src.reg].offset == cfa->offset) {
-
-					/*
-					 * Stack swizzle:
-					 *
-					 * 1: mov %rsp, (%[tos])
-					 * 2: mov %[tos], %rsp
-					 *    ...
-					 * 3: pop %rsp
-					 *
-					 * Where:
-					 *
-					 * 1 - places a pointer to the previous
-					 *     stack at the Top-of-Stack of the
-					 *     new stack.
-					 *
-					 * 2 - switches to the new stack.
-					 *
-					 * 3 - pops the Top-of-Stack to restore
-					 *     the original stack.
-					 *
-					 * Note: we set base to SP_INDIRECT
-					 * here and preserve offset. Therefore
-					 * when the unwinder reaches ToS it
-					 * will dereference SP and then add the
-					 * offset to find the next frame, IOW:
-					 * (%rsp) + offset.
-					 */
-					cfa->base = CFI_SP_INDIRECT;
-
-				} else {
-					cfa->base = CFI_UNDEFINED;
-					cfa->offset = 0;
-				}
-			}
-
-			else if (op->dest.reg == CFI_SP &&
-				 cfi->vals[op->src.reg].base == CFI_SP_INDIRECT &&
-				 cfi->vals[op->src.reg].offset == cfa->offset) {
-
-				/*
-				 * The same stack swizzle case 2) as above. But
-				 * because we can't change cfa->base, case 3)
-				 * will become a regular POP. Pretend we're a
-				 * PUSH so things don't go unbalanced.
-				 */
-				cfi->stack_size += 8;
-			}
-
-
-			break;
-
-		case OP_SRC_ADD:
-			if (op->dest.reg == CFI_SP && op->src.reg == CFI_SP) {
-
-				/* add imm, %rsp */
-				cfi->stack_size -= op->src.offset;
-				if (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 */
-				cfi->stack_size = -(op->src.offset + regs[CFI_BP].offset);
-				break;
-			}
-
-			if (op->src.reg == CFI_SP && cfa->base == CFI_SP) {
-
-				/* drap: lea disp(%rsp), %drap */
-				cfi->drap_reg = op->dest.reg;
-
-				/*
-				 * lea disp(%rsp), %reg
-				 *
-				 * This is needed for the rare case where GCC
-				 * does something dumb like:
-				 *
-				 *   lea    0x8(%rsp), %rcx
-				 *   ...
-				 *   mov    %rcx, %rsp
-				 */
-				cfi->vals[op->dest.reg].base = CFI_CFA;
-				cfi->vals[op->dest.reg].offset = \
-					-cfi->stack_size + op->src.offset;
-
-				break;
-			}
-
-			if (cfi->drap && op->dest.reg == CFI_SP &&
-			    op->src.reg == cfi->drap_reg) {
-
-				 /* drap: lea disp(%drap), %rsp */
-				cfa->base = CFI_SP;
-				cfa->offset = cfi->stack_size = -op->src.offset;
-				cfi->drap_reg = CFI_UNDEFINED;
-				cfi->drap = false;
-				break;
-			}
-
-			if (op->dest.reg == cfi->cfa.base && !(next_insn && next_insn->hint)) {
-				WARN_INSN(insn, "unsupported stack register modification");
-				return -1;
-			}
-
-			break;
-
-		case OP_SRC_AND:
-			if (op->dest.reg != CFI_SP ||
-			    (cfi->drap_reg != CFI_UNDEFINED && cfa->base != CFI_SP) ||
-			    (cfi->drap_reg == CFI_UNDEFINED && cfa->base != CFI_BP)) {
-				WARN_INSN(insn, "unsupported stack pointer realignment");
-				return -1;
-			}
-
-			if (cfi->drap_reg != CFI_UNDEFINED) {
-				/* drap: and imm, %rsp */
-				cfa->base = cfi->drap_reg;
-				cfa->offset = cfi->stack_size = 0;
-				cfi->drap = true;
-			}
-
-			/*
-			 * Older versions of GCC (4.8ish) realign the stack
-			 * without DRAP, with a frame pointer.
-			 */
-
-			break;
-
-		case OP_SRC_POP:
-		case OP_SRC_POPF:
-			if (op->dest.reg == CFI_SP && cfa->base == CFI_SP_INDIRECT) {
-
-				/* pop %rsp; # restore from a stack swizzle */
-				cfa->base = CFI_SP;
-				break;
-			}
-
-			if (!cfi->drap && op->dest.reg == cfa->base) {
-
-				/* pop %rbp */
-				cfa->base = CFI_SP;
-			}
-
-			if (cfi->drap && cfa->base == CFI_BP_INDIRECT &&
-			    op->dest.reg == cfi->drap_reg &&
-			    cfi->drap_offset == -cfi->stack_size) {
-
-				/* drap: pop %drap */
-				cfa->base = cfi->drap_reg;
-				cfa->offset = 0;
-				cfi->drap_offset = -1;
-
-			} else if (cfi->stack_size == -regs[op->dest.reg].offset) {
-
-				/* pop %reg */
-				restore_reg(cfi, op->dest.reg);
-			}
-
-			cfi->stack_size -= 8;
-			if (cfa->base == CFI_SP)
-				cfa->offset -= 8;
-
-			break;
-
-		case OP_SRC_REG_INDIRECT:
-			if (!cfi->drap && op->dest.reg == cfa->base &&
-			    op->dest.reg == CFI_BP) {
-
-				/* mov disp(%rsp), %rbp */
-				cfa->base = CFI_SP;
-				cfa->offset = cfi->stack_size;
-			}
-
-			if (cfi->drap && op->src.reg == CFI_BP &&
-			    op->src.offset == cfi->drap_offset) {
-
-				/* drap: mov disp(%rbp), %drap */
-				cfa->base = cfi->drap_reg;
-				cfa->offset = 0;
-				cfi->drap_offset = -1;
-			}
-
-			if (cfi->drap && op->src.reg == CFI_BP &&
-			    op->src.offset == regs[op->dest.reg].offset) {
-
-				/* drap: mov disp(%rbp), %reg */
-				restore_reg(cfi, op->dest.reg);
-
-			} else if (op->src.reg == cfa->base &&
-			    op->src.offset == regs[op->dest.reg].offset + cfa->offset) {
-
-				/* mov disp(%rbp), %reg */
-				/* mov disp(%rsp), %reg */
-				restore_reg(cfi, op->dest.reg);
-
-			} else if (op->src.reg == CFI_SP &&
-				   op->src.offset == regs[op->dest.reg].offset + cfi->stack_size) {
-
-				/* mov disp(%rsp), %reg */
-				restore_reg(cfi, op->dest.reg);
-			}
-
-			break;
-
-		default:
-			WARN_INSN(insn, "unknown stack-related instruction");
-			return -1;
-		}
-
-		break;
-
-	case OP_DEST_PUSH:
-	case OP_DEST_PUSHF:
-		cfi->stack_size += 8;
-		if (cfa->base == CFI_SP)
-			cfa->offset += 8;
-
-		if (op->src.type != OP_SRC_REG)
-			break;
-
-		if (cfi->drap) {
-			if (op->src.reg == cfa->base && op->src.reg == cfi->drap_reg) {
-
-				/* drap: push %drap */
-				cfa->base = CFI_BP_INDIRECT;
-				cfa->offset = -cfi->stack_size;
-
-				/* save drap so we know when to restore it */
-				cfi->drap_offset = -cfi->stack_size;
-
-			} else if (op->src.reg == CFI_BP && cfa->base == cfi->drap_reg) {
-
-				/* drap: push %rbp */
-				cfi->stack_size = 0;
-
-			} else {
-
-				/* drap: push %reg */
-				save_reg(cfi, op->src.reg, CFI_BP, -cfi->stack_size);
-			}
-
-		} else {
-
-			/* push %reg */
-			save_reg(cfi, op->src.reg, CFI_CFA, -cfi->stack_size);
-		}
-
-		/* detect when asm code uses rbp as a scratch register */
-		if (opts.stackval && insn_func(insn) && op->src.reg == CFI_BP &&
-		    cfa->base != CFI_BP)
-			cfi->bp_scratch = true;
-		break;
-
-	case OP_DEST_REG_INDIRECT:
-
-		if (cfi->drap) {
-			if (op->src.reg == cfa->base && op->src.reg == cfi->drap_reg) {
-
-				/* drap: mov %drap, disp(%rbp) */
-				cfa->base = CFI_BP_INDIRECT;
-				cfa->offset = op->dest.offset;
-
-				/* save drap offset so we know when to restore it */
-				cfi->drap_offset = op->dest.offset;
-			} else {
-
-				/* drap: mov reg, disp(%rbp) */
-				save_reg(cfi, op->src.reg, CFI_BP, op->dest.offset);
-			}
-
-		} else if (op->dest.reg == cfa->base) {
-
-			/* mov reg, disp(%rbp) */
-			/* mov reg, disp(%rsp) */
-			save_reg(cfi, op->src.reg, CFI_CFA,
-				 op->dest.offset - cfi->cfa.offset);
-
-		} else if (op->dest.reg == CFI_SP) {
-
-			/* mov reg, disp(%rsp) */
-			save_reg(cfi, op->src.reg, CFI_CFA,
-				 op->dest.offset - cfi->stack_size);
-
-		} else if (op->src.reg == CFI_SP && op->dest.offset == 0) {
-
-			/* mov %rsp, (%reg); # setup a stack swizzle. */
-			cfi->vals[op->dest.reg].base = CFI_SP_INDIRECT;
-			cfi->vals[op->dest.reg].offset = cfa->offset;
-		}
-
-		break;
-
-	case OP_DEST_MEM:
-		if (op->src.type != OP_SRC_POP && op->src.type != OP_SRC_POPF) {
-			WARN_INSN(insn, "unknown stack-related memory operation");
-			return -1;
-		}
-
-		/* pop mem */
-		cfi->stack_size -= 8;
-		if (cfa->base == CFI_SP)
-			cfa->offset -= 8;
-
-		break;
-
-	default:
-		WARN_INSN(insn, "unknown stack-related instruction");
-		return -1;
-	}
-
-	return 0;
-}
-
 /*
  * The stack layouts of alternatives instructions can sometimes diverge when
  * they have stack modifications.  That's fine as long as the potential stack
@@ -3305,7 +2802,7 @@ static int handle_insn_ops(struct instruction *insn,
 
 	for (op = insn->stack_ops; op; op = op->next) {
 
-		if (update_cfi_state(insn, next_insn, &state->cfi, op))
+		if (arch_update_cfi_state(insn, next_insn, &state->cfi, op))
 			return 1;
 
 		if (!insn->alt_group)
diff --git a/tools/objtool/include/objtool/arch.h b/tools/objtool/include/objtool/arch.h
index 0b303eba660e..c1cf08165e44 100644
--- a/tools/objtool/include/objtool/arch.h
+++ b/tools/objtool/include/objtool/arch.h
@@ -96,4 +96,8 @@ int arch_rewrite_retpolines(struct objtool_file *file);
 
 bool arch_pc_relative_reloc(struct reloc *reloc);
 
+int arch_update_cfi_state(struct instruction *insn,
+			  struct instruction *next_insn,
+			  struct cfi_state *cfi, struct stack_op *op);
+
 #endif /* _ARCH_H */
-- 
2.43.0

Re: [PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Xi Ruoyao]

On Wed, 2024-06-05 at 23:47 +0800, Jinyang He wrote:
> In our 419 repo this func has been renamed arch_update_insn_state (, now it
> should be arch_update_cfi_state) and give some actions to deal with the
> frame pointer case. If we support it we may deal with some case but for 
> clang...
> 
> .global test
> .type test,@function
> test:
> 
> addi.d  $sp,$sp,-448
> st.d    $ra,$sp,440
> st.d    $fp,$sp,432
> addi.d  $fp,$sp,448
> 
> # do something  <- not change $sp

This is simplified.  In the real code $sp is changed, something like:

bstrins.d $sp, $zero, 5, 0

$fp is needed to allow restoring $sp after realigning $sp for some local
over-aligned variables, as demonstrated by this example:

struct x { _Alignas(64) char buf[128]; };

void f(struct x *p);
void g()
{
	struct x x = { .buf = "1145141919810" };
	f(&x);
}

GCC does not align $sp, it produces the aligned address for x from an
unaligned $sp instead:

addi.d $a0, $sp, 63
srli.d $a0, $a0, 6
slli.d $a0, $a0, 6

thus there's no need to use $fp.

/* snip */

> <- restore regs from non-cfa ?
>          ld.d    $ra, $sp, 8                     # 8-byte Folded Reload  

/* snip */

> Maybe Clang have anything wrong?

I don't think we must restore regs based on $fp just because CFA is
based on $fp.  The .cfi directives only provides *one* possible way to
restore the regs.  This way is convenient to the unwinder, but not
necessarily convenient to the CPU thus the real instruction sequence can
use a different way.  They only need to be "equivalent", not necessarily
"exactly same."

Also note that .cfi_* directives are completely irrelevant for objtool.
THe objtool synthesizes the ORC unwind info from the machine
instructions, not the DWARF unwind info coded with .cfi_*.

The entire point of ORC is avoiding DWARF.  It's even named ORC because
ORC and DWARF are sworn enemies in some tales.

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

Re: [PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Jinyang He]

On 2024-06-06 03:05, Xi Ruoyao wrote:

> On Wed, 2024-06-05 at 23:47 +0800, Jinyang He wrote:
>> In our 419 repo this func has been renamed arch_update_insn_state (, now it
>> should be arch_update_cfi_state) and give some actions to deal with the
>> frame pointer case. If we support it we may deal with some case but for
>> clang...
>>
>> .global test
>> .type test,@function
>> test:
>>
>> addi.d  $sp,$sp,-448
>> st.d    $ra,$sp,440
>> st.d    $fp,$sp,432
>> addi.d  $fp,$sp,448
>>
>> # do something  <- not change $sp
> This is simplified.  In the real code $sp is changed, something like:
>
> bstrins.d $sp, $zero, 5, 0
>
> $fp is needed to allow restoring $sp after realigning $sp for some local
> over-aligned variables, as demonstrated by this example:
>
> struct x { _Alignas(64) char buf[128]; };
>
> void f(struct x *p);
> void g()
> {
> 	struct x x = { .buf = "1145141919810" };
> 	f(&x);
> }
>
> GCC does not align $sp, it produces the aligned address for x from an
> unaligned $sp instead:
>
> addi.d $a0, $sp, 63
> srli.d $a0, $a0, 6
> slli.d $a0, $a0, 6
>
> thus there's no need to use $fp.
>
> /* snip */
>
>> <- restore regs from non-cfa ?
>>           ld.d    $ra, $sp, 8                     # 8-byte Folded Reload
> /* snip */
>
>> Maybe Clang have anything wrong?
> I don't think we must restore regs based on $fp just because CFA is
> based on $fp.

You are right.


> The .cfi directives only provides *one* possible way to
> restore the regs.
What I just confused is that there is no ".cfi_*"
in the eplogue by clang, which may cause wrong backtrace if gdb set

breakpoint there and backtrace. (But this is out of this topic.)



> This way is convenient to the unwinder, but not
> necessarily convenient to the CPU thus the real instruction sequence can
> use a different way.  They only need to be "equivalent", not necessarily
> "exactly same."
>
> Also note that .cfi_* directives are completely irrelevant for objtool.
> THe objtool synthesizes the ORC unwind info from the machine
> instructions, not the DWARF unwind info coded with .cfi_*.
> The entire point of ORC is avoiding DWARF.  It's even named ORC because
> ORC and DWARF are sworn enemies in some tales.
Yes.
The yestorday attachment has something wrong.
Enmmmm.... (It seems I'm careless.) diff is,

                 switch (op->src.type) {
                 case OP_SRC_ADD:
                         if (op->dest.reg == CFI_SP && op->src.reg == 
CFI_SP) {
-                               /* addi.d sp, sp, imm */
-                               cfi->stack_size -= op->src.offset;
-                               if (cfa->base == CFI_SP)
                                 /* addi.d sp, sp, imm */
                                 cfi->stack_size -= op->src.offset;
                                 if (cfa->base == CFI_SP)

Or you can get the tmp fix by [1].

[1]: 
https://github.com/MQ-mengqing/linux/commit/c1f7df3eb2a2bb7a1c10c2aa6e0e3d585b457352

Re: [PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Xi Ruoyao]

On Thu, 2024-06-06 at 10:10 +0800, Jinyang He wrote:
> What I just confused is that there is no ".cfi_*"
> in the eplogue by clang, which may cause wrong backtrace if gdb set
> 
> breakpoint there and backtrace. (But this is out of this topic.)

I don't think it'll cause wrong backtrace.  The real assemble code has
restored the registers and missing .cfi_restore will just make unwinder
restore them again.  There are redundant works but not breakages.

For objtool the main difference seems a thing explained in
https://maskray.me/blog/2020-11-08-stack-unwinding by Fangrui:

   Note: on RISC-V and LoongArch, the stack slot for the previous frame
   pointer is stored at fp[-2] instead of fp[0]. See [Consider
   standardising which stack slot fp points
   to](https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/18)
   for the RISC-V discussion.

So perhaps we just need to code a constant named "PREV_BP_OFFSET" or
something in arch/ and use it in update_cfi_state() instead of fully re-
implement the entire function?
-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Jinyang He]

On 2024-06-07 13:42, Xi Ruoyao wrote:
> On Thu, 2024-06-06 at 10:10 +0800, Jinyang He wrote:
>> What I just confused is that there is no ".cfi_*"
>> in the eplogue by clang, which may cause wrong backtrace if gdb set
>>
>> breakpoint there and backtrace. (But this is out of this topic.)
> I don't think it'll cause wrong backtrace.  The real assemble code has
> restored the registers and missing .cfi_restore will just make unwinder
> restore them again.  There are redundant works but not breakages.

$ cat hello.c:
extern void __attribute__((noinline)) foo() {}
int main() {
   foo();
   return (int)(long)__builtin_frame_address(0);
}
$ clang hello.c -S -g -o hello.s -O0 -fPIC
$ cat hello.s:
[...]
         addi.d  $sp, $sp, -32
         .cfi_def_cfa_offset 32
         st.d    $ra, $sp, 24                    # 8-byte Folded Spill
         st.d    $fp, $sp, 16                    # 8-byte Folded Spill
         .cfi_offset 1, -8
         .cfi_offset 22, -16
         addi.d  $fp, $sp, 32
         .cfi_def_cfa 22, 0
         move    $a0, $zero
         st.w    $a0, $fp, -20
.Ltmp2:
         .loc    0 3 3 prologue_end              # hello.c:3:3
         bl      %plt(foo)
         .loc    0 4 21                          # hello.c:4:21
         move    $a0, $fp
         .loc    0 4 3 is_stmt 0                 # hello.c:4:3
         addi.w  $a0, $a0, 0
         ld.d    $fp, $sp, 16                    # 8-byte Folded Reload
<use gdb and set break ponint there>
         ld.d    $ra, $sp, 24                    # 8-byte Folded Reload
         .loc    0 4 3 epilogue_begin            # hello.c:4:3
         addi.d  $sp, $sp, 32
         ret
[...]

So how unwinder do if we <use gdb and set break ponint there>? I think
if we not give ".cfi_restore 22" or others info, it will consider
   1) the $ra is in cfa-8
   2) the cfa is $fp
So it will get $ra by $ra = *(long*)($fp-8). So it may unwind failed
because $fp has been restored and not the CFA now.

> For objtool the main difference seems a thing explained in
> https://maskray.me/blog/2020-11-08-stack-unwinding by Fangrui:
>
>     Note: on RISC-V and LoongArch, the stack slot for the previous frame
>     pointer is stored at fp[-2] instead of fp[0]. See [Consider
>     standardising which stack slot fp points
>     to](https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/18)
>     for the RISC-V discussion.

In most cases the $fp is saved at cfa-16. But for va args, something
becomes different at LoongArch (I do not know the case of riscv), the
$fp isn't saved at cfa-16. (e.g. printk?)

> So perhaps we just need to code a constant named "PREV_BP_OFFSET"
Can you give some detail?
>   or
> something in arch/ and use it in update_cfi_state() instead of fully re-
> implement the entire function?

I feel that the update_cfi_state should be arch specific. I believe
that some logic can be reused, but each arch may have its own logic.


Thanks,
Jinyang

Re: [PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Xi Ruoyao]

On Fri, 2024-06-07 at 15:14 +0800, Jinyang He wrote:
> >      Note: on RISC-V and LoongArch, the stack slot for the previous frame
> >      pointer is stored at fp[-2] instead of fp[0]. See [Consider
> >      standardising which stack slot fp points
> >      to](https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/18)
> >      for the RISC-V discussion.
> 
> In most cases the $fp is saved at cfa-16. But for va args, something
> becomes different at LoongArch (I do not know the case of riscv), the
> $fp isn't saved at cfa-16. (e.g. printk?)

Oops indeed.  Even with a very simple case:

int sum(int a, int b) {
	return a + b;
}

with -fno-omit-frame-pointer we get:

sum:
	addi.d	$r3,$r3,-16
	st.d	$r22,$r3,8
	addi.d	$r22,$r3,16
	ld.d	$r22,$r3,8
	add.w	$r4,$r4,$r5
	addi.d	$r3,$r3,16
	jr	$r1

So for leaf functions (where we don't save $ra) $fp is saved at cfa-8.

> I feel that the update_cfi_state should be arch specific. I believe
> that some logic can be reused, but each arch may have its own logic.

I agree it now.

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

Re: [PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Huacai Chen]

Hi, Ruoyao and Jinyang,

On Fri, Jun 7, 2024 at 4:29 PM Xi Ruoyao <xry111@xry111.site> wrote:
>
> On Fri, 2024-06-07 at 15:14 +0800, Jinyang He wrote:
> > >      Note: on RISC-V and LoongArch, the stack slot for the previous frame
> > >      pointer is stored at fp[-2] instead of fp[0]. See [Consider
> > >      standardising which stack slot fp points
> > >      to](https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/18)
> > >      for the RISC-V discussion.
> >
> > In most cases the $fp is saved at cfa-16. But for va args, something
> > becomes different at LoongArch (I do not know the case of riscv), the
> > $fp isn't saved at cfa-16. (e.g. printk?)
>
> Oops indeed.  Even with a very simple case:
>
> int sum(int a, int b) {
>         return a + b;
> }
>
> with -fno-omit-frame-pointer we get:
>
> sum:
>         addi.d  $r3,$r3,-16
>         st.d    $r22,$r3,8
>         addi.d  $r22,$r3,16
>         ld.d    $r22,$r3,8
>         add.w   $r4,$r4,$r5
>         addi.d  $r3,$r3,16
>         jr      $r1
>
> So for leaf functions (where we don't save $ra) $fp is saved at cfa-8.
>
> > I feel that the update_cfi_state should be arch specific. I believe
> > that some logic can be reused, but each arch may have its own logic.
>
> I agree it now.
What is the conclusion about the clang part now? And for the original
-mno-thin-add-sub problem, do you have some way to fix it in the root?
I think we needn't rush, there are some weeks before 6.10 released.

Huacai

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

Re: [PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Xi Ruoyao]

On Sat, 2024-06-15 at 16:45 +0800, Huacai Chen wrote:
> Hi, Ruoyao and Jinyang,
> 
> On Fri, Jun 7, 2024 at 4:29 PM Xi Ruoyao <xry111@xry111.site> wrote:
> > 
> > On Fri, 2024-06-07 at 15:14 +0800, Jinyang He wrote:
> > > >      Note: on RISC-V and LoongArch, the stack slot for the previous frame
> > > >      pointer is stored at fp[-2] instead of fp[0]. See [Consider
> > > >      standardising which stack slot fp points
> > > >      to](https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/18)
> > > >      for the RISC-V discussion.
> > > 
> > > In most cases the $fp is saved at cfa-16. But for va args, something
> > > becomes different at LoongArch (I do not know the case of riscv), the
> > > $fp isn't saved at cfa-16. (e.g. printk?)
> > 
> > Oops indeed.  Even with a very simple case:
> > 
> > int sum(int a, int b) {
> >         return a + b;
> > }
> > 
> > with -fno-omit-frame-pointer we get:
> > 
> > sum:
> >         addi.d  $r3,$r3,-16
> >         st.d    $r22,$r3,8
> >         addi.d  $r22,$r3,16
> >         ld.d    $r22,$r3,8
> >         add.w   $r4,$r4,$r5
> >         addi.d  $r3,$r3,16
> >         jr      $r1
> > 
> > So for leaf functions (where we don't save $ra) $fp is saved at cfa-8.
> > 
> > > I feel that the update_cfi_state should be arch specific. I believe
> > > that some logic can be reused, but each arch may have its own logic.
> > 
> > I agree it now.
> What is the conclusion about the clang part now? And for the original
> -mno-thin-add-sub problem, do you have some way to fix it in the root?
> I think we needn't rush, there are some weeks before 6.10 released.

To me for now we should just make OBJTOOL and ORC depend on BROKEN and
backport to stable...

Even if we can fix both the -mno-thin-add-sub problem and the frame
pointer problem in these weeks, they'll be some nontrivial large change
and improper to backport.  Thus we have to admit objtool doesn't really
work for old releases and mark it broken.


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

Re: [PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Huacai Chen]

On Sat, Jun 15, 2024 at 4:54 PM Xi Ruoyao <xry111@xry111.site> wrote:
>
> On Sat, 2024-06-15 at 16:45 +0800, Huacai Chen wrote:
> > Hi, Ruoyao and Jinyang,
> >
> > On Fri, Jun 7, 2024 at 4:29 PM Xi Ruoyao <xry111@xry111.site> wrote:
> > >
> > > On Fri, 2024-06-07 at 15:14 +0800, Jinyang He wrote:
> > > > >      Note: on RISC-V and LoongArch, the stack slot for the previous frame
> > > > >      pointer is stored at fp[-2] instead of fp[0]. See [Consider
> > > > >      standardising which stack slot fp points
> > > > >      to](https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/18)
> > > > >      for the RISC-V discussion.
> > > >
> > > > In most cases the $fp is saved at cfa-16. But for va args, something
> > > > becomes different at LoongArch (I do not know the case of riscv), the
> > > > $fp isn't saved at cfa-16. (e.g. printk?)
> > >
> > > Oops indeed.  Even with a very simple case:
> > >
> > > int sum(int a, int b) {
> > >         return a + b;
> > > }
> > >
> > > with -fno-omit-frame-pointer we get:
> > >
> > > sum:
> > >         addi.d  $r3,$r3,-16
> > >         st.d    $r22,$r3,8
> > >         addi.d  $r22,$r3,16
> > >         ld.d    $r22,$r3,8
> > >         add.w   $r4,$r4,$r5
> > >         addi.d  $r3,$r3,16
> > >         jr      $r1
> > >
> > > So for leaf functions (where we don't save $ra) $fp is saved at cfa-8.
> > >
> > > > I feel that the update_cfi_state should be arch specific. I believe
> > > > that some logic can be reused, but each arch may have its own logic.
> > >
> > > I agree it now.
> > What is the conclusion about the clang part now? And for the original
> > -mno-thin-add-sub problem, do you have some way to fix it in the root?
> > I think we needn't rush, there are some weeks before 6.10 released.
>
> To me for now we should just make OBJTOOL and ORC depend on BROKEN and
> backport to stable...
But this patch allows clang to build objtool, which seems broken, too.

>
> Even if we can fix both the -mno-thin-add-sub problem and the frame
> pointer problem in these weeks, they'll be some nontrivial large change
> and improper to backport.  Thus we have to admit objtool doesn't really
> work for old releases and mark it broken.
I don't like to disable objtool, unless there is no better solution.
And it seems there has already been some "large fix" in objtool's
history.

Huacai

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

Re: [PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Xi Ruoyao]

On Sat, 2024-06-15 at 17:33 +0800, Huacai Chen wrote:
> > To me for now we should just make OBJTOOL and ORC depend on BROKEN and
> > backport to stable...
> But this patch allows clang to build objtool, which seems broken, too.

Yes, so I mean make objtool depend on CONFIG_BROKEN because it is indeed
broken as at now.

Or we'll end up at least:

select HAVE_OBJTOOL if AS_HAS_EXPLICIT_RELOCS && AS_HAS_THIN_ADD_SUB && !CC_IS_CLANG && !RUST

this is already nasty and I still don't know if it covers all broken
cases (I've no idea if GCC will generate frame pointer in some cases as
well...)

> > Even if we can fix both the -mno-thin-add-sub problem and the frame
> > pointer problem in these weeks, they'll be some nontrivial large change
> > and improper to backport.  Thus we have to admit objtool doesn't really
> > work for old releases and mark it broken.
> I don't like to disable objtool, unless there is no better solution.
> And it seems there has already been some "large fix" in objtool's
> history.

Then we can still backport the large fix to the stable branches when we
finish it up.

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

Re: [PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Huacai Chen]

On Sat, Jun 15, 2024 at 6:22 PM Xi Ruoyao <xry111@xry111.site> wrote:
>
> On Sat, 2024-06-15 at 17:33 +0800, Huacai Chen wrote:
> > > To me for now we should just make OBJTOOL and ORC depend on BROKEN and
> > > backport to stable...
> > But this patch allows clang to build objtool, which seems broken, too.
>
> Yes, so I mean make objtool depend on CONFIG_BROKEN because it is indeed
> broken as at now.
I don't like CONFIG_BROKEN here, that means telling everyone not to
enable OBJTOOL, but that is not the case.

>
> Or we'll end up at least:
>
> select HAVE_OBJTOOL if AS_HAS_EXPLICIT_RELOCS && AS_HAS_THIN_ADD_SUB && !CC_IS_CLANG && !RUST
Maybe we needn't consider RUST here? And can we think
AS_HAS_THIN_ADD_SUB always imply AS_HAS_EXPLICIT_RELOCS here?

If so, we can simplify the condition. And please submit an updated version.

Huacai

>
> this is already nasty and I still don't know if it covers all broken
> cases (I've no idea if GCC will generate frame pointer in some cases as
> well...)
>
> > > Even if we can fix both the -mno-thin-add-sub problem and the frame
> > > pointer problem in these weeks, they'll be some nontrivial large change
> > > and improper to backport.  Thus we have to admit objtool doesn't really
> > > work for old releases and mark it broken.
> > I don't like to disable objtool, unless there is no better solution.
> > And it seems there has already been some "large fix" in objtool's
> > history.
>
> Then we can still backport the large fix to the stable branches when we
> finish it up.
>
> --
> Xi Ruoyao <xry111@xry111.site>
> School of Aerospace Science and Technology, Xidian University

Re: [PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Xi Ruoyao]

On Mon, 2024-06-17 at 21:11 +0800, Huacai Chen wrote:



> > select HAVE_OBJTOOL if AS_HAS_EXPLICIT_RELOCS && AS_HAS_THIN_ADD_SUB && !CC_IS_CLANG && !RUST
> Maybe we needn't consider RUST here?

Rustc does use $fp that objtool cannot handle as at now.  It can be
demonstrated with an over-aligned type, similar to Clang:

$ cat t.rs
#[repr(C, align(64))]
struct X(i32);

extern { fn f(x: &X) -> i64; }

#[no_mangle]
fn g() -> i64 {
    let x = X(114514);
    unsafe {f(&x)}
}
$ rustc t.rs --emit=asm --crate-type=staticlib -O
$ grep fp t.s
	st.d	$fp, $sp, 112
	addi.d	$fp, $sp, 128
	addi.d	$sp, $fp, -128
	ld.d	$fp, $sp, 112

The kernel uses rust-bindgen to generate some .rs file from C headers. 
And __attribute__((aligned(x))) is directly translated to
repr(align(x)).  As __attribute__((aligned(x))) is very common in the
kernel I expect objtool will fail to handle some object code from rustc.

> And can we think AS_HAS_THIN_ADD_SUB always imply
> AS_HAS_EXPLICIT_RELOCS here?

Maybe, AFAIK there's no assembler using thin add-sub but not explicit
relocs.

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

Re: [PATCH] loongarch: Only select HAVE_OBJTOOL and allow ORC unwinder if the inline assembler supports R_LARCH_{32,64}_PCREL

[Huacai Chen]

On Mon, Jun 17, 2024 at 9:38 PM Xi Ruoyao <xry111@xry111.site> wrote:
>
> On Mon, 2024-06-17 at 21:11 +0800, Huacai Chen wrote:
>
>
>
> > > select HAVE_OBJTOOL if AS_HAS_EXPLICIT_RELOCS && AS_HAS_THIN_ADD_SUB && !CC_IS_CLANG && !RUST
> > Maybe we needn't consider RUST here?
>
> Rustc does use $fp that objtool cannot handle as at now.  It can be
> demonstrated with an over-aligned type, similar to Clang:
>
> $ cat t.rs
> #[repr(C, align(64))]
> struct X(i32);
>
> extern { fn f(x: &X) -> i64; }
>
> #[no_mangle]
> fn g() -> i64 {
>     let x = X(114514);
>     unsafe {f(&x)}
> }
> $ rustc t.rs --emit=asm --crate-type=staticlib -O
> $ grep fp t.s
>         st.d    $fp, $sp, 112
>         addi.d  $fp, $sp, 128
>         addi.d  $sp, $fp, -128
>         ld.d    $fp, $sp, 112
>
> The kernel uses rust-bindgen to generate some .rs file from C headers.
> And __attribute__((aligned(x))) is directly translated to
> repr(align(x)).  As __attribute__((aligned(x))) is very common in the
> kernel I expect objtool will fail to handle some object code from rustc.
My meaning is: the build robot doesn't enable RUST now (maybe I'm
wrong), and this problem will surely be fixed in future, so we can
make the condition as simple as possible.

Huacai

>
> > And can we think AS_HAS_THIN_ADD_SUB always imply
> > AS_HAS_EXPLICIT_RELOCS here?
>
> Maybe, AFAIK there's no assembler using thin add-sub but not explicit
> relocs.
>
> --
> Xi Ruoyao <xry111@xry111.site>
> School of Aerospace Science and Technology, Xidian University
>