Re: [PATCH] RISC-V: Remove R_RISCV_GNU_VTINHERIT/R_RISCV_GNU_VTENTRY

[Nelson Chu <nelson.chu@sifive.com>]

Not sure about the history, but if we never use these two relocations,
then removing them makes sense to me.  Btw, just curious that, will we
use numbers 41 and 42 first when adding new relocations?

On Mon, Aug 8, 2022 at 2:55 PM Fangrui Song <maskray@google.com> wrote:
>
> There is no assembler syntax for the two relocation types and therefore
> they appear to be never used. Remove related ld code.
>
> Link: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/323
> ---
>  bfd/elfnn-riscv.c   | 32 +-------------------------------
>  bfd/elfxx-riscv.c   | 32 +++-----------------------------
>  include/elf/riscv.h |  2 --
>  3 files changed, 4 insertions(+), 62 deletions(-)
>
> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> index 8f9f0d8a86a..86cb207b573 100644
> --- a/bfd/elfnn-riscv.c
> +++ b/bfd/elfnn-riscv.c
> @@ -932,16 +932,6 @@ riscv_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
>
>           break;
>
> -       case R_RISCV_GNU_VTINHERIT:
> -         if (!bfd_elf_gc_record_vtinherit (abfd, sec, h, rel->r_offset))
> -           return false;
> -         break;
> -
> -       case R_RISCV_GNU_VTENTRY:
> -         if (!bfd_elf_gc_record_vtentry (abfd, sec, h, rel->r_addend))
> -           return false;
> -         break;
> -
>         default:
>           break;
>         }
> @@ -950,24 +940,6 @@ riscv_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
>    return true;
>  }
>
> -static asection *
> -riscv_elf_gc_mark_hook (asection *sec,
> -                       struct bfd_link_info *info,
> -                       Elf_Internal_Rela *rel,
> -                       struct elf_link_hash_entry *h,
> -                       Elf_Internal_Sym *sym)
> -{
> -  if (h != NULL)
> -    switch (ELFNN_R_TYPE (rel->r_info))
> -      {
> -      case R_RISCV_GNU_VTINHERIT:
> -      case R_RISCV_GNU_VTENTRY:
> -       return NULL;
> -      }
> -
> -  return _bfd_elf_gc_mark_hook (sec, info, rel, h, sym);
> -}
> -
>  /* Adjust a symbol defined by a dynamic object and referenced by a
>     regular object.  The current definition is in some section of the
>     dynamic object, but we're not including those sections.  We have to
> @@ -2027,8 +1999,7 @@ riscv_elf_relocate_section (bfd *output_bfd,
>        char *msg_buf = NULL;
>        bool resolved_to_zero;
>
> -      if (howto == NULL
> -         || r_type == R_RISCV_GNU_VTINHERIT || r_type == R_RISCV_GNU_VTENTRY)
> +      if (howto == NULL)
>         continue;
>
>        /* This is a final link.  */
> @@ -5260,7 +5231,6 @@ riscv_elf_merge_symbol_attribute (struct elf_link_hash_entry *h,
>  #define elf_backend_relocate_section           riscv_elf_relocate_section
>  #define elf_backend_finish_dynamic_symbol      riscv_elf_finish_dynamic_symbol
>  #define elf_backend_finish_dynamic_sections    riscv_elf_finish_dynamic_sections
> -#define elf_backend_gc_mark_hook               riscv_elf_gc_mark_hook
>  #define elf_backend_plt_sym_val                        riscv_elf_plt_sym_val
>  #define elf_backend_grok_prstatus              riscv_elf_grok_prstatus
>  #define elf_backend_grok_psinfo                        riscv_elf_grok_psinfo
> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> index 0b2021f5cc7..c9636bcbdc8 100644
> --- a/bfd/elfxx-riscv.c
> +++ b/bfd/elfxx-riscv.c
> @@ -599,35 +599,9 @@ static reloc_howto_type howto_table[] =
>          MINUS_ONE,                     /* dst_mask */
>          false),                        /* pcrel_offset */
>
> -  /* GNU extension to record C++ vtable hierarchy */
> -  HOWTO (R_RISCV_GNU_VTINHERIT,                /* type */
> -        0,                             /* rightshift */
> -        8,                             /* size */
> -        0,                             /* bitsize */
> -        false,                         /* pc_relative */
> -        0,                             /* bitpos */
> -        complain_overflow_dont,        /* complain_on_overflow */
> -        NULL,                          /* special_function */
> -        "R_RISCV_GNU_VTINHERIT",       /* name */
> -        false,                         /* partial_inplace */
> -        0,                             /* src_mask */
> -        0,                             /* dst_mask */
> -        false),                        /* pcrel_offset */
> -
> -  /* GNU extension to record C++ vtable member usage */
> -  HOWTO (R_RISCV_GNU_VTENTRY,          /* type */
> -        0,                             /* rightshift */
> -        8,                             /* size */
> -        0,                             /* bitsize */
> -        false,                         /* pc_relative */
> -        0,                             /* bitpos */
> -        complain_overflow_dont,        /* complain_on_overflow */
> -        _bfd_elf_rel_vtable_reloc_fn,  /* special_function */
> -        "R_RISCV_GNU_VTENTRY",         /* name */
> -        false,                         /* partial_inplace */
> -        0,                             /* src_mask */
> -        0,                             /* dst_mask */
> -        false),                        /* pcrel_offset */
> +  /* 41 and 42 are reserved.  */
> +  EMPTY_HOWTO (0),
> +  EMPTY_HOWTO (0),
>
>    /* Indicates an alignment statement.  The addend field encodes how many
>       bytes of NOPs follow the statement.  The desired alignment is the
> diff --git a/include/elf/riscv.h b/include/elf/riscv.h
> index d0acf6886d8..9b3ea376ff3 100644
> --- a/include/elf/riscv.h
> +++ b/include/elf/riscv.h
> @@ -71,8 +71,6 @@ START_RELOC_NUMBERS (elf_riscv_reloc_type)
>    RELOC_NUMBER (R_RISCV_SUB16, 38)
>    RELOC_NUMBER (R_RISCV_SUB32, 39)
>    RELOC_NUMBER (R_RISCV_SUB64, 40)
> -  RELOC_NUMBER (R_RISCV_GNU_VTINHERIT, 41)
> -  RELOC_NUMBER (R_RISCV_GNU_VTENTRY, 42)
>    RELOC_NUMBER (R_RISCV_ALIGN, 43)
>    RELOC_NUMBER (R_RISCV_RVC_BRANCH, 44)
>    RELOC_NUMBER (R_RISCV_RVC_JUMP, 45)
> --
> 2.37.1.559.g78731f0fdb-goog
>