Re: [RFC PATCH v1 13/23] objtool: Add next member in struct reloc

[Peter Zijlstra]

On Tue, Jun 20, 2023 at 03:50:09PM +0800, Youling Tang wrote:
> In LoongArch, there may be multiple relocation information in one location,
> so the next member is added to handle this situation.

So Josh did a shrink on struct reloc because there are too many of them;
ideally we find another way to link them for the case where it is
needed.

> 
> The following warning appears when the next member is not added,
> warning: objtool: unexpected relocation symbol type in .rela.discard.unreachable
> 
> Relocation section '.rela.discard.unreachable' at offset 0x1a58 contains 4 entries:
>     Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
> 0000000000000000  0000000200000032 R_LARCH_ADD32          0000000000000000 .text + 354
> 0000000000000000  0000000900000037 R_LARCH_SUB32          0000000000000000 L0^A + 0
> 
> Co-developed-by: Jinyang He <hejinyang@xxxxxxxxxxx>
> Signed-off-by: Jinyang He <hejinyang@xxxxxxxxxxx>
> Signed-off-by: Youling Tang <tangyouling@xxxxxxxxxxx>
> ---
>  tools/objtool/elf.c                 | 11 ++++++++++-
>  tools/objtool/include/objtool/elf.h |  1 +
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> index 6806ce01d933..d345300d269b 100644
> --- a/tools/objtool/elf.c
> +++ b/tools/objtool/elf.c
> @@ -895,7 +895,7 @@ static int read_relocs(struct elf *elf)
>  {
>  	unsigned long nr_reloc, max_reloc = 0, tot_reloc = 0;
>  	struct section *sec;
> -	struct reloc *reloc;
> +	struct reloc *reloc, *next_reloc;
>  	unsigned int symndx;
>  	struct symbol *sym;
>  	int i;
> @@ -915,6 +915,7 @@ static int read_relocs(struct elf *elf)
>  			return -1;
>  		}
>  
> +		next_reloc = NULL;
>  		sec->base->reloc = sec;
>  
>  		nr_reloc = 0;
> @@ -946,6 +947,14 @@ static int read_relocs(struct elf *elf)
>  				return -1;
>  			}
>  
> +			if (next_reloc && reloc->offset == next_reloc->offset) {
> +				next_reloc->next = reloc;
> +				next_reloc = reloc;
> +				continue;
> +			}
> +
> +			next_reloc = reloc;

This seems to rely on 'linked' reloc being adjecent in the ELF tables;
is this required by the LoongArch ELF spec? If not, you really should
not rely on it.

> +
>  			list_add_tail(&reloc->sym_reloc_entry, &sym->reloc_list);
>  			list_add_tail(&reloc->list, &sec->reloc_list);
>  			elf_hash_add(reloc, &reloc->hash, reloc_hash(reloc));
> diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
> index ad0024da262b..7877298fe401 100644
> --- a/tools/objtool/include/objtool/elf.h
> +++ b/tools/objtool/include/objtool/elf.h
> @@ -68,6 +68,7 @@ struct symbol {
>  struct reloc {
>  	struct list_head list;
>  	struct hlist_node hash;
> +	struct reloc *next;
>  	union {
>  		GElf_Rela rela;
>  		GElf_Rel  rel;
> -- 
> 2.39.2
>