Hi Shabab,

On Tue, 2022-12-20 at 12:56 +0100, Shahab Vahedi wrote:
> There is no regression in tests for an x86_64 build, while the new
> hello_arc_hs4.ko is added as well.  This is the only meaningful
> test that I could add at the moment, given the features supported
> by this port.
> 
> $ cat tests/test-suite.log
>   ==========================================
>      elfutils 0.188: tests/test-suite.log
>   ==========================================
> 
>   # TOTAL: 236
>   # PASS:  235
>   # SKIP:  1
>   # XFAIL: 0
>   # FAIL:  0
>   # XPASS: 0
>   # ERROR: 0
> 
>   .. contents:: :depth: 2
> 
>   SKIP: run-lfs-symbols.sh
>   ========================
> 
>   LFS testing is irrelevant on this system
>   SKIP run-lfs-symbols.sh (exit status: 77)
> 
> $ cat tests/run-strip-reloc.sh.log
>   runtest hello_i386.ko
>   runtest hello_x86_64.ko
>   runtest hello_ppc64.ko
>   runtest hello_s390.ko
>   runtest hello_aarch64.ko
>   runtest hello_m68k.ko
>   runtest hello_riscv64.ko
>   runtest hello_csky.ko
>   runtest hello_arc_hs4.ko            <-- [ new ARC HS4 test ]
>   runtest /home/shahab/pahole_pkg/elfutils-git/bld_arc/src/strip
>   runtest /home/shahab/pahole_pkg/elfutils-git/bld_arc/src/strip.o
>   runtest strip-uncompressed.o
>   runtest strip-compressed.o
>   runtest testfile-debug-rel-ppc64.o
>   runtest testfile-debug-rel-ppc64-z.o
>   runtest testfile-debug-rel-ppc64-g.o
>   PASS run-strip-reloc.sh (exit status: 0)
> 
> Signed-off-by: Shahab Vahedi <sha...@synopsys.com>
> ---
> Chagelog:
> v3:
>   - Drop libelf/elf.h changes now that they're synced from glibc.
>   - Drop src/elflint.c changes as EM_ARC was already in
> valid_e_machine[].

It was, but you are (also) providing code for EM_ARCV2 don't you?
So I believe you do want:

diff --git a/src/elflint.c b/src/elflint.c
index b9548862..b4eac32f 100644
--- a/src/elflint.c
+++ b/src/elflint.c
@@ -330,6 +330,7 @@ static const int valid_e_machine[] =
     EM_AVR, EM_FR30, EM_D10V, EM_D30V, EM_V850, EM_M32R, EM_MN10300,
     EM_MN10200, EM_PJ, EM_OPENRISC, EM_ARC_A5, EM_XTENSA, EM_ALPHA,
     EM_TILEGX, EM_TILEPRO, EM_AARCH64, EM_BPF, EM_RISCV, EM_CSKY, EM_LOONGARCH,
+    EM_ARCV2
   };
 #define nvalid_e_machine \
   (sizeof (valid_e_machine) / sizeof (valid_e_machine[0]))

>   - Add test for ARC HS4 in run-strip-reloc.
>   - Update ChangLogs and a few more cosemitic changes.
>   - Reword the commit message.
> 
> v2:
>   - Add ChangeLog entries.
>   - Reduce number of changes in libelf/elf.h
>   - Reword the commit message.
> 
>  backends/ChangeLog         |   9 ++++
>  backends/Makefile.am       |   6 ++-
>  backends/arc_init.c        |  55 +++++++++++++++++++++++
>  backends/arc_reloc.def     |  87
> +++++++++++++++++++++++++++++++++++++
>  backends/arc_symbol.c      |  81 ++++++++++++++++++++++++++++++++++
>  libebl/ChangeLog           |   5 +++
>  libebl/eblopenbackend.c    |   2 +
>  tests/ChangeLog            |   6 +++
>  tests/Makefile.am          |   1 +
>  tests/hello_arc_hs4.ko.bz2 | Bin 0 -> 15004 bytes
>  tests/run-strip-reloc.sh   |   4 +-
>  11 files changed, 253 insertions(+), 3 deletions(-)
>  create mode 100644 backends/arc_init.c
>  create mode 100644 backends/arc_reloc.def
>  create mode 100644 backends/arc_symbol.c
>  create mode 100644 tests/hello_arc_hs4.ko.bz2
> 
> diff --git a/backends/ChangeLog b/backends/ChangeLog
> index 5813ddcc..8dc792fa 100644
> --- a/backends/ChangeLog
> +++ b/backends/ChangeLog
> @@ -1,3 +1,12 @@
> +2022-12-20  Shahab Vahedi  <sha...@synopsys.com>
> +
> +     * Makefile.am (modules): Add arc.
> +     (arc_SRCS): Added.
> +     (libebl_backends_a_SOURCES): Append arc_SRCS.
> +     * arc_init.c: New file.
> +     * arc_reloc.def: New file.
> +     * arc_symbol.c: New file.
> +
>  2022-12-02  Hengqi Chen  <hengqi.c...@gmail.com>
>  
>       * Makefile.am (modules): Add loongarch.
> diff --git a/backends/Makefile.am b/backends/Makefile.am
> index 0824123d..f373e5fb 100644
> --- a/backends/Makefile.am
> +++ b/backends/Makefile.am
> @@ -37,7 +37,7 @@ AM_CPPFLAGS += -I$(top_srcdir)/libebl
> -I$(top_srcdir)/libasm \
>  noinst_LIBRARIES = libebl_backends.a libebl_backends_pic.a
>  
>  modules = i386 sh x86_64 ia64 alpha arm aarch64 sparc ppc ppc64 s390
> \
> -       m68k bpf riscv csky loongarch
> +       m68k bpf riscv csky loongarch arc
>  
>  i386_SRCS = i386_init.c i386_symbol.c i386_corenote.c i386_cfi.c \
>           i386_retval.c i386_regs.c i386_auxv.c \
> @@ -98,12 +98,14 @@ csky_SRCS = csky_attrs.c csky_init.c
> csky_symbol.c csky_cfi.c \
>  
>  loongarch_SRCS = loongarch_init.c loongarch_symbol.c
>  
> +arc_SRCS = arc_init.c arc_symbol.c
> +
>  libebl_backends_a_SOURCES = $(i386_SRCS) $(sh_SRCS) $(x86_64_SRCS) \
>                           $(ia64_SRCS) $(alpha_SRCS) $(arm_SRCS) \
>                           $(aarch64_SRCS) $(sparc_SRCS) $(ppc_SRCS) \
>                           $(ppc64_SRCS) $(s390_SRCS) \
>                           $(m68k_SRCS) $(bpf_SRCS) $(riscv_SRCS)
> $(csky_SRCS) \
> -                         $(loongarch_SRCS)
> +                         $(loongarch_SRCS) $(arc_SRCS)
>  
>  libebl_backends_pic_a_SOURCES =
>  am_libebl_backends_pic_a_OBJECTS =
> $(libebl_backends_a_SOURCES:.c=.os)

Looks good.

> diff --git a/backends/arc_init.c b/backends/arc_init.c
> new file mode 100644
> index 00000000..a013bc4e
> --- /dev/null
> +++ b/backends/arc_init.c
> @@ -0,0 +1,55 @@
> +/* Initialization of ARC specific backend library.
> +   Copyright (C) 2022 Synopsys Inc.
> +   This file is part of elfutils.
> +
> +   This file is free software; you can redistribute it and/or modify
> +   it under the terms of either
> +
> +     * the GNU Lesser General Public License as published by the
> Free
> +       Software Foundation; either version 3 of the License, or (at
> +       your option) any later version
> +
> +   or
> +
> +     * the GNU General Public License as published by the Free
> +       Software Foundation; either version 2 of the License, or (at
> +       your option) any later version
> +
> +   or both in parallel, as here.
> +
> +   elfutils is distributed in the hope that it will be useful, but
> +   WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   General Public License for more details.
> +
> +   You should have received copies of the GNU General Public License
> and
> +   the GNU Lesser General Public License along with this
> program.  If
> +   not, see <http://www.gnu.org/licenses/>.  */
> +
> +#ifdef HAVE_CONFIG_H
> +# include <config.h>
> +#endif
> +
> +#define BACKEND              arc_
> +#define RELOC_PREFIX R_ARC_
> +#include "libebl_CPU.h"
> +
> +/* This defines the common reloc hooks based on arc_reloc.def.  */
> +#include "common-reloc.c"
> +
> +Ebl *
> +arc_init (Elf *elf __attribute__ ((unused)),
> +       GElf_Half machine __attribute__ ((unused)),
> +       Ebl *eh)
> +{
> +  arc_init_reloc (eh);
> +  HOOK (eh, machine_flag_check);
> +  HOOK (eh, reloc_simple_type);
> +  HOOK (eh, section_type_name);
> +
> +  /* /bld/gcc-stage2/arc-snps-linux-gnu/libgcc/libgcc.map.in
> +     #define __LIBGCC_DWARF_FRAME_REGISTERS__.  */
> +  eh->frame_nregs = 146;
> +
> +  return eh;
> +}

OK. That is a lot of frame registers though, only ppc has more.

> diff --git a/backends/arc_reloc.def b/backends/arc_reloc.def
> new file mode 100644
> index 00000000..dfa30629
> --- /dev/null
> +++ b/backends/arc_reloc.def
> @@ -0,0 +1,87 @@
> +/* List the relocation types for ARC.  -*- C -*-
> +   This file is part of elfutils.
> +
> +   This file is free software; you can redistribute it and/or modify
> +   it under the terms of either
> +
> +     * the GNU Lesser General Public License as published by the
> Free
> +       Software Foundation; either version 3 of the License, or (at
> +       your option) any later version
> +
> +   or
> +
> +     * the GNU General Public License as published by the Free
> +       Software Foundation; either version 2 of the License, or (at
> +       your option) any later version
> +
> +   or both in parallel, as here.
> +
> +   elfutils is distributed in the hope that it will be useful, but
> +   WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   General Public License for more details.
> +
> +   You should have received copies of the GNU General Public License
> and
> +   the GNU Lesser General Public License along with this
> program.  If
> +   not, see <http://www.gnu.org/licenses/>.  */
> +
> +/*       NAME,               REL|EXEC|DYN    */
> +
> +RELOC_TYPE (NONE,            EXEC|DYN)
> +RELOC_TYPE (8,                       REL|EXEC|DYN)
> +RELOC_TYPE (16,                      REL|EXEC|DYN)
> +RELOC_TYPE (24,                      REL|EXEC|DYN)
> +RELOC_TYPE (32,                      REL|EXEC|DYN)
> +RELOC_TYPE (N8,                      REL|EXEC|DYN)
> +RELOC_TYPE (N16,             REL|EXEC|DYN)
> +RELOC_TYPE (N24,             REL|EXEC|DYN)
> +RELOC_TYPE (N32,             REL|EXEC|DYN)
> +RELOC_TYPE (SDA,             REL)
> +RELOC_TYPE (SECTOFF,         REL)
> +RELOC_TYPE (S21H_PCREL,              REL)
> +RELOC_TYPE (S21W_PCREL,              REL)
> +RELOC_TYPE (S25H_PCREL,              REL)
> +RELOC_TYPE (S25W_PCREL,              REL)
> +RELOC_TYPE (SDA32,           REL)
> +RELOC_TYPE (SDA_LDST,                REL)
> +RELOC_TYPE (SDA_LDST1,               REL)
> +RELOC_TYPE (SDA_LDST2,               REL)
> +RELOC_TYPE (SDA16_LD,                REL)
> +RELOC_TYPE (SDA16_LD1,               REL)
> +RELOC_TYPE (SDA16_LD2,               REL)
> +RELOC_TYPE (S13_PCREL,               REL)
> +RELOC_TYPE (W,                       REL)
> +RELOC_TYPE (32_ME,           REL)
> +RELOC_TYPE (N32_ME,          REL)
> +RELOC_TYPE (SECTOFF_ME,              REL)
> +RELOC_TYPE (SDA32_ME,                REL)
> +RELOC_TYPE (W_ME,            REL)
> +RELOC_TYPE (SDA_12,          REL)
> +RELOC_TYPE (SDA16_ST2,               REL)
> +RELOC_TYPE (32_PCREL,                REL)
> +RELOC_TYPE (PC32,            REL)
> +RELOC_TYPE (GOTPC32,         REL)
> +RELOC_TYPE (PLT32,           REL)
> +RELOC_TYPE (COPY,            EXEC|DYN)
> +RELOC_TYPE (GLOB_DAT,                EXEC|DYN)
> +RELOC_TYPE (JMP_SLOT,                EXEC|DYN)
> +RELOC_TYPE (RELATIVE,                EXEC|DYN)
> +RELOC_TYPE (GOTOFF,          REL)
> +RELOC_TYPE (GOTPC,           REL)
> +RELOC_TYPE (GOT32,           REL)
> +RELOC_TYPE (S21W_PCREL_PLT,  REL)
> +RELOC_TYPE (S25H_PCREL_PLT,  REL)
> +RELOC_TYPE (JLI_SECTOFF,     REL)
> +RELOC_TYPE (TLS_DTPMOD,              REL)
> +RELOC_TYPE (TLS_DTPOFF,              REL)
> +RELOC_TYPE (TLS_TPOFF,               REL)
> +RELOC_TYPE (TLS_GD_GOT,              REL)
> +RELOC_TYPE (TLS_GD_LD,               REL)
> +RELOC_TYPE (TLS_GD_CALL,     REL)
> +RELOC_TYPE (TLS_IE_GOT,              REL)
> +RELOC_TYPE (TLS_DTPOFF_S9,   REL)
> +RELOC_TYPE (TLS_LE_S9,               REL)
> +RELOC_TYPE (TLS_LE_32,               REL)
> +RELOC_TYPE (S25W_PCREL_PLT,  REL)
> +RELOC_TYPE (S21H_PCREL_PLT,  REL)
> +RELOC_TYPE (NPS_CMEM16,              REL)

Looks OK. Could you add a reference to the ARC ELF spec that describes
these?

> diff --git a/backends/arc_symbol.c b/backends/arc_symbol.c
> new file mode 100644
> index 00000000..be69814e
> --- /dev/null
> +++ b/backends/arc_symbol.c
> @@ -0,0 +1,81 @@
> +/* ARC specific symbolic name handling.
> +   This file is part of elfutils.
> +
> +   This file is free software; you can redistribute it and/or modify
> +   it under the terms of either
> +
> +     * the GNU Lesser General Public License as published by the
> Free
> +       Software Foundation; either version 3 of the License, or (at
> +       your option) any later version
> +
> +   or
> +
> +     * the GNU General Public License as published by the Free
> +       Software Foundation; either version 2 of the License, or (at
> +       your option) any later version
> +
> +   or both in parallel, as here.
> +
> +   elfutils is distributed in the hope that it will be useful, but
> +   WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   General Public License for more details.
> +
> +   You should have received copies of the GNU General Public License
> and
> +   the GNU Lesser General Public License along with this
> program.  If
> +   not, see <http://www.gnu.org/licenses/>.  */
> +
> +#ifdef HAVE_CONFIG_H
> +# include <config.h>
> +#endif
> +
> +#include <assert.h>
> +#include <elf.h>
> +#include <stddef.h>
> +#include <string.h>
> +
> +#define BACKEND arc_
> +#include "libebl_CPU.h"
> +
> +
> +/* Check whether machine flags are valid.  */
> +bool
> +arc_machine_flag_check (GElf_Word flags)
> +{
> +  return ((flags & ~EF_ARC_ALL_MSK) == 0);
> +}
> +
> +/* Check for the simple reloc types.  */
> +Elf_Type
> +arc_reloc_simple_type (Ebl *ebl __attribute__ ((unused)), int type,
> +                    int *addsub __attribute ((unused)))
> +{
> +  switch (type)
> +    {
> +    case R_ARC_32:
> +      return ELF_T_WORD;
> +    case R_ARC_16:
> +      return ELF_T_HALF;
> +    case R_ARC_8:
> +      return ELF_T_BYTE;
> +    default:
> +      return ELF_T_NUM;
> +    }
> +}
> +
> +/* Return symbolic representation of section type.  */
> +const char *
> +arc_section_type_name (int type,
> +                    char *buf __attribute__ ((unused)),
> +                    size_t len __attribute__ ((unused)))
> +{
> +  switch (type)
> +    {
> +    case SHT_ARC_ATTRIBUTES:
> +      return "ARC_ATTRIBUTES";
> +    default:
> +      break;
> +    }
> +
> +  return NULL;
> +}

OK.

> diff --git a/libebl/ChangeLog b/libebl/ChangeLog
> index 5f9ea552..3fb5801f 100644
> --- a/libebl/ChangeLog
> +++ b/libebl/ChangeLog
> @@ -1,3 +1,8 @@
> +2022-12-20  Shahab Vahedi  <sha...@synopsys.com>
> +
> +     * eblopenbackend.c (arc_init): New function declaration.
> +     (machines): Add entry for arc.
> +
>  2022-12-02  Hengqi Chen  <hengqi.c...@gmail.com>
>  
>       * eblopenbackend.c (machines): Add entries for LoongArch.
> diff --git a/libebl/eblopenbackend.c b/libebl/eblopenbackend.c
> index b87aef19..084a1544 100644
> --- a/libebl/eblopenbackend.c
> +++ b/libebl/eblopenbackend.c
> @@ -56,6 +56,7 @@ Ebl *bpf_init (Elf *, GElf_Half, Ebl *);
>  Ebl *riscv_init (Elf *, GElf_Half, Ebl *);
>  Ebl *csky_init (Elf *, GElf_Half, Ebl *);
>  Ebl *loongarch_init (Elf *, GElf_Half, Ebl *);
> +Ebl *arc_init (Elf *, GElf_Half, Ebl *);
>  
>  /* This table should contain the complete list of architectures as
> far
>     as the ELF specification is concerned.  */
> @@ -152,6 +153,7 @@ static const struct
>    { riscv_init, "elf_riscv", "riscv", 5, EM_RISCV, ELFCLASS32,
> ELFDATA2LSB },
>    { csky_init, "elf_csky", "csky", 4, EM_CSKY, ELFCLASS32,
> ELFDATA2LSB },
>    { loongarch_init, "elf_loongarch", "loongarch", 9, EM_LOONGARCH,
> ELFCLASS64, ELFDATA2LSB },
> +  { arc_init, "elf_arc", "arc", 3, EM_ARCV2, ELFCLASS32, ELFDATA2LSB
> },
>  };
>  #define nmachines (sizeof (machines) / sizeof (machines[0]))

Good. Note that this only matches for ARCV2, but I assume that is
intended.

> diff --git a/tests/ChangeLog b/tests/ChangeLog
> index b656029f..b5136869 100644
> --- a/tests/ChangeLog
> +++ b/tests/ChangeLog
> @@ -1,3 +1,9 @@
> +2022-12-20  Shahab Vahedi  <sha...@synopsys.com>
> +
> +     * hello_arc_hs4.ko.bz2: New testfile.
> +     * run-strip-reloc.sh: Add ARC HS4 test.
> +     * Makefile.am (EXTRA_DIST): Add hello_arc_hs4.ko.bz2.
> +
>  2022-11-01  Aaron Merey  <ame...@redhat.com>
>  
>       * run-debuginfod-section.sh (RPM_BUILDID): Use buildid from
> non-zstd
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 356b3fbf..c35a7c33 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -285,6 +285,7 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh
> run-ar.sh \
>            run-strip-reloc.sh hello_i386.ko.bz2 hello_x86_64.ko.bz2 \
>            hello_ppc64.ko.bz2 hello_s390.ko.bz2 hello_aarch64.ko.bz2
> \
>            hello_m68k.ko.bz2 hello_riscv64.ko.bz2 hello_csky.ko.bz2 \
> +          hello_arc_hs4.ko.bz2 \
>            run-unstrip-test.sh run-unstrip-test2.sh \
>            testfile-info-link.bz2 testfile-info-link.debuginfo.bz2 \
>            testfile-info-link.stripped.bz2 run-unstrip-test3.sh \
> diff --git a/tests/hello_arc_hs4.ko.bz2 b/tests/hello_arc_hs4.ko.bz2
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..56ccb3c494e84450c7aeac5f57f
> 94aef8336f8e0
> GIT binary patch
> literal 15004
> [...]
> literal 0
> HcmV?d00001
>
> diff --git a/tests/run-strip-reloc.sh b/tests/run-strip-reloc.sh
> index b7ec1420..033ed278 100755
> --- a/tests/run-strip-reloc.sh
> +++ b/tests/run-strip-reloc.sh
> @@ -18,7 +18,8 @@
>  . $srcdir/test-subr.sh
>  
>  testfiles hello_i386.ko hello_x86_64.ko hello_ppc64.ko hello_s390.ko
> \
> -     hello_aarch64.ko hello_m68k.ko hello_riscv64.ko hello_csky.ko
> +     hello_aarch64.ko hello_m68k.ko hello_riscv64.ko hello_csky.ko \
> +     hello_arc_hs4.ko
>  
>  tempfiles readelf.out readelf.out1 readelf.out2
>  tempfiles out.stripped1 out.debug1 out.stripped2 out.debug2
> @@ -120,6 +121,7 @@ runtest hello_aarch64.ko 1
>  runtest hello_m68k.ko 1
>  runtest hello_riscv64.ko 1
>  runtest hello_csky.ko 1
> +runtest hello_arc_hs4.ko 1
>  
>  # self test, shouldn't impact non-ET_REL files at all.
>  runtest ${abs_top_builddir}/src/strip 0

Very nice, that makes it possible to test some of this on non-ARC
setups. (Seems to run fine here)

Let me know if you want this to go in as is or if you want to sent a v4
with the tweaks suggested above.

Cheers.

Mark
> 

Reply via email to