From: Julien Thierry <jthierry@redhat.com>
To: Matt Helsley <mhelsley@vmware.com>, linux-kernel@vger.kernel.org
Cc: Josh Poimboeuf <jpoimboe@redhat.com>,
Peter Zijlstra <peterz@infradead.org>,
Miroslav Benes <mbenes@suse.cz>,
Steven Rostedt <rostedt@goodmis.org>
Subject: Re: [RFC][PATCH 3/5] objtool: Add support for relocations without addends
Date: Tue, 12 May 2020 18:04:50 +0100 [thread overview]
Message-ID: <f79a3872-b9ad-a966-71fb-4451e0901281@redhat.com> (raw)
In-Reply-To: <17ee3f6f2a246008aaae70f92df24ae92fa0e21e.1588888003.git.mhelsley@vmware.com>
Hi Matt,
On 5/11/20 6:35 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. Since
> object files use one or the other the list can be reused.
>
> Signed-off-by: Matt Helsley <mhelsley@vmware.com>
> ---
> tools/objtool/elf.c | 55 ++++++++++++++++++++++++++++++++++++---------
> tools/objtool/elf.h | 5 ++++-
> 2 files changed, 49 insertions(+), 11 deletions(-)
>
> diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> index c4857fa3f1d1..cd841e3df87d 100644
> --- a/tools/objtool/elf.c
> +++ b/tools/objtool/elf.c
> @@ -465,12 +465,14 @@ static int read_relas(struct elf *elf)
> unsigned long nr_rela, max_rela = 0, tot_rela = 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_name(elf, sec->name + 5);
> + sec->base = find_section_by_name(elf, sec->name +
> + ((sec->sh.sh_type != SHT_REL) ? 5 : 4));
> if (!sec->base) {
> - WARN("can't find base section for rela section %s",
> + WARN("can't find base section for relocation section %s",
> sec->name);
> return -1;
> }
> @@ -486,13 +488,26 @@ static int read_relas(struct elf *elf)
> }
> memset(rela, 0, sizeof(*rela));
>
> - if (!gelf_getrela(sec->data, i, &rela->rela)) {
> - WARN_ELF("gelf_getrela");
> - return -1;
> + switch(sec->sh.sh_type) {
> + case SHT_REL:
> + if (!gelf_getrel(sec->data, i, &rela->rel)) {
> + WARN_ELF("gelf_getrel");
> + return -1;
> + }
> + rela->addend = 0;
> + break;
> + case SHT_RELA:
> + if (!gelf_getrela(sec->data, i, &rela->rela)) {
> + WARN_ELF("gelf_getrela");
> + return -1;
> + }
> + rela->addend = rela->rela.r_addend;
> + break;
> + default:
> + break;
> }
>
> rela->type = GELF_R_TYPE(rela->rela.r_info);
> - rela->addend = rela->rela.r_addend;
> rela->offset = rela->rela.r_offset;
> symndx = GELF_R_SYM(rela->rela.r_info);
> rela->sym = find_symbol_by_index(elf, symndx);
> @@ -717,17 +732,27 @@ int elf_rebuild_rela_section(struct section *sec)
> struct rela *rela;
> int nr, idx = 0, size;
> GElf_Rela *relas;
> + GElf_Rel *rels;
>
> nr = 0;
> list_for_each_entry(rela, &sec->rela_list, list)
> nr++;
>
> + /*
> + * Allocate a buffer for relocations with addends but also use
> + * it for other relocations too. The section type determines
> + * the size of the section, the buffer used, and the entries.
> + */
> size = nr * sizeof(*relas);
> relas = malloc(size);
> if (!relas) {
> perror("malloc");
> return -1;
> }
> + rels = (void *)relas;
> + if (sec->sh.sh_type == SHT_REL) {
> + size = nr * sizeof(*rels);
> + }
This looks a bit error prone to me.
What about having:
void *rel_buf;
[...]
size = nr * (sec->sh.sh_type == SHT_REL ? sizeof(GElf_Rel) :
sizeof(GElf_Rela));
rel_buf = malloc(size);
[...]
And then casting rel_buf to the correct pointer type in the fitting
switch cases?
>
> sec->data->d_buf = relas;
> sec->data->d_size = size;
> @@ -736,9 +761,19 @@ int elf_rebuild_rela_section(struct section *sec)
>
> idx = 0;
> list_for_each_entry(rela, &sec->rela_list, list) {
> - relas[idx].r_offset = rela->offset;
> - relas[idx].r_addend = rela->addend;
> - relas[idx].r_info = GELF_R_INFO(rela->sym->idx, rela->type);
> + switch(sec->sh.sh_type) {
> + case SHT_REL:
> + rels[idx].r_offset = rela->offset;
> + rels[idx].r_info = GELF_R_INFO(rela->sym->idx, rela->type);
> + break;
> + case SHT_RELA:
> + relas[idx].r_addend = rela->addend;
> + relas[idx].r_offset = rela->offset;
> + relas[idx].r_info = GELF_R_INFO(rela->sym->idx, rela->type);
> + break;
> + default:
> + break;
> + }
> idx++;
> }
>
> diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h
> index 0b79c2353a21..71bd47055963 100644
> --- a/tools/objtool/elf.h
> +++ b/tools/objtool/elf.h
> @@ -61,7 +61,10 @@ struct symbol {
> struct rela {
> struct list_head list;
> struct hlist_node hash;
> - GElf_Rela rela;
> + union {
> + GElf_Rela rela;
> + GElf_Rel rel;
> + };
> struct section *sec;
> struct symbol *sym;
> unsigned int type;
>
--
Julien Thierry
next prev parent reply other threads:[~2020-05-12 17:05 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-11 17:35 [RFC][PATCH 0/5] Enable objtool multiarch build Matt Helsley
2020-05-11 17:35 ` [RFC][PATCH 1/5] objtool: Exit successfully when requesting help Matt Helsley
2020-05-15 19:52 ` Josh Poimboeuf
2020-05-18 18:33 ` Matt Helsley
2020-05-11 17:35 ` [RFC][PATCH 2/5] objtool: Move struct objtool_file into arch-independent header Matt Helsley
2020-05-12 17:04 ` Julien Thierry
2020-05-12 18:07 ` Matt Helsley
2020-05-11 17:35 ` [RFC][PATCH 3/5] objtool: Add support for relocations without addends Matt Helsley
2020-05-12 17:04 ` Julien Thierry [this message]
2020-05-13 16:26 ` Matt Helsley
2020-05-13 16:55 ` Julien Thierry
2020-05-14 21:09 ` Matt Helsley
2020-05-15 20:33 ` Josh Poimboeuf
2020-05-18 19:14 ` Matt Helsley
2020-05-11 17:35 ` [RFC][PATCH 4/5] objtool: Enable compilation of objtool for all architectures Matt Helsley
2020-05-12 17:04 ` Julien Thierry
2020-05-13 15:59 ` Matt Helsley
2020-05-13 16:55 ` Julien Thierry
2020-05-15 20:51 ` Josh Poimboeuf
2020-05-18 18:26 ` Matt Helsley
2020-05-15 20:56 ` Josh Poimboeuf
2020-05-18 19:20 ` Matt Helsley
2020-05-18 19:50 ` Matt Helsley
2020-05-18 22:27 ` Josh Poimboeuf
2020-05-19 17:48 ` Matt Helsley
2020-05-11 17:35 ` [RFC][PATCH 5/5] objtool: Report missing support for subcommands Matt Helsley
2020-05-15 21:04 ` Josh Poimboeuf
2020-05-18 18:29 ` Matt Helsley
2020-05-12 17:04 ` [RFC][PATCH 0/5] Enable objtool multiarch build Julien Thierry
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=f79a3872-b9ad-a966-71fb-4451e0901281@redhat.com \
--to=jthierry@redhat.com \
--cc=jpoimboe@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mbenes@suse.cz \
--cc=mhelsley@vmware.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
/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 NNTP newsgroup(s).