Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 2/2] objtool: Add support for relocations without addends
To:     Matt Helsley <mhelsley@vmware.com>, linux-kernel@vger.kernel.org
Cc:     Josh Poimboeuf <jpoimboe@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Steven Rostedt <rostedt@goodmis.org>
References: <cover.1590785960.git.mhelsley@vmware.com>
 <af8b5d5bffc64af00ad88e0b7bd216e91cbfdb36.1590785960.git.mhelsley@vmware.com>
From:   Julien Thierry <jthierry@redhat.com>
Message-ID: <a7ce0b8a-557b-01e6-9a0a-d986c3dbd639@redhat.com>
Date:   Tue, 2 Jun 2020 08:36:48 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.5.0
MIME-Version: 1.0
In-Reply-To: <af8b5d5bffc64af00ad88e0b7bd216e91cbfdb36.1590785960.git.mhelsley@vmware.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Matt,

On 5/29/20 10:01 PM, Matt Helsley wrote:
> Currently objtool only collects information about relocations with
> addends. In recordmcount, which we are about to merge into objtool,
> some supported architectures do not use rela relocations.
> 
> Signed-off-by: Matt Helsley <mhelsley@vmware.com>
> ---
>   tools/objtool/elf.c     | 146 +++++++++++++++++++++++++++++++++++-----
>   tools/objtool/elf.h     |   7 +-
>   tools/objtool/orc_gen.c |   2 +-
>   3 files changed, 135 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> index c21e8417637b..5034918494b6 100644
> --- a/tools/objtool/elf.c
> +++ b/tools/objtool/elf.c
> @@ -490,6 +490,32 @@ void elf_add_reloc(struct elf *elf, struct reloc *reloc)
>   	elf_hash_add(elf->reloc_hash, &reloc->hash, reloc_hash(reloc));
>   }
>   
> +static int read_rel_reloc(struct section *sec, int i, struct reloc *reloc, unsigned int *symndx)
> +{
> +	if (!gelf_getrel(sec->data, i, &reloc->rel)) {
> +		WARN_ELF("gelf_getrel");
> +		return -1;
> +	}
> +	reloc->type = GELF_R_TYPE(reloc->rel.r_info);
> +	reloc->addend = 0;
> +	reloc->offset = reloc->rel.r_offset;
> +	*symndx = GELF_R_SYM(reloc->rel.r_info);
> +	return 0;
> +}
> +
> +static int read_rela_reloc(struct section *sec, int i, struct reloc *reloc, unsigned int *symndx)
> +{
> +	if (!gelf_getrela(sec->data, i, &reloc->rela)) {
> +		WARN_ELF("gelf_getrela");
> +		return -1;
> +	}
> +	reloc->type = GELF_R_TYPE(reloc->rela.r_info);
> +	reloc->addend = reloc->rela.r_addend;
> +	reloc->offset = reloc->rela.r_offset;
> +	*symndx = GELF_R_SYM(reloc->rela.r_info);
> +	return 0;
> +}
> +
>   static int read_relocs(struct elf *elf)
>   {
>   	struct section *sec;
> @@ -499,7 +525,8 @@ static int read_relocs(struct elf *elf)
>   	unsigned long nr_reloc, max_reloc = 0, tot_reloc = 0;
>   
>   	list_for_each_entry(sec, &elf->sections, list) {
> -		if (sec->sh.sh_type != SHT_RELA)
> +		if ((sec->sh.sh_type != SHT_RELA) &&
> +		    (sec->sh.sh_type != SHT_REL))
>   			continue;
>   
>   		sec->base = find_section_by_index(elf, sec->sh.sh_info);
> @@ -519,16 +546,17 @@ static int read_relocs(struct elf *elf)
>   				return -1;
>   			}
>   			memset(reloc, 0, sizeof(*reloc));
> -
> -			if (!gelf_getrela(sec->data, i, &reloc->rela)) {
> -				WARN_ELF("gelf_getrela");
> -				return -1;
> +			switch(sec->sh.sh_type) {
> +			case SHT_REL:
> +				if (read_rel_reloc(sec, i, reloc, &symndx))
> +					return -1;
> +				break;
> +			case SHT_RELA:
> +				if (read_rela_reloc(sec, i, reloc, &symndx))
> +					return -1;
> +				break;
> +			default: return -1;
>   			}
> -
> -			reloc->type = GELF_R_TYPE(reloc->rela.r_info);
> -			reloc->addend = reloc->rela.r_addend;
> -			reloc->offset = reloc->rela.r_offset;
> -			symndx = GELF_R_SYM(reloc->rela.r_info);
>   			reloc->sym = find_symbol_by_index(elf, symndx);
>   			reloc->sec = sec;
>   			if (!reloc->sym) {
> @@ -716,7 +744,38 @@ struct section *elf_create_section(struct elf *elf, const char *name,
>   	return sec;
>   }
>   
> -struct section *elf_create_reloc_section(struct elf *elf, struct section *base)
> +static struct section *elf_create_rel_reloc_section(struct elf *elf, struct section *base)
> +{
> +	char *relocname;
> +	struct section *sec;
> +
> +	relocname = malloc(strlen(base->name) + strlen(".rel") + 1);
> +	if (!relocname) {
> +		perror("malloc");
> +		return NULL;
> +	}
> +	strcpy(relocname, ".rel");
> +	strcat(relocname, base->name);
> +
> +	sec = elf_create_section(elf, relocname, sizeof(GElf_Rel), 0);
> +	free(relocname);
> +	if (!sec)
> +		return NULL;
> +
> +	base->reloc = sec;
> +	sec->base = base;
> +
> +	sec->sh.sh_type = SHT_REL;
> +	sec->sh.sh_addralign = 8;
> +	sec->sh.sh_link = find_section_by_name(elf, ".symtab")->idx;
> +	sec->sh.sh_info = base->idx;
> +	sec->sh.sh_flags = SHF_INFO_LINK;
> +
> +	return sec;
> +}
> +
> +

Nit: Extra new line.

Otherwise:

Reviewed-by: Julien Thierry <jthierry@redhat.com>

-- 
Julien Thierry