perf symbol: Add LoongArch case in get_plt_sizes()

Message ID 1683615352-10794-1-git-send-email-yangtiezhu@loongson.cn
State New
Headers
Series perf symbol: Add LoongArch case in get_plt_sizes() |

Commit Message

Tiezhu Yang May 9, 2023, 6:55 a.m. UTC
  We can see the following definitions in bfd/elfnn-loongarch.c:

  #define PLT_HEADER_INSNS 8
  #define PLT_HEADER_SIZE (PLT_HEADER_INSNS * 4)

  #define PLT_ENTRY_INSNS 4
  #define PLT_ENTRY_SIZE (PLT_ENTRY_INSNS * 4)

so plt header size is 32 and plt entry size is 16 on LoongArch,
let us add LoongArch case in get_plt_sizes().

Link: https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=bfd/elfnn-loongarch.c
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---

This is based on 6.4-rc1

 tools/perf/util/symbol-elf.c | 4 ++++
 1 file changed, 4 insertions(+)
  

Comments

Huacai Chen May 18, 2023, 2:11 a.m. UTC | #1
Queued, thanks.

Huacai

On Tue, May 9, 2023 at 2:56 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> We can see the following definitions in bfd/elfnn-loongarch.c:
>
>   #define PLT_HEADER_INSNS 8
>   #define PLT_HEADER_SIZE (PLT_HEADER_INSNS * 4)
>
>   #define PLT_ENTRY_INSNS 4
>   #define PLT_ENTRY_SIZE (PLT_ENTRY_INSNS * 4)
>
> so plt header size is 32 and plt entry size is 16 on LoongArch,
> let us add LoongArch case in get_plt_sizes().
>
> Link: https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=bfd/elfnn-loongarch.c
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>
> This is based on 6.4-rc1
>
>  tools/perf/util/symbol-elf.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index b2ed9cc..5d409c2 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -411,6 +411,10 @@ static bool get_plt_sizes(struct dso *dso, GElf_Ehdr *ehdr, GElf_Shdr *shdr_plt,
>                 *plt_header_size = 32;
>                 *plt_entry_size = 16;
>                 return true;
> +       case EM_LOONGARCH:
> +               *plt_header_size = 32;
> +               *plt_entry_size = 16;
> +               return true;
>         case EM_SPARC:
>                 *plt_header_size = 48;
>                 *plt_entry_size = 12;
> --
> 2.1.0
>
>
  
Leo Yan May 18, 2023, 3:06 a.m. UTC | #2
On Thu, May 18, 2023 at 10:11:27AM +0800, Huacai Chen wrote:
> Queued, thanks.

The patch is fine for me.

Should not perf patches are to be merged via Arnaldo's tree?

Thanks,
Leo

> On Tue, May 9, 2023 at 2:56 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> >
> > We can see the following definitions in bfd/elfnn-loongarch.c:
> >
> >   #define PLT_HEADER_INSNS 8
> >   #define PLT_HEADER_SIZE (PLT_HEADER_INSNS * 4)
> >
> >   #define PLT_ENTRY_INSNS 4
> >   #define PLT_ENTRY_SIZE (PLT_ENTRY_INSNS * 4)
> >
> > so plt header size is 32 and plt entry size is 16 on LoongArch,
> > let us add LoongArch case in get_plt_sizes().
> >
> > Link: https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=bfd/elfnn-loongarch.c
> > Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> > ---
> >
> > This is based on 6.4-rc1
> >
> >  tools/perf/util/symbol-elf.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> > index b2ed9cc..5d409c2 100644
> > --- a/tools/perf/util/symbol-elf.c
> > +++ b/tools/perf/util/symbol-elf.c
> > @@ -411,6 +411,10 @@ static bool get_plt_sizes(struct dso *dso, GElf_Ehdr *ehdr, GElf_Shdr *shdr_plt,
> >                 *plt_header_size = 32;
> >                 *plt_entry_size = 16;
> >                 return true;
> > +       case EM_LOONGARCH:
> > +               *plt_header_size = 32;
> > +               *plt_entry_size = 16;
> > +               return true;
> >         case EM_SPARC:
> >                 *plt_header_size = 48;
> >                 *plt_entry_size = 12;
> > --
> > 2.1.0
> >
> >
  
Huacai Chen May 18, 2023, 3:12 a.m. UTC | #3
On Thu, May 18, 2023 at 11:06 AM Leo Yan <leo.yan@linaro.org> wrote:
>
> On Thu, May 18, 2023 at 10:11:27AM +0800, Huacai Chen wrote:
> > Queued, thanks.
>
> The patch is fine for me.
>
> Should not perf patches are to be merged via Arnaldo's tree?
I think both are OK, if Arnaldo takes this patch, I will drop it.

Huacai
>
> Thanks,
> Leo
>
> > On Tue, May 9, 2023 at 2:56 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> > >
> > > We can see the following definitions in bfd/elfnn-loongarch.c:
> > >
> > >   #define PLT_HEADER_INSNS 8
> > >   #define PLT_HEADER_SIZE (PLT_HEADER_INSNS * 4)
> > >
> > >   #define PLT_ENTRY_INSNS 4
> > >   #define PLT_ENTRY_SIZE (PLT_ENTRY_INSNS * 4)
> > >
> > > so plt header size is 32 and plt entry size is 16 on LoongArch,
> > > let us add LoongArch case in get_plt_sizes().
> > >
> > > Link: https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=bfd/elfnn-loongarch.c
> > > Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> > > ---
> > >
> > > This is based on 6.4-rc1
> > >
> > >  tools/perf/util/symbol-elf.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> > > index b2ed9cc..5d409c2 100644
> > > --- a/tools/perf/util/symbol-elf.c
> > > +++ b/tools/perf/util/symbol-elf.c
> > > @@ -411,6 +411,10 @@ static bool get_plt_sizes(struct dso *dso, GElf_Ehdr *ehdr, GElf_Shdr *shdr_plt,
> > >                 *plt_header_size = 32;
> > >                 *plt_entry_size = 16;
> > >                 return true;
> > > +       case EM_LOONGARCH:
> > > +               *plt_header_size = 32;
> > > +               *plt_entry_size = 16;
> > > +               return true;
> > >         case EM_SPARC:
> > >                 *plt_header_size = 48;
> > >                 *plt_entry_size = 12;
> > > --
> > > 2.1.0
> > >
> > >
  
Leo Yan May 18, 2023, 3:21 a.m. UTC | #4
On Thu, May 18, 2023 at 11:12:26AM +0800, Huacai Chen wrote:
> On Thu, May 18, 2023 at 11:06 AM Leo Yan <leo.yan@linaro.org> wrote:
> >
> > On Thu, May 18, 2023 at 10:11:27AM +0800, Huacai Chen wrote:
> > > Queued, thanks.
> >
> > The patch is fine for me.
> >
> > Should not perf patches are to be merged via Arnaldo's tree?
>
> I think both are OK, if Arnaldo takes this patch, I will drop it.

A good practice is to firstly inquiry the maintainers.

AFAIK, Arnaldo will test perf patches before sending out pull request;
if perf patches are scattered out, it might be out of the testing
radar.

Thanks,
Leo
  
Huacai Chen May 18, 2023, 3:57 a.m. UTC | #5
On Thu, May 18, 2023 at 11:21 AM Leo Yan <leo.yan@linaro.org> wrote:
>
> On Thu, May 18, 2023 at 11:12:26AM +0800, Huacai Chen wrote:
> > On Thu, May 18, 2023 at 11:06 AM Leo Yan <leo.yan@linaro.org> wrote:
> > >
> > > On Thu, May 18, 2023 at 10:11:27AM +0800, Huacai Chen wrote:
> > > > Queued, thanks.
> > >
> > > The patch is fine for me.
> > >
> > > Should not perf patches are to be merged via Arnaldo's tree?
> >
> > I think both are OK, if Arnaldo takes this patch, I will drop it.
>
> A good practice is to firstly inquiry the maintainers.
>
> AFAIK, Arnaldo will test perf patches before sending out pull request;
> if perf patches are scattered out, it might be out of the testing
> radar.
OK, I know, thank you very much.

Huacai
>
> Thanks,
> Leo
  
Leo Yan May 18, 2023, 4:05 a.m. UTC | #6
On Thu, May 18, 2023 at 11:57:29AM +0800, Huacai Chen wrote:
> On Thu, May 18, 2023 at 11:21 AM Leo Yan <leo.yan@linaro.org> wrote:
> >
> > On Thu, May 18, 2023 at 11:12:26AM +0800, Huacai Chen wrote:
> > > On Thu, May 18, 2023 at 11:06 AM Leo Yan <leo.yan@linaro.org> wrote:
> > > >
> > > > On Thu, May 18, 2023 at 10:11:27AM +0800, Huacai Chen wrote:
> > > > > Queued, thanks.
> > > >
> > > > The patch is fine for me.
> > > >
> > > > Should not perf patches are to be merged via Arnaldo's tree?
> > >
> > > I think both are OK, if Arnaldo takes this patch, I will drop it.
> >
> > A good practice is to firstly inquiry the maintainers.
> >
> > AFAIK, Arnaldo will test perf patches before sending out pull request;
> > if perf patches are scattered out, it might be out of the testing
> > radar.
> OK, I know, thank you very much.

You are welcome!

I found the code base for bfd:
https://github.com/bminor/binutils-gdb/blob/master/bfd/elfnn-loongarch.c

And this patch is consistent with above link, FWIW:

Reviewed-by: Leo Yan <leo.yan@linaro.org>
  
Arnaldo Carvalho de Melo May 18, 2023, 12:16 p.m. UTC | #7
Em Thu, May 18, 2023 at 12:05:53PM +0800, Leo Yan escreveu:
> On Thu, May 18, 2023 at 11:57:29AM +0800, Huacai Chen wrote:
> > On Thu, May 18, 2023 at 11:21 AM Leo Yan <leo.yan@linaro.org> wrote:
> > >
> > > On Thu, May 18, 2023 at 11:12:26AM +0800, Huacai Chen wrote:
> > > > On Thu, May 18, 2023 at 11:06 AM Leo Yan <leo.yan@linaro.org> wrote:
> > > > >
> > > > > On Thu, May 18, 2023 at 10:11:27AM +0800, Huacai Chen wrote:
> > > > > > Queued, thanks.
> > > > >
> > > > > The patch is fine for me.
> > > > >
> > > > > Should not perf patches are to be merged via Arnaldo's tree?
> > > >
> > > > I think both are OK, if Arnaldo takes this patch, I will drop it.
> > >
> > > A good practice is to firstly inquiry the maintainers.
> > >
> > > AFAIK, Arnaldo will test perf patches before sending out pull request;
> > > if perf patches are scattered out, it might be out of the testing
> > > radar.
> > OK, I know, thank you very much.
> 
> You are welcome!
> 
> I found the code base for bfd:
> https://github.com/bminor/binutils-gdb/blob/master/bfd/elfnn-loongarch.c
> 
> And this patch is consistent with above link, FWIW:
> 
> Reviewed-by: Leo Yan <leo.yan@linaro.org>

Thanks, applied.

- Arnaldo
  
Huacai Chen May 22, 2023, 3:50 a.m. UTC | #8
Hi, Arnaldo,

On Thu, May 18, 2023 at 8:16 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Thu, May 18, 2023 at 12:05:53PM +0800, Leo Yan escreveu:
> > On Thu, May 18, 2023 at 11:57:29AM +0800, Huacai Chen wrote:
> > > On Thu, May 18, 2023 at 11:21 AM Leo Yan <leo.yan@linaro.org> wrote:
> > > >
> > > > On Thu, May 18, 2023 at 11:12:26AM +0800, Huacai Chen wrote:
> > > > > On Thu, May 18, 2023 at 11:06 AM Leo Yan <leo.yan@linaro.org> wrote:
> > > > > >
> > > > > > On Thu, May 18, 2023 at 10:11:27AM +0800, Huacai Chen wrote:
> > > > > > > Queued, thanks.
> > > > > >
> > > > > > The patch is fine for me.
> > > > > >
> > > > > > Should not perf patches are to be merged via Arnaldo's tree?
> > > > >
> > > > > I think both are OK, if Arnaldo takes this patch, I will drop it.
> > > >
> > > > A good practice is to firstly inquiry the maintainers.
> > > >
> > > > AFAIK, Arnaldo will test perf patches before sending out pull request;
> > > > if perf patches are scattered out, it might be out of the testing
> > > > radar.
> > > OK, I know, thank you very much.
> >
> > You are welcome!
> >
> > I found the code base for bfd:
> > https://github.com/bminor/binutils-gdb/blob/master/bfd/elfnn-loongarch.c
> >
> > And this patch is consistent with above link, FWIW:
> >
> > Reviewed-by: Leo Yan <leo.yan@linaro.org>
>
> Thanks, applied.
I'm very sorry that this patch breaks cross-build. We need some
additional modification.

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 5d409c26a22e..b3dbf6ca99a7 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -35,6 +35,10 @@
 #define EM_AARCH64     183  /* ARM 64 bit */
 #endif

+#ifndef EM_LOONGARCH
+#define EM_LOONGARCH   258
+#endif
+
 #ifndef ELF32_ST_VISIBILITY
 #define ELF32_ST_VISIBILITY(o) ((o) & 0x03)
 #endif

Then, drop this patch to get an updated version, or let me send an
incremental patch?

Huacai

>
> - Arnaldo
>
  
Tiezhu Yang May 22, 2023, 7:58 a.m. UTC | #9
On 05/22/2023 11:50 AM, Huacai Chen wrote:
> Hi, Arnaldo,
>
> On Thu, May 18, 2023 at 8:16 PM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
>>
>> Em Thu, May 18, 2023 at 12:05:53PM +0800, Leo Yan escreveu:
>>> On Thu, May 18, 2023 at 11:57:29AM +0800, Huacai Chen wrote:
>>>> On Thu, May 18, 2023 at 11:21 AM Leo Yan <leo.yan@linaro.org> wrote:
>>>>>
>>>>> On Thu, May 18, 2023 at 11:12:26AM +0800, Huacai Chen wrote:
>>>>>> On Thu, May 18, 2023 at 11:06 AM Leo Yan <leo.yan@linaro.org> wrote:
>>>>>>>
>>>>>>> On Thu, May 18, 2023 at 10:11:27AM +0800, Huacai Chen wrote:
>>>>>>>> Queued, thanks.
>>>>>>>
>>>>>>> The patch is fine for me.
>>>>>>>
>>>>>>> Should not perf patches are to be merged via Arnaldo's tree?
>>>>>>
>>>>>> I think both are OK, if Arnaldo takes this patch, I will drop it.
>>>>>
>>>>> A good practice is to firstly inquiry the maintainers.
>>>>>
>>>>> AFAIK, Arnaldo will test perf patches before sending out pull request;
>>>>> if perf patches are scattered out, it might be out of the testing
>>>>> radar.
>>>> OK, I know, thank you very much.
>>>
>>> You are welcome!
>>>
>>> I found the code base for bfd:
>>> https://github.com/bminor/binutils-gdb/blob/master/bfd/elfnn-loongarch.c
>>>
>>> And this patch is consistent with above link, FWIW:
>>>
>>> Reviewed-by: Leo Yan <leo.yan@linaro.org>
>>
>> Thanks, applied.
> I'm very sorry that this patch breaks cross-build. We need some
> additional modification.
>
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 5d409c26a22e..b3dbf6ca99a7 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -35,6 +35,10 @@
>  #define EM_AARCH64     183  /* ARM 64 bit */
>  #endif
>
> +#ifndef EM_LOONGARCH
> +#define EM_LOONGARCH   258
> +#endif
> +
>  #ifndef ELF32_ST_VISIBILITY
>  #define ELF32_ST_VISIBILITY(o) ((o) & 0x03)
>  #endif
>
> Then, drop this patch to get an updated version, or let me send an
> incremental patch?
>

I tested this patch on native LoongArch and x86 system, I did not
hit the build error about undeclared EM_LOONGARCH both on LoongArch
and x86, because EM_LOONGARCH is defined in /usr/include/elf.h

Here is the x86 system info:

[root@fedora yangtiezhu]# cat /etc/fedora-release
Fedora release 38 (Thirty Eight)
[root@fedora yangtiezhu]# uname -m
x86_64
[root@fedora yangtiezhu]# grep -rn -w EM_LOONGARCH /usr/include/
/usr/include/linux/elf-em.h:54:#define EM_LOONGARCH    258    /* 
LoongArch */
/usr/include/linux/audit.h:442:#define AUDIT_ARCH_LOONGARCH32 
(EM_LOONGARCH|__AUDIT_ARCH_LE)
/usr/include/linux/audit.h:443:#define AUDIT_ARCH_LOONGARCH64 
(EM_LOONGARCH|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
/usr/include/elf.h:361:#define EM_LOONGARCH    258    /* LoongArch */
[root@fedora yangtiezhu]# rpm -qf /usr/include/elf.h
glibc-headers-x86-2.37-1.fc38.noarch

If I am missing something, please let me know.

Anyway, it is not a bad thing to add the EM_LOONGARCH definition
to avoid the build error on some systems which have no EM_LOONGARCH
in the glibc header.

Thanks,
Tiezhu
  
Huacai Chen May 23, 2023, 1:47 a.m. UTC | #10
On Mon, May 22, 2023 at 3:59 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
>
>
> On 05/22/2023 11:50 AM, Huacai Chen wrote:
> > Hi, Arnaldo,
> >
> > On Thu, May 18, 2023 at 8:16 PM Arnaldo Carvalho de Melo
> > <acme@kernel.org> wrote:
> >>
> >> Em Thu, May 18, 2023 at 12:05:53PM +0800, Leo Yan escreveu:
> >>> On Thu, May 18, 2023 at 11:57:29AM +0800, Huacai Chen wrote:
> >>>> On Thu, May 18, 2023 at 11:21 AM Leo Yan <leo.yan@linaro.org> wrote:
> >>>>>
> >>>>> On Thu, May 18, 2023 at 11:12:26AM +0800, Huacai Chen wrote:
> >>>>>> On Thu, May 18, 2023 at 11:06 AM Leo Yan <leo.yan@linaro.org> wrote:
> >>>>>>>
> >>>>>>> On Thu, May 18, 2023 at 10:11:27AM +0800, Huacai Chen wrote:
> >>>>>>>> Queued, thanks.
> >>>>>>>
> >>>>>>> The patch is fine for me.
> >>>>>>>
> >>>>>>> Should not perf patches are to be merged via Arnaldo's tree?
> >>>>>>
> >>>>>> I think both are OK, if Arnaldo takes this patch, I will drop it.
> >>>>>
> >>>>> A good practice is to firstly inquiry the maintainers.
> >>>>>
> >>>>> AFAIK, Arnaldo will test perf patches before sending out pull request;
> >>>>> if perf patches are scattered out, it might be out of the testing
> >>>>> radar.
> >>>> OK, I know, thank you very much.
> >>>
> >>> You are welcome!
> >>>
> >>> I found the code base for bfd:
> >>> https://github.com/bminor/binutils-gdb/blob/master/bfd/elfnn-loongarch.c
> >>>
> >>> And this patch is consistent with above link, FWIW:
> >>>
> >>> Reviewed-by: Leo Yan <leo.yan@linaro.org>
> >>
> >> Thanks, applied.
> > I'm very sorry that this patch breaks cross-build. We need some
> > additional modification.
> >
> > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> > index 5d409c26a22e..b3dbf6ca99a7 100644
> > --- a/tools/perf/util/symbol-elf.c
> > +++ b/tools/perf/util/symbol-elf.c
> > @@ -35,6 +35,10 @@
> >  #define EM_AARCH64     183  /* ARM 64 bit */
> >  #endif
> >
> > +#ifndef EM_LOONGARCH
> > +#define EM_LOONGARCH   258
> > +#endif
> > +
> >  #ifndef ELF32_ST_VISIBILITY
> >  #define ELF32_ST_VISIBILITY(o) ((o) & 0x03)
> >  #endif
> >
> > Then, drop this patch to get an updated version, or let me send an
> > incremental patch?
> >
>
> I tested this patch on native LoongArch and x86 system, I did not
> hit the build error about undeclared EM_LOONGARCH both on LoongArch
> and x86, because EM_LOONGARCH is defined in /usr/include/elf.h
>
> Here is the x86 system info:
>
> [root@fedora yangtiezhu]# cat /etc/fedora-release
> Fedora release 38 (Thirty Eight)
> [root@fedora yangtiezhu]# uname -m
> x86_64
> [root@fedora yangtiezhu]# grep -rn -w EM_LOONGARCH /usr/include/
> /usr/include/linux/elf-em.h:54:#define EM_LOONGARCH    258    /*
> LoongArch */
> /usr/include/linux/audit.h:442:#define AUDIT_ARCH_LOONGARCH32
> (EM_LOONGARCH|__AUDIT_ARCH_LE)
> /usr/include/linux/audit.h:443:#define AUDIT_ARCH_LOONGARCH64
> (EM_LOONGARCH|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> /usr/include/elf.h:361:#define EM_LOONGARCH    258    /* LoongArch */
> [root@fedora yangtiezhu]# rpm -qf /usr/include/elf.h
> glibc-headers-x86-2.37-1.fc38.noarch
>
> If I am missing something, please let me know.
Fedora 38 has a very new toolchain, older distribution will meet build errors.

Huacai
>
> Anyway, it is not a bad thing to add the EM_LOONGARCH definition
> to avoid the build error on some systems which have no EM_LOONGARCH
> in the glibc header.
>
> Thanks,
> Tiezhu
>
>
  
Leo Yan May 23, 2023, 7:30 a.m. UTC | #11
On Tue, May 23, 2023 at 09:47:19AM +0800, Huacai Chen wrote:

[...]

> > > I'm very sorry that this patch breaks cross-build. We need some
> > > additional modification.
> > >
> > > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> > > index 5d409c26a22e..b3dbf6ca99a7 100644
> > > --- a/tools/perf/util/symbol-elf.c
> > > +++ b/tools/perf/util/symbol-elf.c
> > > @@ -35,6 +35,10 @@
> > >  #define EM_AARCH64     183  /* ARM 64 bit */
> > >  #endif
> > >
> > > +#ifndef EM_LOONGARCH
> > > +#define EM_LOONGARCH   258
> > > +#endif
> > > +

I confirmed that we must apply this change, otherwise, it will
introduce perf building failure on Ubuntu 22.04 (jammy) with my Arm64
machine.

Sorry I didn't detect the building failure when reviewed this patch!

Thanks,
Leo
  
Tiezhu Yang May 23, 2023, 9:02 a.m. UTC | #12
On 05/23/2023 03:30 PM, Leo Yan wrote:
> On Tue, May 23, 2023 at 09:47:19AM +0800, Huacai Chen wrote:
>
> [...]
>
>>>> I'm very sorry that this patch breaks cross-build. We need some
>>>> additional modification.
>>>>
>>>> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
>>>> index 5d409c26a22e..b3dbf6ca99a7 100644
>>>> --- a/tools/perf/util/symbol-elf.c
>>>> +++ b/tools/perf/util/symbol-elf.c
>>>> @@ -35,6 +35,10 @@
>>>>  #define EM_AARCH64     183  /* ARM 64 bit */
>>>>  #endif
>>>>
>>>> +#ifndef EM_LOONGARCH
>>>> +#define EM_LOONGARCH   258
>>>> +#endif
>>>> +
>
> I confirmed that we must apply this change, otherwise, it will
> introduce perf building failure on Ubuntu 22.04 (jammy) with my Arm64
> machine.
>
> Sorry I didn't detect the building failure when reviewed this patch!
>

Sorry for that, EM_LOONGARCH was added in binutils in 2020 [1],
and then it was added in glibc in 2022 [2], so it may be not
exist on some systems.

For now, I do not find the related commit of this patch in any
branch of acme/linux.git, so it is better to send v2 to avoid
the build error, I will do it as soon as possible.

[1] 
https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=4cf2ad720078
[2] https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=2d83247d90c9

Thanks,
Tiezhu
  

Patch

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index b2ed9cc..5d409c2 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -411,6 +411,10 @@  static bool get_plt_sizes(struct dso *dso, GElf_Ehdr *ehdr, GElf_Shdr *shdr_plt,
 		*plt_header_size = 32;
 		*plt_entry_size = 16;
 		return true;
+	case EM_LOONGARCH:
+		*plt_header_size = 32;
+		*plt_entry_size = 16;
+		return true;
 	case EM_SPARC:
 		*plt_header_size = 48;
 		*plt_entry_size = 12;