[PATCH] ld: Add elf{32,64}[lb]riscv_fbsd emulations and riscv{32,64}{,be}-*-freebsd* triples

[Nelson Chu]

Hi Maskray

Thanks for adding this support, I have some thoughts and suggestions.

On Sun, Jan 31, 2021 at 3:07 PM Fangrui Song <maskray@google.com> wrote:
>
> Relax an ABI compatibility check so that ELFOSABI_FREEBSD output can
> subsume ELFOSABI_NONE/ELFOSABI_GNU input object files.
>
>     PR 27296
> bfd/
>     * config.bfd: Add riscvbe-*-freebsd*, riscv32be-*-freebsd*,
>     riscv-*-freebsd*, riscv32*-*-freebsd*, riscv64be-*-freebsd*,
>     riscv64*-*-freebsd*.
>     * configure.ac: Add riscv_elf32_fbsd_vec, riscv_elf32_fbsd_vec,
>     riscv_elf32_fbsd_vec, riscv_elf32_fbsd_vec.
>     * elfnn-riscv.c (TARGET_LITTLE_SYM, TARGET_LITTLE_NAME,
>     TARGET_BIG_SYM, TARGET_BIG_NAME, ELF_OSABI, elf32_bed, elf64_bed):
>     Define for freebsd.
>     * targets.c (riscv_elf32_fbsd_vec, riscv_elf32_fbsd_vec,
>     riscv_elf32_fbsd_vec, riscv_elf32_fbsd_vec): Declare.
>     (_bfd_target_vector): Add them.
>     * configure: Regenerate.
> ld/
>     * Makefile.am (ALL_EMULATION_SOURCES): Add eelf32lriscv_fbsd.c and
>     eelf32briscv_fbsd.c.
>     (ALL_64_EMULATION_SOURCES): Add eelf64lriscv_fbsd.c and
>     eelf64briscv_fbsd.c.
>     Include $(DEPDIR)/eelf*lriscv_fbsd* files.
>     * configure.tgt: Add riscvbe-*-freebsd*, riscv32be-*-freebsd*,
>     riscv-*-freebsd*, riscv32*-*-freebsd*, riscv64be-*-freebsd*,
>     riscv64*-*-freebsd*.
>     * emulparams/elf32lriscv_bsd.sh: New file.
>     * emulparams/elf32briscv_bsd.sh: New file.
>     * emulparams/elf64lriscv_bsd.sh: New file.
>     * emulparams/elf64briscv_bsd.sh: New file.
>     * Makefile.in: Regenerate.
>     * po/BLD-POTFILES.in: Regenerate.

I think we also need to add the FreeBSD support for RISC-V assembler.
After scanning other targets' code quickly, we can set the em=freebsd
in the gas/configure.tgt for target riscv*-*-freebsd*, then the
TE_FreeBSD will be defined, and we should update the
riscv_target_format (gas/config/tc-riscv.c) to output the right format
name.  You can refer to what MIPS did in their mips_target_format
(gas/config/tc-mips.c).

> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> index b2ec6a29fbf..98918b29f4e 100644
> --- a/bfd/elfnn-riscv.c
> +++ b/bfd/elfnn-riscv.c
> @@ -3786,15 +3786,6 @@ _bfd_riscv_elf_merge_private_bfd_data (bfd *ibfd, struct bfd_link_info *info)
>    if (!is_riscv_elf (ibfd) || !is_riscv_elf (obfd))
>      return TRUE;
>
> -  if (strcmp (bfd_get_target (ibfd), bfd_get_target (obfd)) != 0)
> -    {
> -      (*_bfd_error_handler)
> -       (_("%pB: ABI is incompatible with that of the selected emulation:\n"
> -          "  target emulation `%s' does not match `%s'"),
> -        ibfd, bfd_get_target (ibfd), bfd_get_target (obfd));
> -      return FALSE;
> -    }
> -
>    if (!_bfd_elf_merge_object_attributes (ibfd, info))
>      return FALSE;
>
> @@ -3811,6 +3802,18 @@ _bfd_riscv_elf_merge_private_bfd_data (bfd *ibfd, struct bfd_link_info *info)
>        return TRUE;
>      }
>
> +  if (elf_elfheader (ibfd)->e_ident[EI_CLASS]
> +         != elf_elfheader (obfd)->e_ident[EI_CLASS]
> +      || elf_elfheader (ibfd)->e_ident[EI_DATA]
> +            != elf_elfheader (obfd)->e_ident[EI_DATA])

Indent needs to be fixed, it's minor

> +    {
> +      (*_bfd_error_handler)
> +       (_("%pB: ABI is incompatible with that of the selected emulation:\n"
> +          "  target emulation `%s' does not match `%s'"),
> +        ibfd, bfd_get_target (ibfd), bfd_get_target (obfd));
> +      return FALSE;
> +    }
> +

This change broke the GNU linker, I get lots of errors as follows when
just compiling a simple HelloWorld,

/scratch/nelsonc/build-upstream/rv32i-elf/build-install/lib/gcc/riscv32-unknown-elf/10.2.0/../../../../riscv32-unknown-elf/bin/ld:
/scratch/nelsonc/build-upstream/rv32i-elf/build-install/lib/gcc/riscv32-unknown-elf/10.2.0/crtbegin.o:
ABI is incompatible with that of the selected emulation:
  target emulation `elf32-littleriscv' does not match `elf32-littleriscv'
/scratch/nelsonc/build-upstream/rv32i-elf/build-install/lib/gcc/riscv32-unknown-elf/10.2.0/../../../../riscv32-unknown-elf/bin/ld:
failed to merge target specific data of file
/scratch/nelsonc/build-upstream/rv32i-elf/build-install/lib/gcc/riscv32-unknown-elf/10.2.0/crtbegin.o
/scratch/nelsonc/build-upstream/rv32i-elf/build-install/lib/gcc/riscv32-unknown-elf/10.2.0/../../../../riscv32-unknown-elf/bin/ld:
/tmp/ccY5Ib0n.o: ABI is incompatible with that of the selected
emulation:
  target emulation `elf32-littleriscv' does not match `elf32-littleriscv'

The problem is that we just handle/copy the e_flags here when
elf_flags_init (obfd) is FALSE, the correct elf header CLASS/DATA are
set until _bfd_elf_init_file_header in bfd/elf.c, not here.
Therefore, you are comparing an uninitialized elf header CLASS/DATA
from the output bfd with the input bfd which is aleady set.  I suppose
you should remain the original check in the
_bfd_riscv_elf_merge_private_bfd_data.  Adding the related FreeBSD
support in the assembler should let linker can get the correct
elf32-littleriscv-freebsd target name by bfd_get_target, rather than
get the elf32-littleriscv for freebsd.  Besides, we also need to
update the riscv_elf_object_p, to recognize the freebsd targets.

Otherwise, this patch should be fine and won't affect our current
elf/linux toolchains.  I have tested the rv32i-newlib and rv64gc-glibc
toolchains quickly, the gcc/binutils regressions look fine so far.
For safety, I hope we can pass the whole riscv-gnu-toolchain
regressions for the later fixed patches.

Thanks
Nelson