From: WANG Xuerui <i.swmail@xen0n.name>
To: liuzhensong <liuzhensong@loongson.cn>, binutils@sourceware.org
Cc: i.swmail@xen0n.name, xry111@xry111.site, maskray@google.com,
mengqinggang@loongson.cn
Subject: Re: [PATCH v2] LoongArch: Set e_flags to 0x40 of ELF while using new relocations.
Date: Sun, 25 Sep 2022 16:28:10 +0800 [thread overview]
Message-ID: <83e9a5ac-c509-066b-07f6-cb3ec4583e48@xen0n.name> (raw)
In-Reply-To: <20220921011756.451005-1-liuzhensong@loongson.cn>
Hi,
Overall this is going in the right direction (finally!) and implementing
the correct behavior, but a few small nits remain.
Also, @xry111 WDYT?
On 2022/9/21 09:17, liuzhensong wrote:
> As the docs[1] describes, update the e_flags of ELF.
> 1. 32 or 64 bit is represented by EI_CLASS.
> 2. The bits of e_flags
> Base ABI [2:0]:
> soft float -- 0x1
> single float -- 0x2
> double float -- 0x3
>
> ABI version [7:6]:
> obj-v0 -- 0x0
> obj-v1 -- 0x1
> 3. Discussions [2].
>
> docs:
> [1] https://github.com/loongson/LoongArch-Documentation/blob/
> main/docs/LoongArch-ELF-ABI-EN.adoc#e_flags-identifies-abi-type-and-version
> [2] https://github.com/loongson/LoongArch-Documentation/pull/61
Rewritten into (somewhat more) proper English for you, also we have
another significant discussion thread that you hadn't mentioned:
```
Subject: LoongArch: Update ELF e_flags handling according to specification.
Update handling of e_flags according to the documentation update [1]
(discussions [2][3]).
Object file bitness is now represented in the EI_CLASS byte. The e_flags
field is now interpreted as follows:
e_flags[2:0]: Base ABI modifier
- 0x1: soft-float
- 0x2: single-precision hard-float
- 0x3: double-precision hard-float
e_flags[7:6]: ELF object ABI version
- 0x0: v0
- 0x1: v1
[1]: https://github.com/loongson/LoongArch-Documentation/blob/main/docs/LoongArch-ELF-ABI-EN.adoc#e_flags-identifies-abi-type-and-version
[2]: https://github.com/loongson/LoongArch-Documentation/pull/61
[3]: https://github.com/loongson/LoongArch-Documentation/pull/47
```
(Tip: next time when writing commit messages (or in fact anything) in
English, please bear in mind that Chinese is a "topic-prominent"
language [99] unlike English, meaning the idiomatic sentence pattern is
usually different. In general people can still grasp the meaning but
will probably have a headache in trying to do so...)
[99]: https://en.wikipedia.org/wiki/Topic-prominent_language
> ---
> bfd/elfnn-loongarch.c | 21 ++++++++++++++++---
> binutils/readelf.c | 10 ++++-----
> gas/config/tc-loongarch.c | 20 +++++++++---------
> include/elf/loongarch.h | 44 +++++++++++++++++++--------------------
> 4 files changed, 54 insertions(+), 41 deletions(-)
>
> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> index 33c85e5207c..5b4b83eb07a 100644
> --- a/bfd/elfnn-loongarch.c
> +++ b/bfd/elfnn-loongarch.c
> @@ -416,8 +416,22 @@ elfNN_loongarch_merge_private_bfd_data (bfd *ibfd, struct bfd_link_info *info)
> elf_elfheader (obfd)->e_flags = in_flags;
> return true;
> }
> + else if (out_flags != in_flags)
> + {
> + if ((EF_LOONGARCH_IS_OBJ_V0 (out_flags)
> + && EF_LOONGARCH_IS_OBJ_V1 (in_flags))
> + || (EF_LOONGARCH_IS_OBJ_V0 (in_flags)
> + && EF_LOONGARCH_IS_OBJ_V1 (out_flags)))
> + {
> + elf_elfheader (obfd)->e_flags |= EF_LOONGARCH_ABI_OBJ_V1;
> + out_flags = elf_elfheader (obfd)->e_flags;
> + in_flags = out_flags;
> + }
> + }
>
> /* Disallow linking different ABIs. */
> + /* Only check relocation version.
> + The obj_v0 is compatible with obj_v1 in this "release cycle". */
We know this cycle is 2.40 so directly mention that?
> if (EF_LOONGARCH_ABI(out_flags ^ in_flags) & EF_LOONGARCH_ABI_MASK)
> {
> _bfd_error_handler (_("%pB: can't link different ABI object."), ibfd);
> @@ -1579,15 +1593,16 @@ loongarch_elf_size_dynamic_sections (bfd *output_bfd,
> if (bfd_link_executable (info) && !info->nointerp)
> {
> const char *interpreter;
> - flagword flags = elf_elfheader (output_bfd)->e_flags;
> s = bfd_get_linker_section (dynobj, ".interp");
> BFD_ASSERT (s != NULL);
> - if (EF_LOONGARCH_IS_ILP32 (flags))
> +
> + if (elf_elfheader (output_bfd)->e_ident[EI_CLASS] == ELFCLASS32)
> interpreter = "/lib32/ld.so.1";
> - else if (EF_LOONGARCH_IS_LP64 (flags))
> + else if (elf_elfheader (output_bfd)->e_ident[EI_CLASS] == ELFCLASS64)
> interpreter = "/lib64/ld.so.1";
> else
> interpreter = "/lib/ld.so.1";
> +
Wait what, are we still carrying the old-world defaults for the
interpreter path here? I know it's harmless because in fact it gets
overridden by gcc and glibc, but I'd suggest fixing it in a separate
patch later for consistency.
> s->contents = (unsigned char *) interpreter;
> s->size = strlen (interpreter) + 1;
> }
> diff --git a/binutils/readelf.c b/binutils/readelf.c
> index cafba9a4f56..6926cf16140 100644
> --- a/binutils/readelf.c
> +++ b/binutils/readelf.c
> @@ -4340,11 +4340,6 @@ get_machine_flags (Filedata * filedata, unsigned e_flags, unsigned e_machine)
> }
> break;
> case EM_LOONGARCH:
> - if (EF_LOONGARCH_IS_LP64 (e_flags))
> - strcat (buf, ", LP64");
> - else if (EF_LOONGARCH_IS_ILP32 (e_flags))
> - strcat (buf, ", ILP32");
> -
> if (EF_LOONGARCH_IS_SOFT_FLOAT (e_flags))
> strcat (buf, ", SOFT-FLOAT");
> else if (EF_LOONGARCH_IS_SINGLE_FLOAT (e_flags))
> @@ -4352,6 +4347,11 @@ get_machine_flags (Filedata * filedata, unsigned e_flags, unsigned e_machine)
> else if (EF_LOONGARCH_IS_DOUBLE_FLOAT (e_flags))
> strcat (buf, ", DOUBLE-FLOAT");
>
> + if (EF_LOONGARCH_IS_OBJ_V0 (e_flags))
> + strcat (buf, ", OBJ-v0");
> + else if (EF_LOONGARCH_IS_OBJ_V1 (e_flags))
> + strcat (buf, ", OBJ-v1");
> +
> break;
> }
> }
> diff --git a/gas/config/tc-loongarch.c b/gas/config/tc-loongarch.c
> index 5bb8d317c5a..3e9204a5fa4 100644
> --- a/gas/config/tc-loongarch.c
> +++ b/gas/config/tc-loongarch.c
> @@ -143,13 +143,13 @@ md_parse_option (int c, const char *arg)
> char ilp32[256] = "";
> unsigned char *suf = (unsigned char *)arg;
>
> - lp64['s'] = lp64['S'] = EF_LOONGARCH_ABI_LP64_SOFT_FLOAT;
> - lp64['f'] = lp64['F'] = EF_LOONGARCH_ABI_LP64_SINGLE_FLOAT;
> - lp64['d'] = lp64['D'] = EF_LOONGARCH_ABI_LP64_DOUBLE_FLOAT;
> + lp64['s'] = lp64['S'] = EF_LOONGARCH_ABI_SOFT_FLOAT;
> + lp64['f'] = lp64['F'] = EF_LOONGARCH_ABI_SINGLE_FLOAT;
> + lp64['d'] = lp64['D'] = EF_LOONGARCH_ABI_DOUBLE_FLOAT;
>
> - ilp32['s'] = ilp32['S'] = EF_LOONGARCH_ABI_ILP32_SOFT_FLOAT;
> - ilp32['f'] = ilp32['F'] = EF_LOONGARCH_ABI_ILP32_SINGLE_FLOAT;
> - ilp32['d'] = ilp32['D'] = EF_LOONGARCH_ABI_ILP32_DOUBLE_FLOAT;
> + ilp32['s'] = ilp32['S'] = EF_LOONGARCH_ABI_SOFT_FLOAT;
> + ilp32['f'] = ilp32['F'] = EF_LOONGARCH_ABI_SINGLE_FLOAT;
> + ilp32['d'] = ilp32['D'] = EF_LOONGARCH_ABI_DOUBLE_FLOAT;
>
> switch (c)
> {
> @@ -216,24 +216,24 @@ void
> loongarch_after_parse_args ()
> {
> /* Set default ABI/ISA LP64D. */
> - if (!EF_LOONGARCH_IS_LP64(LARCH_opts.ase_abi)
> - && !EF_LOONGARCH_IS_ILP32(LARCH_opts.ase_abi))
> + if (!LARCH_opts.ase_ilp32)
> {
> if (strcmp (default_arch, "loongarch64") == 0)
> {
> - LARCH_opts.ase_abi = EF_LOONGARCH_ABI_LP64_DOUBLE_FLOAT;
> + LARCH_opts.ase_abi = EF_LOONGARCH_ABI_DOUBLE_FLOAT;
> LARCH_opts.ase_ilp32 = 1;
> LARCH_opts.ase_lp64 = 1;
> }
> else if (strcmp (default_arch, "loongarch32") == 0)
> {
> - LARCH_opts.ase_abi = EF_LOONGARCH_ABI_ILP32_DOUBLE_FLOAT;
> + LARCH_opts.ase_abi = EF_LOONGARCH_ABI_DOUBLE_FLOAT;
> LARCH_opts.ase_ilp32 = 1;
> }
> else
> as_bad ("unknown default architecture `%s'", default_arch);
> }
>
> + LARCH_opts.ase_abi |= EF_LOONGARCH_ABI_OBJ_V1;
> /* Set default ISA double-float. */
> if (!LARCH_opts.ase_nf
> && !LARCH_opts.ase_sf
> diff --git a/include/elf/loongarch.h b/include/elf/loongarch.h
> index 74757b82ca8..cceaa8cccc4 100644
> --- a/include/elf/loongarch.h
> +++ b/include/elf/loongarch.h
> @@ -232,36 +232,34 @@ RELOC_NUMBER (R_LARCH_RELAX, 100)
> END_RELOC_NUMBERS (R_LARCH_count)
>
> /* Processor specific flags for the ELF header e_flags field. */
> -/*The flag lp64s/lp64f/lp64d/ilp32s/ilp32f/ilp32d 3bits. */
> -#define EF_LOONGARCH_ABI_LP64_SOFT_FLOAT 0x1
> -#define EF_LOONGARCH_ABI_LP64_SINGLE_FLOAT 0x2
> -#define EF_LOONGARCH_ABI_LP64_DOUBLE_FLOAT 0x3
> +/* ase ABI modifier, 3bits. */
Oh please no more MIPS "ASE"s. It's "Base ABI modifier" in the docs...
> +#define EF_LOONGARCH_ABI_SOFT_FLOAT 0x1
> +#define EF_LOONGARCH_ABI_SINGLE_FLOAT 0x2
> +#define EF_LOONGARCH_ABI_DOUBLE_FLOAT 0x3
> +#define EF_LOONGARCH_ABI_MODIFIER_MASK 0x7
>
> -#define EF_LOONGARCH_ABI_ILP32_SOFT_FLOAT 0x5
> -#define EF_LOONGARCH_ABI_ILP32_SINGLE_FLOAT 0x6
> -#define EF_LOONGARCH_ABI_ILP32_DOUBLE_FLOAT 0x7
> +#define EF_LOONGARCH_ABI_OBJ_V1 0x40
> +#define EF_LOONGARCH_ABI_OBJ_MASK 0xC0
I'd bikeshed a little and suggest "EF_LOONGARCH_OBJABI_foo" because it's
actually "ELF object file ABI v1" and not "ELF ABI object v1".
>
> -#define EF_LOONGARCH_ABI_MASK 0x7
> -#define EF_LOONGARCH_ABI_ILP32_MASK 0x4
> -#define EF_LOONGARCH_ABI_FLOAT_MASK 0x3
> -#define EF_LOONGARCH_ABI_SOFT_FLOAT_MASK 0x1
> -#define EF_LOONGARCH_ABI_SINGLE_FLOAT_MASK 0x2
> -#define EF_LOONGARCH_ABI_DOUBLE_FLOAT_MASK 0x3
> +#define EF_LOONGARCH_ABI_MASK \
> + (EF_LOONGARCH_ABI_OBJ_MASK | EF_LOONGARCH_ABI_MODIFIER_MASK)
>
> -#define EF_LOONGARCH_ABI(abi) (EF_LOONGARCH_ABI_MASK & (abi))
> +#define EF_LOONGARCH_ABI_MODIFIER(abi) \
> + (EF_LOONGARCH_ABI_MODIFIER_MASK & (abi))
> +#define EF_LOONGARCH_ABI_OBJ(abi) \
> + (EF_LOONGARCH_ABI_OBJ_MASK & (abi))
>
> -#define EF_LOONGARCH_IS_LP64(abi) \
> - (EF_LOONGARCH_ABI(abi) && (!(EF_LOONGARCH_ABI(abi) & EF_LOONGARCH_ABI_ILP32_MASK)))
> -#define EF_LOONGARCH_IS_ILP32(abi) \
> - (EF_LOONGARCH_ABI(abi) && (EF_LOONGARCH_ABI(abi) & EF_LOONGARCH_ABI_ILP32_MASK))
> +#define EF_LOONGARCH_ABI(abi) ((abi) & EF_LOONGARCH_ABI_MASK)
>
> #define EF_LOONGARCH_IS_SOFT_FLOAT(abi) \
> - (!((EF_LOONGARCH_ABI(abi) & EF_LOONGARCH_ABI_FLOAT_MASK) ^ EF_LOONGARCH_ABI_SOFT_FLOAT_MASK))
> -
> + (EF_LOONGARCH_ABI_MODIFIER (abi) == EF_LOONGARCH_ABI_SOFT_FLOAT)
> #define EF_LOONGARCH_IS_SINGLE_FLOAT(abi) \
> - (!((EF_LOONGARCH_ABI(abi) & EF_LOONGARCH_ABI_FLOAT_MASK) ^ EF_LOONGARCH_ABI_SINGLE_FLOAT_MASK))
> -
> + (EF_LOONGARCH_ABI_MODIFIER (abi) == EF_LOONGARCH_ABI_SINGLE_FLOAT)
> #define EF_LOONGARCH_IS_DOUBLE_FLOAT(abi) \
> - (!((EF_LOONGARCH_ABI(abi) & EF_LOONGARCH_ABI_FLOAT_MASK) ^ EF_LOONGARCH_ABI_DOUBLE_FLOAT_MASK))
> + (EF_LOONGARCH_ABI_MODIFIER (abi) == EF_LOONGARCH_ABI_DOUBLE_FLOAT)
> +
> +#define EF_LOONGARCH_IS_OBJ_V0(abi) (!EF_LOONGARCH_ABI_OBJ(abi))
> +#define EF_LOONGARCH_IS_OBJ_V1(abi) \
> + (EF_LOONGARCH_ABI_OBJ (abi) == EF_LOONGARCH_ABI_OBJ_V1)
>
> #endif /* _ELF_LOONGARCH_H */
next prev parent reply other threads:[~2022-09-25 8:28 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-21 1:17 liuzhensong
2022-09-25 8:28 ` WANG Xuerui [this message]
2022-09-25 9:22 ` Xi Ruoyao
2022-09-27 16:49 ` Fangrui Song
2022-09-27 2:11 ` liuzhensong
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=83e9a5ac-c509-066b-07f6-cb3ec4583e48@xen0n.name \
--to=i.swmail@xen0n.name \
--cc=binutils@sourceware.org \
--cc=liuzhensong@loongson.cn \
--cc=maskray@google.com \
--cc=mengqinggang@loongson.cn \
--cc=xry111@xry111.site \
/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).