* arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives
@ 2022-09-26 13:27 Naresh Kamboju
2022-09-26 15:44 ` Nathan Chancellor
2022-09-26 16:11 ` arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives Russell King (Oracle)
0 siblings, 2 replies; 13+ messages in thread
From: Naresh Kamboju @ 2022-09-26 13:27 UTC (permalink / raw)
To: open list, Linux ARM
Cc: Arnd Bergmann, Russell King (Oracle),
Masami Hiramatsu, Nathan Chancellor, regressions, lkft-triage
Following build warnings / errors noticed on arm with clang-13 / 14
on Linux next-20220923.
Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
Regressions found on arm:
- build-clang-13-bcm2835_defconfig
- build-clang-nightly-imx_v6_v7_defconfig
- build-clang-nightly-orion5x_defconfig
- build-clang-13-keystone_defconfig
- build-clang-13-omap2plus_defconfig
- build-clang-14-imx_v6_v7_defconfig
- build-clang-nightly-omap2plus_defconfig
- build-clang-nightly-multi_v5_defconfig
- build-clang-nightly-bcm2835_defconfig
- build-clang-13-imx_v6_v7_defconfig
- build-clang-13-imx_v4_v5_defconfig
- build-clang-14-imx_v4_v5_defconfig
- build-clang-13-orion5x_defconfig
- build-clang-14-multi_v5_defconfig-65236a87
- build-clang-14-lkftconfig
- build-clang-nightly-imx_v4_v5_defconfig
- build-clang-13-multi_v5_defconfig
- build-clang-13-lkftconfig
- build-clang-nightly-keystone_defconfig
- build-clang-14-multi_v5_defconfig
- build-clang-14-orion5x_defconfig
- build-clang-14-omap2plus_defconfig
- build-clang-nightly-multi_v5_defconfig-65236a87
- build-clang-14-bcm2835_defconfig
- build-clang-14-keystone_defconfig
- build-clang-nightly-lkftconfig
arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede
.save or .vsave directives
"stmdb sp, {sp, lr, pc} \n\t"
^
<inline asm>:3:2: note: instantiated into assembly here
.save {sp, lr, pc}
^
/builds/linux/arch/arm/probes/kprobes/core.c:412:29: error: .fnstart
must precede .pad directive
"stmdb sp!, {r0 - r11} \n\t"
^
<inline asm>:6:2: note: instantiated into assembly here
.pad #52
^
2 errors generated.
make[5]: *** [/builds/linux/scripts/Makefile.build:250:
arch/arm/probes/kprobes/core.o] Error 1
build log:
https://builds.tuxbuild.com/2FAyD1qcTlzjIYE7mjrugjCsxu1/
--
Linaro LKFT
https://lkft.linaro.org
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives
2022-09-26 13:27 arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives Naresh Kamboju
@ 2022-09-26 15:44 ` Nathan Chancellor
2022-09-26 15:46 ` Nathan Chancellor
2022-09-26 16:11 ` arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives Russell King (Oracle)
1 sibling, 1 reply; 13+ messages in thread
From: Nathan Chancellor @ 2022-09-26 15:44 UTC (permalink / raw)
To: Naresh Kamboju
Cc: open list, Linux ARM, Arnd Bergmann, Russell King (Oracle),
Masami Hiramatsu, regressions, lkft-triage
Hi Naresh,
On Mon, Sep 26, 2022 at 06:57:00PM +0530, Naresh Kamboju wrote:
> Following build warnings / errors noticed on arm with clang-13 / 14
> on Linux next-20220923.
>
> Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
>
> Regressions found on arm:
>
> - build-clang-13-bcm2835_defconfig
> - build-clang-nightly-imx_v6_v7_defconfig
> - build-clang-nightly-orion5x_defconfig
> - build-clang-13-keystone_defconfig
> - build-clang-13-omap2plus_defconfig
> - build-clang-14-imx_v6_v7_defconfig
> - build-clang-nightly-omap2plus_defconfig
> - build-clang-nightly-multi_v5_defconfig
> - build-clang-nightly-bcm2835_defconfig
> - build-clang-13-imx_v6_v7_defconfig
> - build-clang-13-imx_v4_v5_defconfig
> - build-clang-14-imx_v4_v5_defconfig
> - build-clang-13-orion5x_defconfig
> - build-clang-14-multi_v5_defconfig-65236a87
> - build-clang-14-lkftconfig
> - build-clang-nightly-imx_v4_v5_defconfig
> - build-clang-13-multi_v5_defconfig
> - build-clang-13-lkftconfig
> - build-clang-nightly-keystone_defconfig
> - build-clang-14-multi_v5_defconfig
> - build-clang-14-orion5x_defconfig
> - build-clang-14-omap2plus_defconfig
> - build-clang-nightly-multi_v5_defconfig-65236a87
> - build-clang-14-bcm2835_defconfig
> - build-clang-14-keystone_defconfig
> - build-clang-nightly-lkftconfig
>
> arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede
> .save or .vsave directives
> "stmdb sp, {sp, lr, pc} \n\t"
> ^
> <inline asm>:3:2: note: instantiated into assembly here
> .save {sp, lr, pc}
> ^
> /builds/linux/arch/arm/probes/kprobes/core.c:412:29: error: .fnstart
> must precede .pad directive
> "stmdb sp!, {r0 - r11} \n\t"
> ^
> <inline asm>:6:2: note: instantiated into assembly here
> .pad #52
> ^
> 2 errors generated.
> make[5]: *** [/builds/linux/scripts/Makefile.build:250:
> arch/arm/probes/kprobes/core.o] Error 1
>
> build log:
> https://builds.tuxbuild.com/2FAyD1qcTlzjIYE7mjrugjCsxu1/
Thank you for the testing and report! I brought this up on GitHub on
Friday as I noticed this as well:
https://github.com/ClangBuiltLinux/linux/issues/1718
It sounds like we can avoid this by rewriting __kretprobe_trampoline()
in out of line assembly but I have not had a chance to sit down and try
it.
Cheers,
Nathan
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives
2022-09-26 15:44 ` Nathan Chancellor
@ 2022-09-26 15:46 ` Nathan Chancellor
2022-09-26 17:42 ` Nick Desaulniers
0 siblings, 1 reply; 13+ messages in thread
From: Nathan Chancellor @ 2022-09-26 15:46 UTC (permalink / raw)
To: Naresh Kamboju
Cc: open list, Linux ARM, Arnd Bergmann, Russell King (Oracle),
Masami Hiramatsu, regressions, lkft-triage, llvm
+ our mailing list, I should have added it with that message.
On Mon, Sep 26, 2022 at 08:44:05AM -0700, Nathan Chancellor wrote:
> Hi Naresh,
>
> On Mon, Sep 26, 2022 at 06:57:00PM +0530, Naresh Kamboju wrote:
> > Following build warnings / errors noticed on arm with clang-13 / 14
> > on Linux next-20220923.
> >
> > Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
> >
> > Regressions found on arm:
> >
> > - build-clang-13-bcm2835_defconfig
> > - build-clang-nightly-imx_v6_v7_defconfig
> > - build-clang-nightly-orion5x_defconfig
> > - build-clang-13-keystone_defconfig
> > - build-clang-13-omap2plus_defconfig
> > - build-clang-14-imx_v6_v7_defconfig
> > - build-clang-nightly-omap2plus_defconfig
> > - build-clang-nightly-multi_v5_defconfig
> > - build-clang-nightly-bcm2835_defconfig
> > - build-clang-13-imx_v6_v7_defconfig
> > - build-clang-13-imx_v4_v5_defconfig
> > - build-clang-14-imx_v4_v5_defconfig
> > - build-clang-13-orion5x_defconfig
> > - build-clang-14-multi_v5_defconfig-65236a87
> > - build-clang-14-lkftconfig
> > - build-clang-nightly-imx_v4_v5_defconfig
> > - build-clang-13-multi_v5_defconfig
> > - build-clang-13-lkftconfig
> > - build-clang-nightly-keystone_defconfig
> > - build-clang-14-multi_v5_defconfig
> > - build-clang-14-orion5x_defconfig
> > - build-clang-14-omap2plus_defconfig
> > - build-clang-nightly-multi_v5_defconfig-65236a87
> > - build-clang-14-bcm2835_defconfig
> > - build-clang-14-keystone_defconfig
> > - build-clang-nightly-lkftconfig
> >
> > arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede
> > .save or .vsave directives
> > "stmdb sp, {sp, lr, pc} \n\t"
> > ^
> > <inline asm>:3:2: note: instantiated into assembly here
> > .save {sp, lr, pc}
> > ^
> > /builds/linux/arch/arm/probes/kprobes/core.c:412:29: error: .fnstart
> > must precede .pad directive
> > "stmdb sp!, {r0 - r11} \n\t"
> > ^
> > <inline asm>:6:2: note: instantiated into assembly here
> > .pad #52
> > ^
> > 2 errors generated.
> > make[5]: *** [/builds/linux/scripts/Makefile.build:250:
> > arch/arm/probes/kprobes/core.o] Error 1
> >
> > build log:
> > https://builds.tuxbuild.com/2FAyD1qcTlzjIYE7mjrugjCsxu1/
>
> Thank you for the testing and report! I brought this up on GitHub on
> Friday as I noticed this as well:
>
> https://github.com/ClangBuiltLinux/linux/issues/1718
>
> It sounds like we can avoid this by rewriting __kretprobe_trampoline()
> in out of line assembly but I have not had a chance to sit down and try
> it.
>
> Cheers,
> Nathan
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives
2022-09-26 13:27 arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives Naresh Kamboju
2022-09-26 15:44 ` Nathan Chancellor
@ 2022-09-26 16:11 ` Russell King (Oracle)
1 sibling, 0 replies; 13+ messages in thread
From: Russell King (Oracle) @ 2022-09-26 16:11 UTC (permalink / raw)
To: Chen Zhongjin, Naresh Kamboju
Cc: open list, Linux ARM, Arnd Bergmann, Masami Hiramatsu,
Nathan Chancellor, regressions, lkft-triage
Looks to me like a failure caused by Chen Zhongjin's change:
"Recover kretprobes return address for EABI stack unwinder"
I will drop this patch; please submit a replacement that builds.
Thanks.
On Mon, Sep 26, 2022 at 06:57:00PM +0530, Naresh Kamboju wrote:
> Following build warnings / errors noticed on arm with clang-13 / 14
> on Linux next-20220923.
>
> Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
>
> Regressions found on arm:
>
> - build-clang-13-bcm2835_defconfig
> - build-clang-nightly-imx_v6_v7_defconfig
> - build-clang-nightly-orion5x_defconfig
> - build-clang-13-keystone_defconfig
> - build-clang-13-omap2plus_defconfig
> - build-clang-14-imx_v6_v7_defconfig
> - build-clang-nightly-omap2plus_defconfig
> - build-clang-nightly-multi_v5_defconfig
> - build-clang-nightly-bcm2835_defconfig
> - build-clang-13-imx_v6_v7_defconfig
> - build-clang-13-imx_v4_v5_defconfig
> - build-clang-14-imx_v4_v5_defconfig
> - build-clang-13-orion5x_defconfig
> - build-clang-14-multi_v5_defconfig-65236a87
> - build-clang-14-lkftconfig
> - build-clang-nightly-imx_v4_v5_defconfig
> - build-clang-13-multi_v5_defconfig
> - build-clang-13-lkftconfig
> - build-clang-nightly-keystone_defconfig
> - build-clang-14-multi_v5_defconfig
> - build-clang-14-orion5x_defconfig
> - build-clang-14-omap2plus_defconfig
> - build-clang-nightly-multi_v5_defconfig-65236a87
> - build-clang-14-bcm2835_defconfig
> - build-clang-14-keystone_defconfig
> - build-clang-nightly-lkftconfig
>
> arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede
> .save or .vsave directives
> "stmdb sp, {sp, lr, pc} \n\t"
> ^
> <inline asm>:3:2: note: instantiated into assembly here
> .save {sp, lr, pc}
> ^
> /builds/linux/arch/arm/probes/kprobes/core.c:412:29: error: .fnstart
> must precede .pad directive
> "stmdb sp!, {r0 - r11} \n\t"
> ^
> <inline asm>:6:2: note: instantiated into assembly here
> .pad #52
> ^
> 2 errors generated.
> make[5]: *** [/builds/linux/scripts/Makefile.build:250:
> arch/arm/probes/kprobes/core.o] Error 1
>
> build log:
> https://builds.tuxbuild.com/2FAyD1qcTlzjIYE7mjrugjCsxu1/
>
> --
> Linaro LKFT
> https://lkft.linaro.org
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives
2022-09-26 15:46 ` Nathan Chancellor
@ 2022-09-26 17:42 ` Nick Desaulniers
2022-09-26 18:02 ` Russell King (Oracle)
0 siblings, 1 reply; 13+ messages in thread
From: Nick Desaulniers @ 2022-09-26 17:42 UTC (permalink / raw)
To: Nathan Chancellor, Naresh Kamboju
Cc: open list, Linux ARM, Arnd Bergmann, Russell King (Oracle),
Masami Hiramatsu, regressions, lkft-triage, llvm
On Mon, Sep 26, 2022 at 8:46 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> + our mailing list, I should have added it with that message.
>
> On Mon, Sep 26, 2022 at 08:44:05AM -0700, Nathan Chancellor wrote:
> > Hi Naresh,
> >
> > On Mon, Sep 26, 2022 at 06:57:00PM +0530, Naresh Kamboju wrote:
> > > Following build warnings / errors noticed on arm with clang-13 / 14
> > > on Linux next-20220923.
> > >
> > > Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
> > >
> > > Regressions found on arm:
> > >
> > > - build-clang-13-bcm2835_defconfig
> > > - build-clang-nightly-imx_v6_v7_defconfig
> > > - build-clang-nightly-orion5x_defconfig
> > > - build-clang-13-keystone_defconfig
> > > - build-clang-13-omap2plus_defconfig
> > > - build-clang-14-imx_v6_v7_defconfig
> > > - build-clang-nightly-omap2plus_defconfig
> > > - build-clang-nightly-multi_v5_defconfig
> > > - build-clang-nightly-bcm2835_defconfig
> > > - build-clang-13-imx_v6_v7_defconfig
> > > - build-clang-13-imx_v4_v5_defconfig
> > > - build-clang-14-imx_v4_v5_defconfig
> > > - build-clang-13-orion5x_defconfig
> > > - build-clang-14-multi_v5_defconfig-65236a87
> > > - build-clang-14-lkftconfig
> > > - build-clang-nightly-imx_v4_v5_defconfig
> > > - build-clang-13-multi_v5_defconfig
> > > - build-clang-13-lkftconfig
> > > - build-clang-nightly-keystone_defconfig
> > > - build-clang-14-multi_v5_defconfig
> > > - build-clang-14-orion5x_defconfig
> > > - build-clang-14-omap2plus_defconfig
> > > - build-clang-nightly-multi_v5_defconfig-65236a87
> > > - build-clang-14-bcm2835_defconfig
> > > - build-clang-14-keystone_defconfig
> > > - build-clang-nightly-lkftconfig
> > >
> > > arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede
> > > .save or .vsave directives
> > > "stmdb sp, {sp, lr, pc} \n\t"
> > > ^
> > > <inline asm>:3:2: note: instantiated into assembly here
> > > .save {sp, lr, pc}
> > > ^
> > > /builds/linux/arch/arm/probes/kprobes/core.c:412:29: error: .fnstart
> > > must precede .pad directive
> > > "stmdb sp!, {r0 - r11} \n\t"
> > > ^
> > > <inline asm>:6:2: note: instantiated into assembly here
> > > .pad #52
> > > ^
> > > 2 errors generated.
> > > make[5]: *** [/builds/linux/scripts/Makefile.build:250:
> > > arch/arm/probes/kprobes/core.o] Error 1
> > >
> > > build log:
> > > https://builds.tuxbuild.com/2FAyD1qcTlzjIYE7mjrugjCsxu1/
> >
> > Thank you for the testing and report! I brought this up on GitHub on
> > Friday as I noticed this as well:
> >
> > https://github.com/ClangBuiltLinux/linux/issues/1718
Thanks for the reports. I'll take a look at filing additional bug
reports against clang, then moving the definition of
__kretprobe_trampoline to out of line assembler.
> >
> > It sounds like we can avoid this by rewriting __kretprobe_trampoline()
> > in out of line assembly but I have not had a chance to sit down and try
> > it.
> >
> > Cheers,
> > Nathan
>
--
Thanks,
~Nick Desaulniers
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives
2022-09-26 17:42 ` Nick Desaulniers
@ 2022-09-26 18:02 ` Russell King (Oracle)
2022-09-26 18:17 ` Nick Desaulniers
0 siblings, 1 reply; 13+ messages in thread
From: Russell King (Oracle) @ 2022-09-26 18:02 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Nathan Chancellor, Naresh Kamboju, open list, Linux ARM,
Arnd Bergmann, Masami Hiramatsu, regressions, lkft-triage, llvm
On Mon, Sep 26, 2022 at 10:42:45AM -0700, Nick Desaulniers wrote:
> On Mon, Sep 26, 2022 at 8:46 AM Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > + our mailing list, I should have added it with that message.
> >
> > On Mon, Sep 26, 2022 at 08:44:05AM -0700, Nathan Chancellor wrote:
> > > Hi Naresh,
> > >
> > > On Mon, Sep 26, 2022 at 06:57:00PM +0530, Naresh Kamboju wrote:
> > > > Following build warnings / errors noticed on arm with clang-13 / 14
> > > > on Linux next-20220923.
> > > >
> > > > Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
> > > >
> > > > Regressions found on arm:
> > > >
> > > > - build-clang-13-bcm2835_defconfig
> > > > - build-clang-nightly-imx_v6_v7_defconfig
> > > > - build-clang-nightly-orion5x_defconfig
> > > > - build-clang-13-keystone_defconfig
> > > > - build-clang-13-omap2plus_defconfig
> > > > - build-clang-14-imx_v6_v7_defconfig
> > > > - build-clang-nightly-omap2plus_defconfig
> > > > - build-clang-nightly-multi_v5_defconfig
> > > > - build-clang-nightly-bcm2835_defconfig
> > > > - build-clang-13-imx_v6_v7_defconfig
> > > > - build-clang-13-imx_v4_v5_defconfig
> > > > - build-clang-14-imx_v4_v5_defconfig
> > > > - build-clang-13-orion5x_defconfig
> > > > - build-clang-14-multi_v5_defconfig-65236a87
> > > > - build-clang-14-lkftconfig
> > > > - build-clang-nightly-imx_v4_v5_defconfig
> > > > - build-clang-13-multi_v5_defconfig
> > > > - build-clang-13-lkftconfig
> > > > - build-clang-nightly-keystone_defconfig
> > > > - build-clang-14-multi_v5_defconfig
> > > > - build-clang-14-orion5x_defconfig
> > > > - build-clang-14-omap2plus_defconfig
> > > > - build-clang-nightly-multi_v5_defconfig-65236a87
> > > > - build-clang-14-bcm2835_defconfig
> > > > - build-clang-14-keystone_defconfig
> > > > - build-clang-nightly-lkftconfig
> > > >
> > > > arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede
> > > > .save or .vsave directives
> > > > "stmdb sp, {sp, lr, pc} \n\t"
> > > > ^
> > > > <inline asm>:3:2: note: instantiated into assembly here
> > > > .save {sp, lr, pc}
> > > > ^
> > > > /builds/linux/arch/arm/probes/kprobes/core.c:412:29: error: .fnstart
> > > > must precede .pad directive
> > > > "stmdb sp!, {r0 - r11} \n\t"
> > > > ^
> > > > <inline asm>:6:2: note: instantiated into assembly here
> > > > .pad #52
> > > > ^
> > > > 2 errors generated.
> > > > make[5]: *** [/builds/linux/scripts/Makefile.build:250:
> > > > arch/arm/probes/kprobes/core.o] Error 1
> > > >
> > > > build log:
> > > > https://builds.tuxbuild.com/2FAyD1qcTlzjIYE7mjrugjCsxu1/
> > >
> > > Thank you for the testing and report! I brought this up on GitHub on
> > > Friday as I noticed this as well:
> > >
> > > https://github.com/ClangBuiltLinux/linux/issues/1718
>
> Thanks for the reports. I'll take a look at filing additional bug
> reports against clang, then moving the definition of
> __kretprobe_trampoline to out of line assembler.
Are you saying that .save should be accepted without a .fnstart?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives
2022-09-26 18:02 ` Russell King (Oracle)
@ 2022-09-26 18:17 ` Nick Desaulniers
2022-09-26 18:37 ` [PATCH] ARM: kprobes: move __kretprobe_trampoline to out of line assembler Nick Desaulniers
0 siblings, 1 reply; 13+ messages in thread
From: Nick Desaulniers @ 2022-09-26 18:17 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Nathan Chancellor, Naresh Kamboju, open list, Linux ARM,
Arnd Bergmann, Masami Hiramatsu, regressions, lkft-triage, llvm
On Mon, Sep 26, 2022 at 11:02 AM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Mon, Sep 26, 2022 at 10:42:45AM -0700, Nick Desaulniers wrote:
> > > On Mon, Sep 26, 2022 at 08:44:05AM -0700, Nathan Chancellor wrote:
> > > > Thank you for the testing and report! I brought this up on GitHub on
> > > > Friday as I noticed this as well:
> > > >
> > > > https://github.com/ClangBuiltLinux/linux/issues/1718
> >
> > Thanks for the reports. I'll take a look at filing additional bug
> > reports against clang, then moving the definition of
> > __kretprobe_trampoline to out of line assembler.
>
> Are you saying that .save should be accepted without a .fnstart?
No. It's just a bug in clang's inline assembler. But it does make
sense to just move it to out of line assembler anyways; having it be
inline provides little to no benefit.
Should I be using UNWIND from arch/arm/include/asm/unwind.h on these
.fnstart/.save/.pad/.fnend directives?
--
Thanks,
~Nick Desaulniers
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] ARM: kprobes: move __kretprobe_trampoline to out of line assembler
2022-09-26 18:17 ` Nick Desaulniers
@ 2022-09-26 18:37 ` Nick Desaulniers
2022-09-26 18:42 ` Nick Desaulniers
2022-10-05 22:59 ` [PATCH] " Ard Biesheuvel
0 siblings, 2 replies; 13+ messages in thread
From: Nick Desaulniers @ 2022-09-26 18:37 UTC (permalink / raw)
To: Russell King, Masami Hiramatsu
Cc: Naveen N . Rao, Anil S Keshavamurthy, David S . Miller,
Arnd Bergmann, Nathan Chancellor, Tom Rix, sparkhuang,
Ard Biesheuvel, Steven Rostedt, Linus Walleij, Chen Zhongjin,
linux-arm-kernel, linux-kernel, linux-arch, llvm, Naresh Kamboju,
regressions, lkft-triage, Nick Desaulniers,
Linux Kernel Functional Testing, Logan Chien
commit 1069c1dd20a3 ("ARM: 9231/1: Recover kretprobes return address for
EABI stack unwinder")
tickled a bug in clang's integrated assembler where the .save and .pad
directives must have corresponding .fnstart directives. The integrated
assembler is unaware that the compiler will be generating the .fnstart
directive.
arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede
.save or .vsave directives
<inline asm>:3:2: note: instantiated into assembly here
.save {sp, lr, pc}
^
arch/arm/probes/kprobes/core.c:412:29: error: .fnstart must precede
.pad directive
<inline asm>:6:2: note: instantiated into assembly here
.pad #52
^
__kretprobe_trampoline's definition is already entirely inline asm. Move
it to out-of-line asm to avoid breaking the build.
Link: https://github.com/llvm/llvm-project/issues/57993
Link: https://github.com/ClangBuiltLinux/linux/issues/1718
Reported-by: Nathan Chancellor <nathan@kernel.org>
Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
Suggested-by: Logan Chien <loganchien@google.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Note: I wasn't quite sure if a Fixes tag against 1069c1dd20a3 was
appropriate here? Either way, if 1069c1dd20a3 gets picked up for stable
without this, it will break clang builds.
arch/arm/probes/kprobes/Makefile | 1 +
arch/arm/probes/kprobes/core.c | 54 ++----------------
.../arm/probes/kprobes/kretprobe-trampoline.S | 55 +++++++++++++++++++
include/asm-generic/kprobes.h | 13 +++--
4 files changed, 69 insertions(+), 54 deletions(-)
create mode 100644 arch/arm/probes/kprobes/kretprobe-trampoline.S
diff --git a/arch/arm/probes/kprobes/Makefile b/arch/arm/probes/kprobes/Makefile
index 6159010dac4a..cdbe9dd99e28 100644
--- a/arch/arm/probes/kprobes/Makefile
+++ b/arch/arm/probes/kprobes/Makefile
@@ -3,6 +3,7 @@ KASAN_SANITIZE_actions-common.o := n
KASAN_SANITIZE_actions-arm.o := n
KASAN_SANITIZE_actions-thumb.o := n
obj-$(CONFIG_KPROBES) += core.o actions-common.o checkers-common.o
+obj-$(CONFIG_KPROBES) += kretprobe-trampoline.o
obj-$(CONFIG_ARM_KPROBES_TEST) += test-kprobes.o
test-kprobes-objs := test-core.o
diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
index 1435b508aa36..17d7e0259e63 100644
--- a/arch/arm/probes/kprobes/core.c
+++ b/arch/arm/probes/kprobes/core.c
@@ -375,58 +375,10 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
return NOTIFY_DONE;
}
-/*
- * When a retprobed function returns, trampoline_handler() is called,
- * calling the kretprobe's handler. We construct a struct pt_regs to
- * give a view of registers r0-r11, sp, lr, and pc to the user
- * return-handler. This is not a complete pt_regs structure, but that
- * should be enough for stacktrace from the return handler with or
- * without pt_regs.
- */
-void __naked __kprobes __kretprobe_trampoline(void)
-{
- __asm__ __volatile__ (
- "ldr lr, =__kretprobe_trampoline \n\t"
-#ifdef CONFIG_FRAME_POINTER
- /* __kretprobe_trampoline makes a framepointer on pt_regs. */
-#ifdef CONFIG_CC_IS_CLANG
- "stmdb sp, {sp, lr, pc} \n\t"
- "sub sp, sp, #12 \n\t"
- /* In clang case, pt_regs->ip = lr. */
- "stmdb sp!, {r0 - r11, lr} \n\t"
- /* fp points regs->r11 (fp) */
- "add fp, sp, #44 \n\t"
-#else /* !CONFIG_CC_IS_CLANG */
- /* In gcc case, pt_regs->ip = fp. */
- "stmdb sp, {fp, sp, lr, pc} \n\t"
- "sub sp, sp, #16 \n\t"
- "stmdb sp!, {r0 - r11} \n\t"
- /* fp points regs->r15 (pc) */
- "add fp, sp, #60 \n\t"
-#endif /* CONFIG_CC_IS_CLANG */
-#else /* !CONFIG_FRAME_POINTER */
- /* store SP, LR on stack and add EABI unwind hint */
- "stmdb sp, {sp, lr, pc} \n\t"
- ".save {sp, lr, pc} \n\t"
- "sub sp, sp, #16 \n\t"
- "stmdb sp!, {r0 - r11} \n\t"
- ".pad #52 \n\t"
-#endif /* CONFIG_FRAME_POINTER */
- "mov r0, sp \n\t"
- "bl trampoline_handler \n\t"
- "mov lr, r0 \n\t"
- "ldmia sp!, {r0 - r11} \n\t"
- "add sp, sp, #16 \n\t"
-#ifdef CONFIG_THUMB2_KERNEL
- "bx lr \n\t"
-#else
- "mov pc, lr \n\t"
-#endif
- : : : "memory");
-}
+/*void __kretprobe_trampoline(void);*/
/* Called from __kretprobe_trampoline */
-static __used __kprobes void *trampoline_handler(struct pt_regs *regs)
+__kprobes void *trampoline_handler(struct pt_regs *regs)
{
return (void *)kretprobe_trampoline_handler(regs, (void *)regs->TRAMP_FP);
}
@@ -434,6 +386,8 @@ static __used __kprobes void *trampoline_handler(struct pt_regs *regs)
void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
struct pt_regs *regs)
{
+ extern void __kretprobe_trampoline(void);
+
ri->ret_addr = (kprobe_opcode_t *)regs->ARM_lr;
ri->fp = (void *)regs->TRAMP_FP;
diff --git a/arch/arm/probes/kprobes/kretprobe-trampoline.S b/arch/arm/probes/kprobes/kretprobe-trampoline.S
new file mode 100644
index 000000000000..261c99b8c17f
--- /dev/null
+++ b/arch/arm/probes/kprobes/kretprobe-trampoline.S
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <linux/linkage.h>
+#include <asm/unwind.h>
+#include <asm-generic/kprobes.h>
+
+/*
+ * When a retprobed function returns, trampoline_handler() is called,
+ * calling the kretprobe's handler. We construct a struct pt_regs to
+ * give a view of registers r0-r11, sp, lr, and pc to the user
+ * return-handler. This is not a complete pt_regs structure, but that
+ * should be enough for stacktrace from the return handler with or
+ * without pt_regs.
+ */
+__KPROBE
+SYM_FUNC_START(__kretprobe_trampoline)
+UNWIND(.fnstart)
+ ldr lr, =__kretprobe_trampoline
+#ifdef CONFIG_FRAME_POINTER
+ /* __kretprobe_trampoline makes a framepointer on pt_regs. */
+#ifdef CONFIG_CC_IS_CLANG
+ stmdb sp, {sp, lr, pc}
+ sub sp, sp, #12
+ /* In clang case, pt_regs->ip = lr. */
+ stmdb sp!, {r0 - r11, lr}
+ /* fp points regs->r11 (fp) */
+ add fp, sp, #44
+#else /* !CONFIG_CC_IS_CLANG */
+ /* In gcc case, pt_regs->ip = fp. */
+ stmdb sp, {fp, sp, lr, pc}
+ sub sp, sp, #16
+ stmdb sp!, {r0 - r11}
+ /* fp points regs->r15 (pc) */
+ add fp, sp, #60
+#endif /* CONFIG_CC_IS_CLANG */
+#else /* !CONFIG_FRAME_POINTER */
+ /* store SP, LR on stack and add EABI unwind hint */
+ stmdb sp, {sp, lr, pc}
+UNWIND(.save {sp, lr, pc})
+ sub sp, sp, #16
+ stmdb sp!, {r0 - r11}
+UNWIND(.pad #52)
+#endif /* CONFIG_FRAME_POINTER */
+ mov r0, sp
+ bl trampoline_handler
+ mov lr, r0
+ ldmia sp!, {r0 - r11}
+ add sp, sp, #16
+#ifdef CONFIG_THUMB2_KERNEL
+ bx lr
+#else
+ mov pc, lr
+#endif
+UNWIND(.fnend)
+SYM_FUNC_END(__kretprobe_trampoline)
diff --git a/include/asm-generic/kprobes.h b/include/asm-generic/kprobes.h
index 060eab094e5a..1509daa281b8 100644
--- a/include/asm-generic/kprobes.h
+++ b/include/asm-generic/kprobes.h
@@ -2,7 +2,11 @@
#ifndef _ASM_GENERIC_KPROBES_H
#define _ASM_GENERIC_KPROBES_H
-#if defined(__KERNEL__) && !defined(__ASSEMBLY__)
+#ifdef __KERNEL__
+
+#ifdef __ASSEMBLY__
+# define __KPROBE .section ".kprobes.text", "ax"
+#else
#ifdef CONFIG_KPROBES
/*
* Blacklist ganerating macro. Specify functions which is not probed
@@ -16,11 +20,12 @@ static unsigned long __used \
/* Use this to forbid a kprobes attach on very low level functions */
# define __kprobes __section(".kprobes.text")
# define nokprobe_inline __always_inline
-#else
+#else /* !defined(CONFIG_KPROBES) */
# define NOKPROBE_SYMBOL(fname)
# define __kprobes
# define nokprobe_inline inline
-#endif
-#endif /* defined(__KERNEL__) && !defined(__ASSEMBLY__) */
+#endif /* defined(CONFIG_KPROBES) */
+#endif /* defined(__ASSEMBLY__) */
+#endif /* defined(__KERNEL__) */
#endif /* _ASM_GENERIC_KPROBES_H */
--
2.37.3.998.g577e59143f-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] ARM: kprobes: move __kretprobe_trampoline to out of line assembler
2022-09-26 18:37 ` [PATCH] ARM: kprobes: move __kretprobe_trampoline to out of line assembler Nick Desaulniers
@ 2022-09-26 18:42 ` Nick Desaulniers
2022-09-27 22:28 ` [PATCH v2] " Nick Desaulniers
2022-10-05 22:59 ` [PATCH] " Ard Biesheuvel
1 sibling, 1 reply; 13+ messages in thread
From: Nick Desaulniers @ 2022-09-26 18:42 UTC (permalink / raw)
To: Russell King, Masami Hiramatsu
Cc: Naveen N . Rao, Anil S Keshavamurthy, David S . Miller,
Arnd Bergmann, Nathan Chancellor, Tom Rix, sparkhuang,
Ard Biesheuvel, Steven Rostedt, Linus Walleij, Chen Zhongjin,
linux-arm-kernel, linux-kernel, linux-arch, llvm, Naresh Kamboju,
regressions, lkft-triage, Linux Kernel Functional Testing,
Logan Chien
On Mon, Sep 26, 2022 at 11:37 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
> index 1435b508aa36..17d7e0259e63 100644
> --- a/arch/arm/probes/kprobes/core.c
> +++ b/arch/arm/probes/kprobes/core.c
> @@ -375,58 +375,10 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
> return NOTIFY_DONE;
> }
>
> -/*
> - * When a retprobed function returns, trampoline_handler() is called,
> - * calling the kretprobe's handler. We construct a struct pt_regs to
> - * give a view of registers r0-r11, sp, lr, and pc to the user
> - * return-handler. This is not a complete pt_regs structure, but that
> - * should be enough for stacktrace from the return handler with or
> - * without pt_regs.
> - */
> -void __naked __kprobes __kretprobe_trampoline(void)
> -{
> - __asm__ __volatile__ (
> - "ldr lr, =__kretprobe_trampoline \n\t"
> -#ifdef CONFIG_FRAME_POINTER
> - /* __kretprobe_trampoline makes a framepointer on pt_regs. */
> -#ifdef CONFIG_CC_IS_CLANG
> - "stmdb sp, {sp, lr, pc} \n\t"
> - "sub sp, sp, #12 \n\t"
> - /* In clang case, pt_regs->ip = lr. */
> - "stmdb sp!, {r0 - r11, lr} \n\t"
> - /* fp points regs->r11 (fp) */
> - "add fp, sp, #44 \n\t"
> -#else /* !CONFIG_CC_IS_CLANG */
> - /* In gcc case, pt_regs->ip = fp. */
> - "stmdb sp, {fp, sp, lr, pc} \n\t"
> - "sub sp, sp, #16 \n\t"
> - "stmdb sp!, {r0 - r11} \n\t"
> - /* fp points regs->r15 (pc) */
> - "add fp, sp, #60 \n\t"
> -#endif /* CONFIG_CC_IS_CLANG */
> -#else /* !CONFIG_FRAME_POINTER */
> - /* store SP, LR on stack and add EABI unwind hint */
> - "stmdb sp, {sp, lr, pc} \n\t"
> - ".save {sp, lr, pc} \n\t"
> - "sub sp, sp, #16 \n\t"
> - "stmdb sp!, {r0 - r11} \n\t"
> - ".pad #52 \n\t"
> -#endif /* CONFIG_FRAME_POINTER */
> - "mov r0, sp \n\t"
> - "bl trampoline_handler \n\t"
> - "mov lr, r0 \n\t"
> - "ldmia sp!, {r0 - r11} \n\t"
> - "add sp, sp, #16 \n\t"
> -#ifdef CONFIG_THUMB2_KERNEL
> - "bx lr \n\t"
> -#else
> - "mov pc, lr \n\t"
> -#endif
> - : : : "memory");
> -}
> +/*void __kretprobe_trampoline(void);*/
^ d'oh, that commented out declaration should have been removed. Will
wait for feedback wrt. usage of UNWIND and Fixes tag to see if a v2 is
necessary, vs what can be modified if/when applied by maintainers.
--
Thanks,
~Nick Desaulniers
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2] ARM: kprobes: move __kretprobe_trampoline to out of line assembler
2022-09-26 18:42 ` Nick Desaulniers
@ 2022-09-27 22:28 ` Nick Desaulniers
2022-09-27 22:35 ` Russell King (Oracle)
0 siblings, 1 reply; 13+ messages in thread
From: Nick Desaulniers @ 2022-09-27 22:28 UTC (permalink / raw)
To: Russell King, Masami Hiramatsu
Cc: Naveen N . Rao, Anil S Keshavamurthy, David S . Miller,
Arnd Bergmann, Nathan Chancellor, Tom Rix, sparkhuang,
Ard Biesheuvel, Steven Rostedt, Linus Walleij, Chen Zhongjin,
linux-arm-kernel, linux-kernel, linux-arch, llvm, Naresh Kamboju,
regressions, lkft-triage, Nick Desaulniers,
Linux Kernel Functional Testing, Logan Chien
commit 1069c1dd20a3 ("ARM: 9231/1: Recover kretprobes return address for
EABI stack unwinder")
tickled a bug in clang's integrated assembler where the .save and .pad
directives must have corresponding .fnstart directives. The integrated
assembler is unaware that the compiler will be generating the .fnstart
directive.
arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede
.save or .vsave directives
<inline asm>:3:2: note: instantiated into assembly here
.save {sp, lr, pc}
^
arch/arm/probes/kprobes/core.c:412:29: error: .fnstart must precede
.pad directive
<inline asm>:6:2: note: instantiated into assembly here
.pad #52
^
__kretprobe_trampoline's definition is already entirely inline asm. Move
it to out-of-line asm to avoid breaking the build.
Link: https://github.com/llvm/llvm-project/issues/57993
Link: https://github.com/ClangBuiltLinux/linux/issues/1718
Reported-by: Nathan Chancellor <nathan@kernel.org>
Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
Suggested-by: Logan Chien <loganchien@google.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Changes v1 -> v2:
* rebase on linux-next again.
* drop commented out declaration of __kretprobe_trampoline from v1.
arch/arm/probes/kprobes/Makefile | 1 +
arch/arm/probes/kprobes/core.c | 50 +----------------
.../arm/probes/kprobes/kretprobe-trampoline.S | 55 +++++++++++++++++++
include/asm-generic/kprobes.h | 13 +++--
4 files changed, 68 insertions(+), 51 deletions(-)
create mode 100644 arch/arm/probes/kprobes/kretprobe-trampoline.S
diff --git a/arch/arm/probes/kprobes/Makefile b/arch/arm/probes/kprobes/Makefile
index 6159010dac4a..cdbe9dd99e28 100644
--- a/arch/arm/probes/kprobes/Makefile
+++ b/arch/arm/probes/kprobes/Makefile
@@ -3,6 +3,7 @@ KASAN_SANITIZE_actions-common.o := n
KASAN_SANITIZE_actions-arm.o := n
KASAN_SANITIZE_actions-thumb.o := n
obj-$(CONFIG_KPROBES) += core.o actions-common.o checkers-common.o
+obj-$(CONFIG_KPROBES) += kretprobe-trampoline.o
obj-$(CONFIG_ARM_KPROBES_TEST) += test-kprobes.o
test-kprobes-objs := test-core.o
diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
index 9090c3a74dcc..53f17529d2cb 100644
--- a/arch/arm/probes/kprobes/core.c
+++ b/arch/arm/probes/kprobes/core.c
@@ -365,54 +365,8 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
return NOTIFY_DONE;
}
-/*
- * When a retprobed function returns, trampoline_handler() is called,
- * calling the kretprobe's handler. We construct a struct pt_regs to
- * give a view of registers r0-r11, sp, lr, and pc to the user
- * return-handler. This is not a complete pt_regs structure, but that
- * should be enough for stacktrace from the return handler with or
- * without pt_regs.
- */
-void __naked __kprobes __kretprobe_trampoline(void)
-{
- __asm__ __volatile__ (
-#ifdef CONFIG_FRAME_POINTER
- "ldr lr, =__kretprobe_trampoline \n\t"
- /* __kretprobe_trampoline makes a framepointer on pt_regs. */
-#ifdef CONFIG_CC_IS_CLANG
- "stmdb sp, {sp, lr, pc} \n\t"
- "sub sp, sp, #12 \n\t"
- /* In clang case, pt_regs->ip = lr. */
- "stmdb sp!, {r0 - r11, lr} \n\t"
- /* fp points regs->r11 (fp) */
- "add fp, sp, #44 \n\t"
-#else /* !CONFIG_CC_IS_CLANG */
- /* In gcc case, pt_regs->ip = fp. */
- "stmdb sp, {fp, sp, lr, pc} \n\t"
- "sub sp, sp, #16 \n\t"
- "stmdb sp!, {r0 - r11} \n\t"
- /* fp points regs->r15 (pc) */
- "add fp, sp, #60 \n\t"
-#endif /* CONFIG_CC_IS_CLANG */
-#else /* !CONFIG_FRAME_POINTER */
- "sub sp, sp, #16 \n\t"
- "stmdb sp!, {r0 - r11} \n\t"
-#endif /* CONFIG_FRAME_POINTER */
- "mov r0, sp \n\t"
- "bl trampoline_handler \n\t"
- "mov lr, r0 \n\t"
- "ldmia sp!, {r0 - r11} \n\t"
- "add sp, sp, #16 \n\t"
-#ifdef CONFIG_THUMB2_KERNEL
- "bx lr \n\t"
-#else
- "mov pc, lr \n\t"
-#endif
- : : : "memory");
-}
-
/* Called from __kretprobe_trampoline */
-static __used __kprobes void *trampoline_handler(struct pt_regs *regs)
+__kprobes void *trampoline_handler(struct pt_regs *regs)
{
return (void *)kretprobe_trampoline_handler(regs, (void *)regs->ARM_fp);
}
@@ -420,6 +374,8 @@ static __used __kprobes void *trampoline_handler(struct pt_regs *regs)
void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
struct pt_regs *regs)
{
+ extern void __kretprobe_trampoline(void);
+
ri->ret_addr = (kprobe_opcode_t *)regs->ARM_lr;
ri->fp = (void *)regs->ARM_fp;
diff --git a/arch/arm/probes/kprobes/kretprobe-trampoline.S b/arch/arm/probes/kprobes/kretprobe-trampoline.S
new file mode 100644
index 000000000000..261c99b8c17f
--- /dev/null
+++ b/arch/arm/probes/kprobes/kretprobe-trampoline.S
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <linux/linkage.h>
+#include <asm/unwind.h>
+#include <asm-generic/kprobes.h>
+
+/*
+ * When a retprobed function returns, trampoline_handler() is called,
+ * calling the kretprobe's handler. We construct a struct pt_regs to
+ * give a view of registers r0-r11, sp, lr, and pc to the user
+ * return-handler. This is not a complete pt_regs structure, but that
+ * should be enough for stacktrace from the return handler with or
+ * without pt_regs.
+ */
+__KPROBE
+SYM_FUNC_START(__kretprobe_trampoline)
+UNWIND(.fnstart)
+ ldr lr, =__kretprobe_trampoline
+#ifdef CONFIG_FRAME_POINTER
+ /* __kretprobe_trampoline makes a framepointer on pt_regs. */
+#ifdef CONFIG_CC_IS_CLANG
+ stmdb sp, {sp, lr, pc}
+ sub sp, sp, #12
+ /* In clang case, pt_regs->ip = lr. */
+ stmdb sp!, {r0 - r11, lr}
+ /* fp points regs->r11 (fp) */
+ add fp, sp, #44
+#else /* !CONFIG_CC_IS_CLANG */
+ /* In gcc case, pt_regs->ip = fp. */
+ stmdb sp, {fp, sp, lr, pc}
+ sub sp, sp, #16
+ stmdb sp!, {r0 - r11}
+ /* fp points regs->r15 (pc) */
+ add fp, sp, #60
+#endif /* CONFIG_CC_IS_CLANG */
+#else /* !CONFIG_FRAME_POINTER */
+ /* store SP, LR on stack and add EABI unwind hint */
+ stmdb sp, {sp, lr, pc}
+UNWIND(.save {sp, lr, pc})
+ sub sp, sp, #16
+ stmdb sp!, {r0 - r11}
+UNWIND(.pad #52)
+#endif /* CONFIG_FRAME_POINTER */
+ mov r0, sp
+ bl trampoline_handler
+ mov lr, r0
+ ldmia sp!, {r0 - r11}
+ add sp, sp, #16
+#ifdef CONFIG_THUMB2_KERNEL
+ bx lr
+#else
+ mov pc, lr
+#endif
+UNWIND(.fnend)
+SYM_FUNC_END(__kretprobe_trampoline)
diff --git a/include/asm-generic/kprobes.h b/include/asm-generic/kprobes.h
index 060eab094e5a..1509daa281b8 100644
--- a/include/asm-generic/kprobes.h
+++ b/include/asm-generic/kprobes.h
@@ -2,7 +2,11 @@
#ifndef _ASM_GENERIC_KPROBES_H
#define _ASM_GENERIC_KPROBES_H
-#if defined(__KERNEL__) && !defined(__ASSEMBLY__)
+#ifdef __KERNEL__
+
+#ifdef __ASSEMBLY__
+# define __KPROBE .section ".kprobes.text", "ax"
+#else
#ifdef CONFIG_KPROBES
/*
* Blacklist ganerating macro. Specify functions which is not probed
@@ -16,11 +20,12 @@ static unsigned long __used \
/* Use this to forbid a kprobes attach on very low level functions */
# define __kprobes __section(".kprobes.text")
# define nokprobe_inline __always_inline
-#else
+#else /* !defined(CONFIG_KPROBES) */
# define NOKPROBE_SYMBOL(fname)
# define __kprobes
# define nokprobe_inline inline
-#endif
-#endif /* defined(__KERNEL__) && !defined(__ASSEMBLY__) */
+#endif /* defined(CONFIG_KPROBES) */
+#endif /* defined(__ASSEMBLY__) */
+#endif /* defined(__KERNEL__) */
#endif /* _ASM_GENERIC_KPROBES_H */
--
2.37.3.998.g577e59143f-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2] ARM: kprobes: move __kretprobe_trampoline to out of line assembler
2022-09-27 22:28 ` [PATCH v2] " Nick Desaulniers
@ 2022-09-27 22:35 ` Russell King (Oracle)
2022-09-28 16:39 ` Nick Desaulniers
0 siblings, 1 reply; 13+ messages in thread
From: Russell King (Oracle) @ 2022-09-27 22:35 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Masami Hiramatsu, Naveen N . Rao, Anil S Keshavamurthy,
David S . Miller, Arnd Bergmann, Nathan Chancellor, Tom Rix,
sparkhuang, Ard Biesheuvel, Steven Rostedt, Linus Walleij,
Chen Zhongjin, linux-arm-kernel, linux-kernel, linux-arch, llvm,
Naresh Kamboju, regressions, lkft-triage,
Linux Kernel Functional Testing, Logan Chien
On Tue, Sep 27, 2022 at 03:28:51PM -0700, Nick Desaulniers wrote:
> commit 1069c1dd20a3 ("ARM: 9231/1: Recover kretprobes return address for
> EABI stack unwinder")
> tickled a bug in clang's integrated assembler where the .save and .pad
> directives must have corresponding .fnstart directives. The integrated
> assembler is unaware that the compiler will be generating the .fnstart
> directive.
Has it been confirmed that gcc does generate a .fnstart for naked
functions?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] ARM: kprobes: move __kretprobe_trampoline to out of line assembler
2022-09-27 22:35 ` Russell King (Oracle)
@ 2022-09-28 16:39 ` Nick Desaulniers
0 siblings, 0 replies; 13+ messages in thread
From: Nick Desaulniers @ 2022-09-28 16:39 UTC (permalink / raw)
To: Russell King (Oracle), Logan Chien
Cc: Masami Hiramatsu, Naveen N . Rao, Anil S Keshavamurthy,
David S . Miller, Arnd Bergmann, Nathan Chancellor, Tom Rix,
sparkhuang, Ard Biesheuvel, Steven Rostedt, Linus Walleij,
Chen Zhongjin, linux-arm-kernel, linux-kernel, linux-arch, llvm,
Naresh Kamboju, regressions, lkft-triage,
Linux Kernel Functional Testing
On Tue, Sep 27, 2022 at 3:35 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Tue, Sep 27, 2022 at 03:28:51PM -0700, Nick Desaulniers wrote:
> > commit 1069c1dd20a3 ("ARM: 9231/1: Recover kretprobes return address for
> > EABI stack unwinder")
> > tickled a bug in clang's integrated assembler where the .save and .pad
> > directives must have corresponding .fnstart directives. The integrated
> > assembler is unaware that the compiler will be generating the .fnstart
> > directive.
>
> Has it been confirmed that gcc does generate a .fnstart for naked
> functions?
From what I can tell, the presence of __attribute__((naked)) makes no
difference with regards to the emission of the .fnstart directive for
GCC.
One thing I did notice though: https://godbolt.org/z/Mv5GEobc8
GCC will emit .fnstart directives when -fasynchronous-unwind-tables is
specified for C (omitting the directive otherwise), or regardless of
-fasynchronous-unwind-tables/-fno-asynchronous-unwind-tables for C++.
Clang will unconditionally emit .fnstart directives regardless of language mode.
I don't see -fasynchronous-unwind-tables being specified under
arch/arm/. But there are many instances of
UNWIND(.fnstart)
in various .S files under arch/arm/.
https://sourceware.org/binutils/docs/as/ARM-Unwinding-Tutorial.html
https://sourceware.org/binutils/docs/as/ARM-Directives.html#arm_005ffnstart
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
--
Thanks,
~Nick Desaulniers
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] ARM: kprobes: move __kretprobe_trampoline to out of line assembler
2022-09-26 18:37 ` [PATCH] ARM: kprobes: move __kretprobe_trampoline to out of line assembler Nick Desaulniers
2022-09-26 18:42 ` Nick Desaulniers
@ 2022-10-05 22:59 ` Ard Biesheuvel
1 sibling, 0 replies; 13+ messages in thread
From: Ard Biesheuvel @ 2022-10-05 22:59 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Russell King, Masami Hiramatsu, Naveen N . Rao,
Anil S Keshavamurthy, David S . Miller, Arnd Bergmann,
Nathan Chancellor, Tom Rix, sparkhuang, Steven Rostedt,
Linus Walleij, Chen Zhongjin, linux-arm-kernel, linux-kernel,
linux-arch, llvm, Naresh Kamboju, regressions, lkft-triage,
Linux Kernel Functional Testing, Logan Chien
Hi NIck,
On Mon, 26 Sept 2022 at 20:37, Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> commit 1069c1dd20a3 ("ARM: 9231/1: Recover kretprobes return address for
> EABI stack unwinder")
> tickled a bug in clang's integrated assembler where the .save and .pad
> directives must have corresponding .fnstart directives. The integrated
> assembler is unaware that the compiler will be generating the .fnstart
> directive.
>
> arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede
> .save or .vsave directives
> <inline asm>:3:2: note: instantiated into assembly here
> .save {sp, lr, pc}
> ^
> arch/arm/probes/kprobes/core.c:412:29: error: .fnstart must precede
> .pad directive
> <inline asm>:6:2: note: instantiated into assembly here
> .pad #52
> ^
>
> __kretprobe_trampoline's definition is already entirely inline asm. Move
> it to out-of-line asm to avoid breaking the build.
>
I think this is the right approach. I don't think is is even specified
what exactly attribute((naked)) entails in the context of unwind
information
Some nits below, but regardless:
Acked-by: Ard Biesheuvel <ardb@kernel.org>
> Link: https://github.com/llvm/llvm-project/issues/57993
> Link: https://github.com/ClangBuiltLinux/linux/issues/1718
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
> Suggested-by: Logan Chien <loganchien@google.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> Note: I wasn't quite sure if a Fixes tag against 1069c1dd20a3 was
> appropriate here? Either way, if 1069c1dd20a3 gets picked up for stable
> without this, it will break clang builds.
>
> arch/arm/probes/kprobes/Makefile | 1 +
> arch/arm/probes/kprobes/core.c | 54 ++----------------
> .../arm/probes/kprobes/kretprobe-trampoline.S | 55 +++++++++++++++++++
> include/asm-generic/kprobes.h | 13 +++--
> 4 files changed, 69 insertions(+), 54 deletions(-)
> create mode 100644 arch/arm/probes/kprobes/kretprobe-trampoline.S
>
> diff --git a/arch/arm/probes/kprobes/Makefile b/arch/arm/probes/kprobes/Makefile
> index 6159010dac4a..cdbe9dd99e28 100644
> --- a/arch/arm/probes/kprobes/Makefile
> +++ b/arch/arm/probes/kprobes/Makefile
> @@ -3,6 +3,7 @@ KASAN_SANITIZE_actions-common.o := n
> KASAN_SANITIZE_actions-arm.o := n
> KASAN_SANITIZE_actions-thumb.o := n
> obj-$(CONFIG_KPROBES) += core.o actions-common.o checkers-common.o
> +obj-$(CONFIG_KPROBES) += kretprobe-trampoline.o
> obj-$(CONFIG_ARM_KPROBES_TEST) += test-kprobes.o
> test-kprobes-objs := test-core.o
>
> diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
> index 1435b508aa36..17d7e0259e63 100644
> --- a/arch/arm/probes/kprobes/core.c
> +++ b/arch/arm/probes/kprobes/core.c
> @@ -375,58 +375,10 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
> return NOTIFY_DONE;
> }
>
> -/*
> - * When a retprobed function returns, trampoline_handler() is called,
> - * calling the kretprobe's handler. We construct a struct pt_regs to
> - * give a view of registers r0-r11, sp, lr, and pc to the user
> - * return-handler. This is not a complete pt_regs structure, but that
> - * should be enough for stacktrace from the return handler with or
> - * without pt_regs.
> - */
> -void __naked __kprobes __kretprobe_trampoline(void)
> -{
> - __asm__ __volatile__ (
> - "ldr lr, =__kretprobe_trampoline \n\t"
> -#ifdef CONFIG_FRAME_POINTER
> - /* __kretprobe_trampoline makes a framepointer on pt_regs. */
> -#ifdef CONFIG_CC_IS_CLANG
> - "stmdb sp, {sp, lr, pc} \n\t"
> - "sub sp, sp, #12 \n\t"
> - /* In clang case, pt_regs->ip = lr. */
> - "stmdb sp!, {r0 - r11, lr} \n\t"
> - /* fp points regs->r11 (fp) */
> - "add fp, sp, #44 \n\t"
> -#else /* !CONFIG_CC_IS_CLANG */
> - /* In gcc case, pt_regs->ip = fp. */
> - "stmdb sp, {fp, sp, lr, pc} \n\t"
> - "sub sp, sp, #16 \n\t"
> - "stmdb sp!, {r0 - r11} \n\t"
> - /* fp points regs->r15 (pc) */
> - "add fp, sp, #60 \n\t"
> -#endif /* CONFIG_CC_IS_CLANG */
> -#else /* !CONFIG_FRAME_POINTER */
> - /* store SP, LR on stack and add EABI unwind hint */
> - "stmdb sp, {sp, lr, pc} \n\t"
> - ".save {sp, lr, pc} \n\t"
> - "sub sp, sp, #16 \n\t"
> - "stmdb sp!, {r0 - r11} \n\t"
> - ".pad #52 \n\t"
> -#endif /* CONFIG_FRAME_POINTER */
> - "mov r0, sp \n\t"
> - "bl trampoline_handler \n\t"
> - "mov lr, r0 \n\t"
> - "ldmia sp!, {r0 - r11} \n\t"
> - "add sp, sp, #16 \n\t"
> -#ifdef CONFIG_THUMB2_KERNEL
> - "bx lr \n\t"
> -#else
> - "mov pc, lr \n\t"
> -#endif
> - : : : "memory");
> -}
> +/*void __kretprobe_trampoline(void);*/
>
> /* Called from __kretprobe_trampoline */
> -static __used __kprobes void *trampoline_handler(struct pt_regs *regs)
> +__kprobes void *trampoline_handler(struct pt_regs *regs)
> {
> return (void *)kretprobe_trampoline_handler(regs, (void *)regs->TRAMP_FP);
> }
> @@ -434,6 +386,8 @@ static __used __kprobes void *trampoline_handler(struct pt_regs *regs)
> void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
> struct pt_regs *regs)
> {
> + extern void __kretprobe_trampoline(void);
> +
> ri->ret_addr = (kprobe_opcode_t *)regs->ARM_lr;
> ri->fp = (void *)regs->TRAMP_FP;
>
> diff --git a/arch/arm/probes/kprobes/kretprobe-trampoline.S b/arch/arm/probes/kprobes/kretprobe-trampoline.S
> new file mode 100644
> index 000000000000..261c99b8c17f
> --- /dev/null
> +++ b/arch/arm/probes/kprobes/kretprobe-trampoline.S
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#include <linux/linkage.h>
> +#include <asm/unwind.h>
> +#include <asm-generic/kprobes.h>
> +
> +/*
> + * When a retprobed function returns, trampoline_handler() is called,
> + * calling the kretprobe's handler. We construct a struct pt_regs to
> + * give a view of registers r0-r11, sp, lr, and pc to the user
> + * return-handler. This is not a complete pt_regs structure, but that
> + * should be enough for stacktrace from the return handler with or
> + * without pt_regs.
> + */
> +__KPROBE
> +SYM_FUNC_START(__kretprobe_trampoline)
> +UNWIND(.fnstart)
> + ldr lr, =__kretprobe_trampoline
Nit: adr lr, __kretprobe_trampoline
Now that you made a clean spot, might as well clean this up a bit
(although perhaps not in the same patch). I'd merge the
!CONFIG_FRAME_POINTER and CC_IS_CLANG cases, and just put the UNWIND()
directives in there.
> +#ifdef CONFIG_FRAME_POINTER
Drop this
> + /* __kretprobe_trampoline makes a framepointer on pt_regs. */
> +#ifdef CONFIG_CC_IS_CLANG
|| !defined(CONFIG_FRAME_POINTER)
> + stmdb sp, {sp, lr, pc}
> + sub sp, sp, #12
UNWIND(.save {sp, lr, pc})
> + /* In clang case, pt_regs->ip = lr. */
For .S, it is more idiomatic to put the comments in a column on the
right. This reduces visual clutter a lot, so I think it would be worth
the effort.
> + stmdb sp!, {r0 - r11, lr}
UNWIND(.pad #52)
> + /* fp points regs->r11 (fp) */
> + add fp, sp, #44
> +#else /* !CONFIG_CC_IS_CLANG */
> + /* In gcc case, pt_regs->ip = fp. */
> + stmdb sp, {fp, sp, lr, pc}
> + sub sp, sp, #16
> + stmdb sp!, {r0 - r11}
> + /* fp points regs->r15 (pc) */
> + add fp, sp, #60
> +#endif /* CONFIG_CC_IS_CLANG */
> +#else /* !CONFIG_FRAME_POINTER */
> + /* store SP, LR on stack and add EABI unwind hint */
> + stmdb sp, {sp, lr, pc}
> +UNWIND(.save {sp, lr, pc})
> + sub sp, sp, #16
> + stmdb sp!, {r0 - r11}
> +UNWIND(.pad #52)
> +#endif /* CONFIG_FRAME_POINTER */
> + mov r0, sp
> + bl trampoline_handler
> + mov lr, r0
> + ldmia sp!, {r0 - r11}
> + add sp, sp, #16
> +#ifdef CONFIG_THUMB2_KERNEL
> + bx lr
> +#else
> + mov pc, lr
> +#endif
Please use 'ret lr' here. Note that this code does not even compile
for Thumb2, but 'bx lr' should be used at least on v6+ in any case.
> +UNWIND(.fnend)
> +SYM_FUNC_END(__kretprobe_trampoline)
> diff --git a/include/asm-generic/kprobes.h b/include/asm-generic/kprobes.h
> index 060eab094e5a..1509daa281b8 100644
> --- a/include/asm-generic/kprobes.h
> +++ b/include/asm-generic/kprobes.h
> @@ -2,7 +2,11 @@
> #ifndef _ASM_GENERIC_KPROBES_H
> #define _ASM_GENERIC_KPROBES_H
>
> -#if defined(__KERNEL__) && !defined(__ASSEMBLY__)
> +#ifdef __KERNEL__
> +
> +#ifdef __ASSEMBLY__
> +# define __KPROBE .section ".kprobes.text", "ax"
> +#else
> #ifdef CONFIG_KPROBES
> /*
> * Blacklist ganerating macro. Specify functions which is not probed
> @@ -16,11 +20,12 @@ static unsigned long __used \
> /* Use this to forbid a kprobes attach on very low level functions */
> # define __kprobes __section(".kprobes.text")
> # define nokprobe_inline __always_inline
> -#else
> +#else /* !defined(CONFIG_KPROBES) */
> # define NOKPROBE_SYMBOL(fname)
> # define __kprobes
> # define nokprobe_inline inline
> -#endif
> -#endif /* defined(__KERNEL__) && !defined(__ASSEMBLY__) */
> +#endif /* defined(CONFIG_KPROBES) */
> +#endif /* defined(__ASSEMBLY__) */
> +#endif /* defined(__KERNEL__) */
>
> #endif /* _ASM_GENERIC_KPROBES_H */
> --
> 2.37.3.998.g577e59143f-goog
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-10-05 23:01 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-26 13:27 arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives Naresh Kamboju
2022-09-26 15:44 ` Nathan Chancellor
2022-09-26 15:46 ` Nathan Chancellor
2022-09-26 17:42 ` Nick Desaulniers
2022-09-26 18:02 ` Russell King (Oracle)
2022-09-26 18:17 ` Nick Desaulniers
2022-09-26 18:37 ` [PATCH] ARM: kprobes: move __kretprobe_trampoline to out of line assembler Nick Desaulniers
2022-09-26 18:42 ` Nick Desaulniers
2022-09-27 22:28 ` [PATCH v2] " Nick Desaulniers
2022-09-27 22:35 ` Russell King (Oracle)
2022-09-28 16:39 ` Nick Desaulniers
2022-10-05 22:59 ` [PATCH] " Ard Biesheuvel
2022-09-26 16:11 ` arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives Russell King (Oracle)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).