2021-03-25 22:40:18

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH 0/3] Fix CONFIG_FUNCTION_TRACER with clang

Hi all,

This series fixes function tracing with clang.

Patch 1 adjusts the mcount regex in scripts/recordmcount.pl to handle
the presence of PLT relocations, which happen with clang. Without this,
the mcount_loc section will not be created properly.

Patch 2 adds a workaround for clang less than 13.0.0 in relation to the
mcount symbol name, which was "mcount" rather than "_mcount". This was
written as a separate patch so that it can be reverted when the minimum
clang version is bumped to clang 13.0.0.

Patch 3 avoids a build error when -fpatchable-function-entry is not
available, which is the case with clang less than 13.0.0. This will make
dynamic ftrace unavailable but all of the other function tracing should
work due to the prescence of the previous patch.

I am hoping this series can go in as fixes for 5.12, due to patch 3, but
if not, they can always be backported (patches 1 and 2 are already
marked for stable).

This series has been build tested with gcc-8 through gcc-10 and clang-11
through clang-13 with defconfig and nommu_virt_defconfig plus
CONFIG_FTRACE=y and CONFIG_FUNCTION_TRACER=y then boot tested under
QEMU.

Cheers,
Nathan

Nathan Chancellor (3):
scripts/recordmcount.pl: Fix RISC-V regex for clang
riscv: Workaround mcount name prior to clang-13
riscv: Select HAVE_DYNAMIC_FTRACE when -fpatchable-function-entry is
available

arch/riscv/Kconfig | 2 +-
arch/riscv/include/asm/ftrace.h | 14 ++++++++++++--
arch/riscv/kernel/mcount.S | 10 +++++-----
scripts/recordmcount.pl | 2 +-
4 files changed, 19 insertions(+), 9 deletions(-)

--
2.31.0


2021-03-25 22:41:41

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH 3/3] riscv: Select HAVE_DYNAMIC_FTRACE when -fpatchable-function-entry is available

clang prior to 13.0.0 does not support -fpatchable-function-entry for
RISC-V.

clang: error: unsupported option '-fpatchable-function-entry=8' for target 'riscv64-unknown-linux-gnu'

To avoid this error, only select HAVE_DYNAMIC_FTRACE when this option is
not available.

Fixes: afc76b8b8011 ("riscv: Using PATCHABLE_FUNCTION_ENTRY instead of MCOUNT")
Link: https://github.com/ClangBuiltLinux/linux/issues/1268
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Nathan Chancellor <[email protected]>
---
arch/riscv/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 87d7b52f278f..ba1d07640b66 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -227,7 +227,7 @@ config ARCH_RV64I
bool "RV64I"
select 64BIT
select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && GCC_VERSION >= 50000
- select HAVE_DYNAMIC_FTRACE if MMU
+ select HAVE_DYNAMIC_FTRACE if MMU && $(cc-option,-fpatchable-function-entry=8)
select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
select HAVE_FTRACE_MCOUNT_RECORD
select HAVE_FUNCTION_GRAPH_TRACER
--
2.31.0

2021-03-25 22:42:47

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH 1/3] scripts/recordmcount.pl: Fix RISC-V regex for clang

Clang can generate R_RISCV_CALL_PLT relocations to _mcount:

$ llvm-objdump -dr build/riscv/init/main.o | rg mcount
000000000000000e: R_RISCV_CALL_PLT _mcount
000000000000004e: R_RISCV_CALL_PLT _mcount

After this, the __start_mcount_loc section is properly generated and
function tracing still works.

Cc: [email protected]
Link: https://github.com/ClangBuiltLinux/linux/issues/1331
Signed-off-by: Nathan Chancellor <[email protected]>
---
scripts/recordmcount.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 867860ea57da..a36df04cfa09 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -392,7 +392,7 @@ if ($arch eq "x86_64") {
$mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\s_mcount\$";
} elsif ($arch eq "riscv") {
$function_regex = "^([0-9a-fA-F]+)\\s+<([^.0-9][0-9a-zA-Z_\\.]+)>:";
- $mcount_regex = "^\\s*([0-9a-fA-F]+):\\sR_RISCV_CALL\\s_mcount\$";
+ $mcount_regex = "^\\s*([0-9a-fA-F]+):\\sR_RISCV_CALL(_PLT)?\\s_mcount\$";
$type = ".quad";
$alignment = 2;
} elsif ($arch eq "nds32") {
--
2.31.0

2021-03-25 22:42:47

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH 2/3] riscv: Workaround mcount name prior to clang-13

Prior to clang 13.0.0, the RISC-V name for the mcount symbol was
"mcount", which differs from the GCC version of "_mcount", which results
in the following errors:

riscv64-linux-gnu-ld: init/main.o: in function `__traceiter_initcall_level':
main.c:(.text+0xe): undefined reference to `mcount'
riscv64-linux-gnu-ld: init/main.o: in function `__traceiter_initcall_start':
main.c:(.text+0x4e): undefined reference to `mcount'
riscv64-linux-gnu-ld: init/main.o: in function `__traceiter_initcall_finish':
main.c:(.text+0x92): undefined reference to `mcount'
riscv64-linux-gnu-ld: init/main.o: in function `.LBB32_28':
main.c:(.text+0x30c): undefined reference to `mcount'
riscv64-linux-gnu-ld: init/main.o: in function `free_initmem':
main.c:(.text+0x54c): undefined reference to `mcount'

This has been corrected in https://reviews.llvm.org/D98881 but the
minimum supported clang version is 10.0.1. To avoid build errors and to
gain a working function tracer, adjust the name of the mcount symbol for
older versions of clang in mount.S and recordmcount.pl.

Cc: [email protected]
Link: https://github.com/ClangBuiltLinux/linux/issues/1331
Signed-off-by: Nathan Chancellor <[email protected]>
---
arch/riscv/include/asm/ftrace.h | 14 ++++++++++++--
arch/riscv/kernel/mcount.S | 10 +++++-----
scripts/recordmcount.pl | 2 +-
3 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
index 845002cc2e57..04dad3380041 100644
--- a/arch/riscv/include/asm/ftrace.h
+++ b/arch/riscv/include/asm/ftrace.h
@@ -13,9 +13,19 @@
#endif
#define HAVE_FUNCTION_GRAPH_RET_ADDR_PTR

+/*
+ * Clang prior to 13 had "mcount" instead of "_mcount":
+ * https://reviews.llvm.org/D98881
+ */
+#if defined(CONFIG_CC_IS_GCC) || CONFIG_CLANG_VERSION >= 130000
+#define MCOUNT_NAME _mcount
+#else
+#define MCOUNT_NAME mcount
+#endif
+
#define ARCH_SUPPORTS_FTRACE_OPS 1
#ifndef __ASSEMBLY__
-void _mcount(void);
+void MCOUNT_NAME(void);
static inline unsigned long ftrace_call_adjust(unsigned long addr)
{
return addr;
@@ -36,7 +46,7 @@ struct dyn_arch_ftrace {
* both auipc and jalr at the same time.
*/

-#define MCOUNT_ADDR ((unsigned long)_mcount)
+#define MCOUNT_ADDR ((unsigned long)MCOUNT_NAME)
#define JALR_SIGN_MASK (0x00000800)
#define JALR_OFFSET_MASK (0x00000fff)
#define AUIPC_OFFSET_MASK (0xfffff000)
diff --git a/arch/riscv/kernel/mcount.S b/arch/riscv/kernel/mcount.S
index 8a5593ff9ff3..6d462681c9c0 100644
--- a/arch/riscv/kernel/mcount.S
+++ b/arch/riscv/kernel/mcount.S
@@ -47,8 +47,8 @@

ENTRY(ftrace_stub)
#ifdef CONFIG_DYNAMIC_FTRACE
- .global _mcount
- .set _mcount, ftrace_stub
+ .global MCOUNT_NAME
+ .set MCOUNT_NAME, ftrace_stub
#endif
ret
ENDPROC(ftrace_stub)
@@ -78,7 +78,7 @@ ENDPROC(return_to_handler)
#endif

#ifndef CONFIG_DYNAMIC_FTRACE
-ENTRY(_mcount)
+ENTRY(MCOUNT_NAME)
la t4, ftrace_stub
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
la t0, ftrace_graph_return
@@ -124,6 +124,6 @@ do_trace:
jalr t5
RESTORE_ABI_STATE
ret
-ENDPROC(_mcount)
+ENDPROC(MCOUNT_NAME)
#endif
-EXPORT_SYMBOL(_mcount)
+EXPORT_SYMBOL(MCOUNT_NAME)
diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index a36df04cfa09..7b83a1aaec98 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -392,7 +392,7 @@ if ($arch eq "x86_64") {
$mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\s_mcount\$";
} elsif ($arch eq "riscv") {
$function_regex = "^([0-9a-fA-F]+)\\s+<([^.0-9][0-9a-zA-Z_\\.]+)>:";
- $mcount_regex = "^\\s*([0-9a-fA-F]+):\\sR_RISCV_CALL(_PLT)?\\s_mcount\$";
+ $mcount_regex = "^\\s*([0-9a-fA-F]+):\\sR_RISCV_CALL(_PLT)?\\s_?mcount\$";
$type = ".quad";
$alignment = 2;
} elsif ($arch eq "nds32") {
--
2.31.0

2021-03-25 23:40:29

by Fangrui Song

[permalink] [raw]
Subject: Re: [PATCH 3/3] riscv: Select HAVE_DYNAMIC_FTRACE when -fpatchable-function-entry is available

On 2021-03-25, Nathan Chancellor wrote:
>clang prior to 13.0.0 does not support -fpatchable-function-entry for
>RISC-V.
>
>clang: error: unsupported option '-fpatchable-function-entry=8' for target 'riscv64-unknown-linux-gnu'
>
>To avoid this error, only select HAVE_DYNAMIC_FTRACE when this option is
>not available.

If clang -fpatchable-function-entry=8 does not error "unsupported
option" for one target, it means the backend feature is supported on
this target.

Reviewed-by: Fangrui Song <[email protected]>

>Fixes: afc76b8b8011 ("riscv: Using PATCHABLE_FUNCTION_ENTRY instead of MCOUNT")
>Link: https://github.com/ClangBuiltLinux/linux/issues/1268
>Reported-by: kernel test robot <[email protected]>
>Signed-off-by: Nathan Chancellor <[email protected]>
>---
> arch/riscv/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>index 87d7b52f278f..ba1d07640b66 100644
>--- a/arch/riscv/Kconfig
>+++ b/arch/riscv/Kconfig
>@@ -227,7 +227,7 @@ config ARCH_RV64I
> bool "RV64I"
> select 64BIT
> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && GCC_VERSION >= 50000
>- select HAVE_DYNAMIC_FTRACE if MMU
>+ select HAVE_DYNAMIC_FTRACE if MMU && $(cc-option,-fpatchable-function-entry=8)
> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> select HAVE_FTRACE_MCOUNT_RECORD
> select HAVE_FUNCTION_GRAPH_TRACER
>--
>2.31.0
>
>--
>You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
>To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
>To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20210325223807.2423265-4-nathan%40kernel.org.

2021-03-25 23:40:34

by Fangrui Song

[permalink] [raw]
Subject: Re: [PATCH 1/3] scripts/recordmcount.pl: Fix RISC-V regex for clang

On 2021-03-25, Nathan Chancellor wrote:
>Clang can generate R_RISCV_CALL_PLT relocations to _mcount:
>
>$ llvm-objdump -dr build/riscv/init/main.o | rg mcount
> 000000000000000e: R_RISCV_CALL_PLT _mcount
> 000000000000004e: R_RISCV_CALL_PLT _mcount
>
>After this, the __start_mcount_loc section is properly generated and
>function tracing still works.
>

R_RISCV_CALL_PLT can replace R_RISCV_CALL in all use cases.
R_RISCV_CALL can/may be deprecated:
https://github.com/ClangBuiltLinux/linux/issues/1331#issuecomment-802468296

Reviewed-by: Fangrui Song <[email protected]>


>Cc: [email protected]
>Link: https://github.com/ClangBuiltLinux/linux/issues/1331
>Signed-off-by: Nathan Chancellor <[email protected]>
>---
> scripts/recordmcount.pl | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
>index 867860ea57da..a36df04cfa09 100755
>--- a/scripts/recordmcount.pl
>+++ b/scripts/recordmcount.pl
>@@ -392,7 +392,7 @@ if ($arch eq "x86_64") {
> $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\s_mcount\$";
> } elsif ($arch eq "riscv") {
> $function_regex = "^([0-9a-fA-F]+)\\s+<([^.0-9][0-9a-zA-Z_\\.]+)>:";
>- $mcount_regex = "^\\s*([0-9a-fA-F]+):\\sR_RISCV_CALL\\s_mcount\$";
>+ $mcount_regex = "^\\s*([0-9a-fA-F]+):\\sR_RISCV_CALL(_PLT)?\\s_mcount\$";
> $type = ".quad";
> $alignment = 2;
> } elsif ($arch eq "nds32") {
>--
>2.31.0
>
>--
>You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
>To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
>To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20210325223807.2423265-2-nathan%40kernel.org.

2021-03-26 08:42:42

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH 0/3] Fix CONFIG_FUNCTION_TRACER with clang

On Thu, Mar 25, 2021 at 11:38 PM Nathan Chancellor <[email protected]> wrote:
>
> Hi all,
>
> This series fixes function tracing with clang.
>
> Patch 1 adjusts the mcount regex in scripts/recordmcount.pl to handle
> the presence of PLT relocations, which happen with clang. Without this,
> the mcount_loc section will not be created properly.
>
> Patch 2 adds a workaround for clang less than 13.0.0 in relation to the
> mcount symbol name, which was "mcount" rather than "_mcount". This was
> written as a separate patch so that it can be reverted when the minimum
> clang version is bumped to clang 13.0.0.
>
> Patch 3 avoids a build error when -fpatchable-function-entry is not
> available, which is the case with clang less than 13.0.0. This will make
> dynamic ftrace unavailable but all of the other function tracing should
> work due to the prescence of the previous patch.
>
> I am hoping this series can go in as fixes for 5.12, due to patch 3, but
> if not, they can always be backported (patches 1 and 2 are already
> marked for stable).
>
> This series has been build tested with gcc-8 through gcc-10 and clang-11
> through clang-13 with defconfig and nommu_virt_defconfig plus
> CONFIG_FTRACE=y and CONFIG_FUNCTION_TRACER=y then boot tested under
> QEMU.
>
> Cheers,
> Nathan
>
> Nathan Chancellor (3):
> scripts/recordmcount.pl: Fix RISC-V regex for clang
> riscv: Workaround mcount name prior to clang-13
> riscv: Select HAVE_DYNAMIC_FTRACE when -fpatchable-function-entry is
> available
>

Does this only fixes stuff for clang + riscv?
If so, please put a label "riscv" also in the cover-letter.

- Sedat -

> arch/riscv/Kconfig | 2 +-
> arch/riscv/include/asm/ftrace.h | 14 ++++++++++++--
> arch/riscv/kernel/mcount.S | 10 +++++-----
> scripts/recordmcount.pl | 2 +-
> 4 files changed, 19 insertions(+), 9 deletions(-)
>
> --
> 2.31.0
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20210325223807.2423265-1-nathan%40kernel.org.

2021-03-26 13:08:58

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH 0/3] Fix CONFIG_FUNCTION_TRACER with clang

On Fri, Mar 26, 2021 at 09:37:55AM +0100, Sedat Dilek wrote:
> On Thu, Mar 25, 2021 at 11:38 PM Nathan Chancellor <[email protected]> wrote:
> >
> > Hi all,
> >
> > This series fixes function tracing with clang.
> >
> > Patch 1 adjusts the mcount regex in scripts/recordmcount.pl to handle
> > the presence of PLT relocations, which happen with clang. Without this,
> > the mcount_loc section will not be created properly.
> >
> > Patch 2 adds a workaround for clang less than 13.0.0 in relation to the
> > mcount symbol name, which was "mcount" rather than "_mcount". This was
> > written as a separate patch so that it can be reverted when the minimum
> > clang version is bumped to clang 13.0.0.
> >
> > Patch 3 avoids a build error when -fpatchable-function-entry is not
> > available, which is the case with clang less than 13.0.0. This will make
> > dynamic ftrace unavailable but all of the other function tracing should
> > work due to the prescence of the previous patch.
> >
> > I am hoping this series can go in as fixes for 5.12, due to patch 3, but
> > if not, they can always be backported (patches 1 and 2 are already
> > marked for stable).
> >
> > This series has been build tested with gcc-8 through gcc-10 and clang-11
> > through clang-13 with defconfig and nommu_virt_defconfig plus
> > CONFIG_FTRACE=y and CONFIG_FUNCTION_TRACER=y then boot tested under
> > QEMU.
> >
> > Cheers,
> > Nathan
> >
> > Nathan Chancellor (3):
> > scripts/recordmcount.pl: Fix RISC-V regex for clang
> > riscv: Workaround mcount name prior to clang-13
> > riscv: Select HAVE_DYNAMIC_FTRACE when -fpatchable-function-entry is
> > available
> >
>
> Does this only fixes stuff for clang + riscv?

Yes.

> If so, please put a label "riscv" also in the cover-letter.

Sure, my apologies for not doing that in the first place, I must have
been in a rush with the cover letter.

In my defense, I think the titles of my commit messages and the diffstat
below make that obvious without the tag :)

Cheers,
Nathan

> - Sedat -
>
> > arch/riscv/Kconfig | 2 +-
> > arch/riscv/include/asm/ftrace.h | 14 ++++++++++++--
> > arch/riscv/kernel/mcount.S | 10 +++++-----
> > scripts/recordmcount.pl | 2 +-
> > 4 files changed, 19 insertions(+), 9 deletions(-)
> >
> > --
> > 2.31.0
> >

2021-03-27 12:19:39

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH 0/3] Fix CONFIG_FUNCTION_TRACER with clang

On Fri, Mar 26, 2021 at 2:07 PM Nathan Chancellor <[email protected]> wrote:
>
> On Fri, Mar 26, 2021 at 09:37:55AM +0100, Sedat Dilek wrote:
> > On Thu, Mar 25, 2021 at 11:38 PM Nathan Chancellor <[email protected]> wrote:
> > >
> > > Hi all,
> > >
> > > This series fixes function tracing with clang.
> > >
> > > Patch 1 adjusts the mcount regex in scripts/recordmcount.pl to handle
> > > the presence of PLT relocations, which happen with clang. Without this,
> > > the mcount_loc section will not be created properly.
> > >
> > > Patch 2 adds a workaround for clang less than 13.0.0 in relation to the
> > > mcount symbol name, which was "mcount" rather than "_mcount". This was
> > > written as a separate patch so that it can be reverted when the minimum
> > > clang version is bumped to clang 13.0.0.
> > >
> > > Patch 3 avoids a build error when -fpatchable-function-entry is not
> > > available, which is the case with clang less than 13.0.0. This will make
> > > dynamic ftrace unavailable but all of the other function tracing should
> > > work due to the prescence of the previous patch.
> > >
> > > I am hoping this series can go in as fixes for 5.12, due to patch 3, but
> > > if not, they can always be backported (patches 1 and 2 are already
> > > marked for stable).
> > >
> > > This series has been build tested with gcc-8 through gcc-10 and clang-11
> > > through clang-13 with defconfig and nommu_virt_defconfig plus
> > > CONFIG_FTRACE=y and CONFIG_FUNCTION_TRACER=y then boot tested under
> > > QEMU.
> > >
> > > Cheers,
> > > Nathan
> > >
> > > Nathan Chancellor (3):
> > > scripts/recordmcount.pl: Fix RISC-V regex for clang
> > > riscv: Workaround mcount name prior to clang-13
> > > riscv: Select HAVE_DYNAMIC_FTRACE when -fpatchable-function-entry is
> > > available
> > >
> >
> > Does this only fixes stuff for clang + riscv?
>
> Yes.
>
> > If so, please put a label "riscv" also in the cover-letter.
>
> Sure, my apologies for not doing that in the first place, I must have
> been in a rush with the cover letter.
>
> In my defense, I think the titles of my commit messages and the diffstat
> below make that obvious without the tag :)
>

No need for any apologies.
I was fooled as you sent two triple-patchset nearly simultaneously.
This riscv patchset here was not of interest to me.

- Sedat -

>
> > - Sedat -
> >
> > > arch/riscv/Kconfig | 2 +-
> > > arch/riscv/include/asm/ftrace.h | 14 ++++++++++++--
> > > arch/riscv/kernel/mcount.S | 10 +++++-----
> > > scripts/recordmcount.pl | 2 +-
> > > 4 files changed, 19 insertions(+), 9 deletions(-)
> > >
> > > --
> > > 2.31.0
> > >

2021-03-29 18:36:22

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH 2/3] riscv: Workaround mcount name prior to clang-13

On Thu, Mar 25, 2021 at 3:38 PM Nathan Chancellor <[email protected]> wrote:
>
> Prior to clang 13.0.0, the RISC-V name for the mcount symbol was
> "mcount", which differs from the GCC version of "_mcount", which results
> in the following errors:
>
> riscv64-linux-gnu-ld: init/main.o: in function `__traceiter_initcall_level':
> main.c:(.text+0xe): undefined reference to `mcount'
> riscv64-linux-gnu-ld: init/main.o: in function `__traceiter_initcall_start':
> main.c:(.text+0x4e): undefined reference to `mcount'
> riscv64-linux-gnu-ld: init/main.o: in function `__traceiter_initcall_finish':
> main.c:(.text+0x92): undefined reference to `mcount'
> riscv64-linux-gnu-ld: init/main.o: in function `.LBB32_28':
> main.c:(.text+0x30c): undefined reference to `mcount'
> riscv64-linux-gnu-ld: init/main.o: in function `free_initmem':
> main.c:(.text+0x54c): undefined reference to `mcount'
>
> This has been corrected in https://reviews.llvm.org/D98881 but the
> minimum supported clang version is 10.0.1. To avoid build errors and to
> gain a working function tracer, adjust the name of the mcount symbol for
> older versions of clang in mount.S and recordmcount.pl.
>
> Cc: [email protected]
> Link: https://github.com/ClangBuiltLinux/linux/issues/1331
> Signed-off-by: Nathan Chancellor <[email protected]>

Thanks for keeping this alive on clang-10, and resolving it for future releases!
Reviewed-by: Nick Desaulniers <[email protected]>

> ---
> arch/riscv/include/asm/ftrace.h | 14 ++++++++++++--
> arch/riscv/kernel/mcount.S | 10 +++++-----
> scripts/recordmcount.pl | 2 +-
> 3 files changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> index 845002cc2e57..04dad3380041 100644
> --- a/arch/riscv/include/asm/ftrace.h
> +++ b/arch/riscv/include/asm/ftrace.h
> @@ -13,9 +13,19 @@
> #endif
> #define HAVE_FUNCTION_GRAPH_RET_ADDR_PTR
>
> +/*
> + * Clang prior to 13 had "mcount" instead of "_mcount":
> + * https://reviews.llvm.org/D98881
> + */
> +#if defined(CONFIG_CC_IS_GCC) || CONFIG_CLANG_VERSION >= 130000
> +#define MCOUNT_NAME _mcount
> +#else
> +#define MCOUNT_NAME mcount
> +#endif
> +
> #define ARCH_SUPPORTS_FTRACE_OPS 1
> #ifndef __ASSEMBLY__
> -void _mcount(void);
> +void MCOUNT_NAME(void);
> static inline unsigned long ftrace_call_adjust(unsigned long addr)
> {
> return addr;
> @@ -36,7 +46,7 @@ struct dyn_arch_ftrace {
> * both auipc and jalr at the same time.
> */
>
> -#define MCOUNT_ADDR ((unsigned long)_mcount)
> +#define MCOUNT_ADDR ((unsigned long)MCOUNT_NAME)
> #define JALR_SIGN_MASK (0x00000800)
> #define JALR_OFFSET_MASK (0x00000fff)
> #define AUIPC_OFFSET_MASK (0xfffff000)
> diff --git a/arch/riscv/kernel/mcount.S b/arch/riscv/kernel/mcount.S
> index 8a5593ff9ff3..6d462681c9c0 100644
> --- a/arch/riscv/kernel/mcount.S
> +++ b/arch/riscv/kernel/mcount.S
> @@ -47,8 +47,8 @@
>
> ENTRY(ftrace_stub)
> #ifdef CONFIG_DYNAMIC_FTRACE
> - .global _mcount
> - .set _mcount, ftrace_stub
> + .global MCOUNT_NAME
> + .set MCOUNT_NAME, ftrace_stub
> #endif
> ret
> ENDPROC(ftrace_stub)
> @@ -78,7 +78,7 @@ ENDPROC(return_to_handler)
> #endif
>
> #ifndef CONFIG_DYNAMIC_FTRACE
> -ENTRY(_mcount)
> +ENTRY(MCOUNT_NAME)
> la t4, ftrace_stub
> #ifdef CONFIG_FUNCTION_GRAPH_TRACER
> la t0, ftrace_graph_return
> @@ -124,6 +124,6 @@ do_trace:
> jalr t5
> RESTORE_ABI_STATE
> ret
> -ENDPROC(_mcount)
> +ENDPROC(MCOUNT_NAME)
> #endif
> -EXPORT_SYMBOL(_mcount)
> +EXPORT_SYMBOL(MCOUNT_NAME)
> diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
> index a36df04cfa09..7b83a1aaec98 100755
> --- a/scripts/recordmcount.pl
> +++ b/scripts/recordmcount.pl
> @@ -392,7 +392,7 @@ if ($arch eq "x86_64") {
> $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\s_mcount\$";
> } elsif ($arch eq "riscv") {
> $function_regex = "^([0-9a-fA-F]+)\\s+<([^.0-9][0-9a-zA-Z_\\.]+)>:";
> - $mcount_regex = "^\\s*([0-9a-fA-F]+):\\sR_RISCV_CALL(_PLT)?\\s_mcount\$";
> + $mcount_regex = "^\\s*([0-9a-fA-F]+):\\sR_RISCV_CALL(_PLT)?\\s_?mcount\$";
> $type = ".quad";
> $alignment = 2;
> } elsif ($arch eq "nds32") {
> --
> 2.31.0
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20210325223807.2423265-3-nathan%40kernel.org.



--
Thanks,
~Nick Desaulniers

2021-04-11 21:41:25

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH 0/3] Fix CONFIG_FUNCTION_TRACER with clang

On Thu, 25 Mar 2021 15:38:04 PDT (-0700), [email protected] wrote:
> Hi all,
>
> This series fixes function tracing with clang.
>
> Patch 1 adjusts the mcount regex in scripts/recordmcount.pl to handle
> the presence of PLT relocations, which happen with clang. Without this,
> the mcount_loc section will not be created properly.
>
> Patch 2 adds a workaround for clang less than 13.0.0 in relation to the
> mcount symbol name, which was "mcount" rather than "_mcount". This was
> written as a separate patch so that it can be reverted when the minimum
> clang version is bumped to clang 13.0.0.
>
> Patch 3 avoids a build error when -fpatchable-function-entry is not
> available, which is the case with clang less than 13.0.0. This will make
> dynamic ftrace unavailable but all of the other function tracing should
> work due to the prescence of the previous patch.
>
> I am hoping this series can go in as fixes for 5.12, due to patch 3, but
> if not, they can always be backported (patches 1 and 2 are already
> marked for stable).
>
> This series has been build tested with gcc-8 through gcc-10 and clang-11
> through clang-13 with defconfig and nommu_virt_defconfig plus
> CONFIG_FTRACE=y and CONFIG_FUNCTION_TRACER=y then boot tested under
> QEMU.
>
> Cheers,
> Nathan
>
> Nathan Chancellor (3):
> scripts/recordmcount.pl: Fix RISC-V regex for clang
> riscv: Workaround mcount name prior to clang-13
> riscv: Select HAVE_DYNAMIC_FTRACE when -fpatchable-function-entry is
> available
>
> arch/riscv/Kconfig | 2 +-
> arch/riscv/include/asm/ftrace.h | 14 ++++++++++++--
> arch/riscv/kernel/mcount.S | 10 +++++-----
> scripts/recordmcount.pl | 2 +-
> 4 files changed, 19 insertions(+), 9 deletions(-)

Thanks, these are on for-next.