[v2] modpost: Skip .llvm.call-graph-profile section check

[PATCH v2] modpost: Skip .llvm.call-graph-profile section check

Posted by Denis Nikitin 2 years, 3 months ago

.llvm.call-graph-profile section is added by clang when the kernel is
built with profiles (e.g. -fprofile-sample-use= or -fprofile-use=).

The section contains edge information derived from text sections,
so .llvm.call-graph-profile itself doesn't need more analysis as
the text sections have been analyzed.

This change fixes the kernel build with clang and a sample profile
which currently fails with:

"FATAL: modpost: Please add code to calculate addend for this architecture"

Signed-off-by: Denis Nikitin <denik@chromium.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
---
 scripts/mod/modpost.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index b29b29707f10..64bd13f7199c 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -761,6 +761,7 @@ static const char *const section_white_list[] =
 	".fmt_slot*",			/* EZchip */
 	".gnu.lto*",
 	".discard.*",
+	".llvm.call-graph-profile",	/* call graph */
 	NULL
 };
 
-- 
2.42.0.rc1.204.g551eb34607-goog

Re: [PATCH v2] modpost: Skip .llvm.call-graph-profile section check

Posted by Fangrui Song 2 years, 3 months ago

On Tue, Aug 22, 2023 at 10:49 AM Denis Nikitin <denik@chromium.org> wrote:
>
> .llvm.call-graph-profile section is added by clang when the kernel is
> built with profiles (e.g. -fprofile-sample-use= or -fprofile-use=).
>
> The section contains edge information derived from text sections,
> so .llvm.call-graph-profile itself doesn't need more analysis as
> the text sections have been analyzed.
>
> This change fixes the kernel build with clang and a sample profile
> which currently fails with:
>
> "FATAL: modpost: Please add code to calculate addend for this architecture"
>
> Signed-off-by: Denis Nikitin <denik@chromium.org>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

Thanks. The new commit message looks good to me.

Reviewed-by: Fangrui Song <maskray@google.com>

> ---
>  scripts/mod/modpost.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index b29b29707f10..64bd13f7199c 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -761,6 +761,7 @@ static const char *const section_white_list[] =
>         ".fmt_slot*",                   /* EZchip */
>         ".gnu.lto*",
>         ".discard.*",
> +       ".llvm.call-graph-profile",     /* call graph */
>         NULL
>  };
>
> --
> 2.42.0.rc1.204.g551eb34607-goog
>


-- 
宋方睿

Re: [PATCH v2] modpost: Skip .llvm.call-graph-profile section check

Posted by Masahiro Yamada 2 years, 3 months ago

On Wed, Aug 23, 2023 at 3:00 AM Fangrui Song <maskray@google.com> wrote:
>
> On Tue, Aug 22, 2023 at 10:49 AM Denis Nikitin <denik@chromium.org> wrote:
> >
> > .llvm.call-graph-profile section is added by clang when the kernel is
> > built with profiles (e.g. -fprofile-sample-use= or -fprofile-use=).
> >
> > The section contains edge information derived from text sections,
> > so .llvm.call-graph-profile itself doesn't need more analysis as
> > the text sections have been analyzed.
> >
> > This change fixes the kernel build with clang and a sample profile
> > which currently fails with:
> >
> > "FATAL: modpost: Please add code to calculate addend for this architecture"


Curious.

This message is only displayed for REL.

(Please not it is located in section_rel() function)


I think modern architectures use RELA instead of REL.
Which architecture are we talking about?


What does the output of this command look like?

$ llvm-readelf -S vmlinux.o | grep  .llvm.call-graph-profile


Is it REL?









> >
> > Signed-off-by: Denis Nikitin <denik@chromium.org>
> > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> Thanks. The new commit message looks good to me.
>
> Reviewed-by: Fangrui Song <maskray@google.com>
>
> > ---
> >  scripts/mod/modpost.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > index b29b29707f10..64bd13f7199c 100644
> > --- a/scripts/mod/modpost.c
> > +++ b/scripts/mod/modpost.c
> > @@ -761,6 +761,7 @@ static const char *const section_white_list[] =
> >         ".fmt_slot*",                   /* EZchip */
> >         ".gnu.lto*",
> >         ".discard.*",
> > +       ".llvm.call-graph-profile",     /* call graph */
> >         NULL
> >  };
> >
> > --
> > 2.42.0.rc1.204.g551eb34607-goog
> >
>
>
> --
> 宋方睿



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2] modpost: Skip .llvm.call-graph-profile section check

Posted by Denis Nikitin 2 years, 3 months ago

On Wed, Aug 23, 2023 at 4:02 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Wed, Aug 23, 2023 at 3:00 AM Fangrui Song <maskray@google.com> wrote:
> >
> > On Tue, Aug 22, 2023 at 10:49 AM Denis Nikitin <denik@chromium.org> wrote:
> > >
> > > .llvm.call-graph-profile section is added by clang when the kernel is
> > > built with profiles (e.g. -fprofile-sample-use= or -fprofile-use=).
> > >
> > > The section contains edge information derived from text sections,
> > > so .llvm.call-graph-profile itself doesn't need more analysis as
> > > the text sections have been analyzed.
> > >
> > > This change fixes the kernel build with clang and a sample profile
> > > which currently fails with:
> > >
> > > "FATAL: modpost: Please add code to calculate addend for this architecture"
>
>
> Curious.
>
> This message is only displayed for REL.
>
> (Please not it is located in section_rel() function)
>
>
> I think modern architectures use RELA instead of REL.
> Which architecture are we talking about?

Aarch64. There was also a report on x86-64 but the error message could be
different there.

>
>
> What does the output of this command look like?
>
> $ llvm-readelf -S vmlinux.o | grep  .llvm.call-graph-profile
>
>
> Is it REL?
>

  [119] .llvm.call-graph-profile LLVM_CALL_GRAPH_PROFILE 0000000000000000
1c74a458 0104c8 08   E  0   0  1
  [120] .rel.llvm.call-graph-profile REL 0000000000000000 1c75a920 041320 10
I 26090 119  8

Thanks,
Denis

Re: [PATCH v2] modpost: Skip .llvm.call-graph-profile section check

Posted by Masahiro Yamada 2 years, 3 months ago

On Thu, Aug 24, 2023 at 8:30 AM Denis Nikitin <denik@chromium.org> wrote:
>
> On Wed, Aug 23, 2023 at 4:02 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Wed, Aug 23, 2023 at 3:00 AM Fangrui Song <maskray@google.com> wrote:
> > >
> > > On Tue, Aug 22, 2023 at 10:49 AM Denis Nikitin <denik@chromium.org> wrote:
> > > >
> > > > .llvm.call-graph-profile section is added by clang when the kernel is
> > > > built with profiles (e.g. -fprofile-sample-use= or -fprofile-use=).
> > > >
> > > > The section contains edge information derived from text sections,
> > > > so .llvm.call-graph-profile itself doesn't need more analysis as
> > > > the text sections have been analyzed.
> > > >
> > > > This change fixes the kernel build with clang and a sample profile
> > > > which currently fails with:
> > > >
> > > > "FATAL: modpost: Please add code to calculate addend for this architecture"
> >
> >
> > Curious.
> >
> > This message is only displayed for REL.
> >
> > (Please not it is located in section_rel() function)
> >
> >
> > I think modern architectures use RELA instead of REL.
> > Which architecture are we talking about?
>
> Aarch64. There was also a report on x86-64 but the error message could be
> different there.
>
> >
> >
> > What does the output of this command look like?
> >
> > $ llvm-readelf -S vmlinux.o | grep  .llvm.call-graph-profile
> >
> >
> > Is it REL?
> >
>
>   [119] .llvm.call-graph-profile LLVM_CALL_GRAPH_PROFILE 0000000000000000
> 1c74a458 0104c8 08   E  0   0  1
>   [120] .rel.llvm.call-graph-profile REL 0000000000000000 1c75a920 041320 10
> I 26090 119  8


Fangrui,

Aarch64 uses RELA for other sections, but REL for this one.

I'd like to confirm if this is an expectation, not a toolchain bug.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2] modpost: Skip .llvm.call-graph-profile section check

Posted by Fangrui Song 2 years, 3 months ago

On Wed, Aug 23, 2023 at 6:34 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Thu, Aug 24, 2023 at 8:30 AM Denis Nikitin <denik@chromium.org> wrote:
> >
> > On Wed, Aug 23, 2023 at 4:02 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > >
> > > On Wed, Aug 23, 2023 at 3:00 AM Fangrui Song <maskray@google.com> wrote:
> > > >
> > > > On Tue, Aug 22, 2023 at 10:49 AM Denis Nikitin <denik@chromium.org> wrote:
> > > > >
> > > > > .llvm.call-graph-profile section is added by clang when the kernel is
> > > > > built with profiles (e.g. -fprofile-sample-use= or -fprofile-use=).
> > > > >
> > > > > The section contains edge information derived from text sections,
> > > > > so .llvm.call-graph-profile itself doesn't need more analysis as
> > > > > the text sections have been analyzed.
> > > > >
> > > > > This change fixes the kernel build with clang and a sample profile
> > > > > which currently fails with:
> > > > >
> > > > > "FATAL: modpost: Please add code to calculate addend for this architecture"
> > >
> > >
> > > Curious.
> > >
> > > This message is only displayed for REL.
> > >
> > > (Please not it is located in section_rel() function)
> > >
> > >
> > > I think modern architectures use RELA instead of REL.
> > > Which architecture are we talking about?
> >
> > Aarch64. There was also a report on x86-64 but the error message could be
> > different there.
> >
> > >
> > >
> > > What does the output of this command look like?
> > >
> > > $ llvm-readelf -S vmlinux.o | grep  .llvm.call-graph-profile
> > >
> > >
> > > Is it REL?
> > >
> >
> >   [119] .llvm.call-graph-profile LLVM_CALL_GRAPH_PROFILE 0000000000000000
> > 1c74a458 0104c8 08   E  0   0  1
> >   [120] .rel.llvm.call-graph-profile REL 0000000000000000 1c75a920 041320 10
> > I 26090 119  8
>
>
> Fangrui,
>
> Aarch64 uses RELA for other sections, but REL for this one.
>
> I'd like to confirm if this is an expectation, not a toolchain bug.

Hi Masahiro,

Yes, using REL is intentional. It makes the relocations of
.llvm.call-graph-profile smaller.
The format encodes the (from,to,count) information with

* the section content holds 'count'
* two R_*_NONE relocations hold 'from' and 'to'. The addend field is
unused, therefore REL is better.


-- 
宋方睿

Re: [PATCH v2] modpost: Skip .llvm.call-graph-profile section check

Posted by Fangrui Song 2 years, 3 months ago

On Wed, Aug 23, 2023 at 4:13 PM Denis Nikitin <denik@chromium.org> wrote:
>
> On Wed, Aug 23, 2023 at 4:02 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Wed, Aug 23, 2023 at 3:00 AM Fangrui Song <maskray@google.com> wrote:
> > >
> > > On Tue, Aug 22, 2023 at 10:49 AM Denis Nikitin <denik@chromium.org> wrote:
> > > >
> > > > .llvm.call-graph-profile section is added by clang when the kernel is
> > > > built with profiles (e.g. -fprofile-sample-use= or -fprofile-use=).
> > > >
> > > > The section contains edge information derived from text sections,
> > > > so .llvm.call-graph-profile itself doesn't need more analysis as
> > > > the text sections have been analyzed.
> > > >
> > > > This change fixes the kernel build with clang and a sample profile
> > > > which currently fails with:
> > > >
> > > > "FATAL: modpost: Please add code to calculate addend for this architecture"
> >
> >
> > Curious.
> >
> > This message is only displayed for REL.
> >
> > (Please not it is located in section_rel() function)
> >
> >
> > I think modern architectures use RELA instead of REL.
> > Which architecture are we talking about?
>
> Aarch64. There was also a report on x86-64 but the error message could be
> different there.

Regarding REL:

The original format of .llvm.call-graph-profile
(SHT_LLVM_CALL_GRAPH_PROFILE=0x6fff4c02) used symbol indices without
relocations and could be corrupted by symbol table change.
https://github.com/llvm/llvm-project/commit/a224c5199b327ed0efcdcd87b6dbf950cf4d9ee1
(2021) changed the format to represent call edge information with
R_*_NONE and changed SHT_LLVM_CALL_GRAPH_PROFILE to 0x6fff4c09.

We don't use the addend field of R_*_NONE relocations, so I proposed
that we use REL for all targets.
My https://github.com/llvm/llvm-project/commit/ca3bdb57fa1ac98b711a735de048c12b5fdd8086
selected REL for .llvm.call-graph-profile

aaelf64 says:

> A binary file may use ``REL`` or ``RELA`` relocations or a mixture of the two (but multiple relocations of the same place must use only one type).

Other psABI documents may be more vague on how REL is used, but as
long as the tool that needs to process it (currently just lld and
readelf like tools) supports it, it's fine.
binutils seems to support REL for all ELF targets, even if its
objcopy/strip may unintentionally convert REL to RELA. lld can consume
RELA SHT_LLVM_CALL_GRAPH_PROFILE.

> >
> >
> > What does the output of this command look like?
> >
> > $ llvm-readelf -S vmlinux.o | grep  .llvm.call-graph-profile
> >
> >
> > Is it REL?
> >
>
>   [119] .llvm.call-graph-profile LLVM_CALL_GRAPH_PROFILE 0000000000000000
> 1c74a458 0104c8 08   E  0   0  1
>   [120] .rel.llvm.call-graph-profile REL 0000000000000000 1c75a920 041320 10
> I 26090 119  8
>
> Thanks,
> Denis

-- 
宋方睿

Re: [PATCH v2] modpost: Skip .llvm.call-graph-profile section check

Posted by Masahiro Yamada 2 years, 3 months ago

On Fri, Aug 25, 2023 at 2:30 AM Fangrui Song <maskray@google.com> wrote:
>
> On Wed, Aug 23, 2023 at 4:13 PM Denis Nikitin <denik@chromium.org> wrote:
> >
> > On Wed, Aug 23, 2023 at 4:02 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > >
> > > On Wed, Aug 23, 2023 at 3:00 AM Fangrui Song <maskray@google.com> wrote:
> > > >
> > > > On Tue, Aug 22, 2023 at 10:49 AM Denis Nikitin <denik@chromium.org> wrote:
> > > > >
> > > > > .llvm.call-graph-profile section is added by clang when the kernel is
> > > > > built with profiles (e.g. -fprofile-sample-use= or -fprofile-use=).
> > > > >
> > > > > The section contains edge information derived from text sections,
> > > > > so .llvm.call-graph-profile itself doesn't need more analysis as
> > > > > the text sections have been analyzed.
> > > > >
> > > > > This change fixes the kernel build with clang and a sample profile
> > > > > which currently fails with:
> > > > >
> > > > > "FATAL: modpost: Please add code to calculate addend for this architecture"
> > >
> > >
> > > Curious.
> > >
> > > This message is only displayed for REL.
> > >
> > > (Please not it is located in section_rel() function)
> > >
> > >
> > > I think modern architectures use RELA instead of REL.
> > > Which architecture are we talking about?
> >
> > Aarch64. There was also a report on x86-64 but the error message could be
> > different there.
>
> Regarding REL:
>
> The original format of .llvm.call-graph-profile
> (SHT_LLVM_CALL_GRAPH_PROFILE=0x6fff4c02) used symbol indices without
> relocations and could be corrupted by symbol table change.
> https://github.com/llvm/llvm-project/commit/a224c5199b327ed0efcdcd87b6dbf950cf4d9ee1
> (2021) changed the format to represent call edge information with
> R_*_NONE and changed SHT_LLVM_CALL_GRAPH_PROFILE to 0x6fff4c09.
>
> We don't use the addend field of R_*_NONE relocations, so I proposed
> that we use REL for all targets.
> My https://github.com/llvm/llvm-project/commit/ca3bdb57fa1ac98b711a735de048c12b5fdd8086
> selected REL for .llvm.call-graph-profile
>
> aaelf64 says:
>
> > A binary file may use ``REL`` or ``RELA`` relocations or a mixture of the two (but multiple relocations of the same place must use only one type).
>
> Other psABI documents may be more vague on how REL is used, but as
> long as the tool that needs to process it (currently just lld and
> readelf like tools) supports it, it's fine.
> binutils seems to support REL for all ELF targets, even if its
> objcopy/strip may unintentionally convert REL to RELA. lld can consume
> RELA SHT_LLVM_CALL_GRAPH_PROFILE.
>
> > >
> > >
> > > What does the output of this command look like?
> > >
> > > $ llvm-readelf -S vmlinux.o | grep  .llvm.call-graph-profile
> > >
> > >
> > > Is it REL?
> > >
> >
> >   [119] .llvm.call-graph-profile LLVM_CALL_GRAPH_PROFILE 0000000000000000
> > 1c74a458 0104c8 08   E  0   0  1
> >   [120] .rel.llvm.call-graph-profile REL 0000000000000000 1c75a920 041320 10
> > I 26090 119  8
> >
> > Thanks,
> > Denis



Thanks, Fangrui.


I'd like the commit log to record the use of REL for
.llvm.call-graph-profile is intentional.


Denis, could you add some more comments?



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2] modpost: Skip .llvm.call-graph-profile section check

Posted by Denis Nikitin 2 years, 3 months ago

>
> Thanks, Fangrui.
>
>
> I'd like the commit log to record the use of REL for
> .llvm.call-graph-profile is intentional.
>
>
> Denis, could you add some more comments?
>

Sure, I will send a new version.

Thanks,
Denis

>
>
> --
> Best Regards
> Masahiro Yamada