From: Mark Wielaard <mark@klomp.org>
To: Shahab Vahedi <Shahab.Vahedi@synopsys.com>,
elfutils-devel@sourceware.org
Cc: Shahab Vahedi <shahab.vahedi@gmail.com>,
Claudiu Zissulescu <Claudiu.Zissulescu@synopsys.com>,
Francois Bedard <Francois.Bedard@synopsys.com>
Subject: Re: [PATCH v3] Add support for Synopsys ARCv2 processors
Date: Tue, 20 Dec 2022 18:33:34 +0100 [thread overview]
Message-ID: <07da11b1353a5c98cf15ebede7f3b19c9afba6c0.camel@klomp.org> (raw)
In-Reply-To: <20221220115627.10483-1-shahab@synopsys.com>
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 <shahab@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 <shahab@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.chen@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 <shahab@synopsys.com>
> +
> + * eblopenbackend.c (arc_init): New function declaration.
> + (machines): Add entry for arc.
> +
> 2022-12-02 Hengqi Chen <hengqi.chen@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 <shahab@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 <amerey@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
>
next prev parent reply other threads:[~2022-12-20 17:33 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-20 11:56 Shahab Vahedi
2022-12-20 17:33 ` Mark Wielaard [this message]
2022-12-21 11:27 ` [PATCH v4] " Shahab Vahedi
2022-12-21 23:30 ` Mark Wielaard
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=07da11b1353a5c98cf15ebede7f3b19c9afba6c0.camel@klomp.org \
--to=mark@klomp.org \
--cc=Claudiu.Zissulescu@synopsys.com \
--cc=Francois.Bedard@synopsys.com \
--cc=Shahab.Vahedi@synopsys.com \
--cc=elfutils-devel@sourceware.org \
--cc=shahab.vahedi@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).