| Message ID | 20230822174835.253469-1-denik@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] modpost: Skip .llvm.call-graph-profile section check | expand |
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 >
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 > > > > > -- > 宋方睿
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
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
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.
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.
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?
> > 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
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 };