Re: [PATCH 6/9] objtool: Add elf_create_undef_symbol()
On Wed, Mar 17, 2021 at 07:46:14PM -0500, Josh Poimboeuf wrote:
> On Wed, Mar 17, 2021 at 03:13:43PM +0100, Peter Zijlstra wrote:
> > On Wed, Mar 17, 2021 at 02:52:23PM +0100, Miroslav Benes wrote:
> >
> > > > + if (!elf_symbol_add(elf, sym, SHN_XINDEX)) {
> > > > + WARN("elf_symbol_add");
> > > > + return NULL;
> > > > + }
> > >
> > > SHN_XINDEX means that the extended section index is used. Above you seem
> > > to use it in the opposite sense too (assigning to shndx when shndx_data
> > > is
> > > NULL). While it makes the code easier to handle, it is a bit confusing
> > > (and maybe I am just confused now). Could you add a comment about that,
> > > please? elf_symbol_add() seems like a good place.
> >
> > Yes, that was a horrible thing to do :/ And you understood it right.
> >
> > Looking at it again, I'm not sure it is actually correct tho; shouldn't
> > elf_create_undef_symbol() also look at gelf_getsymshndx() of symtab ?
> >
> > What toolchain generates these extended sections and how? That is, how
> > do I test this crud..
>
> SHN_XINDEX is basically a special-case extension to original ELF for
> supporting more than 64k sections.
Yeah, I figured it all out again last night; the patch as presented is
hideous but correct(ish). But I've rewritten it and it's all much better
now. Will post later.
Re: [PATCH 6/9] objtool: Add elf_create_undef_symbol()
On Wed, Mar 17, 2021 at 03:13:43PM +0100, Peter Zijlstra wrote:
> On Wed, Mar 17, 2021 at 02:52:23PM +0100, Miroslav Benes wrote:
>
> > > + if (!elf_symbol_add(elf, sym, SHN_XINDEX)) {
> > > + WARN("elf_symbol_add");
> > > + return NULL;
> > > + }
> >
> > SHN_XINDEX means that the extended section index is used. Above you seem
> > to use it in the opposite sense too (assigning to shndx when shndx_data is
> > NULL). While it makes the code easier to handle, it is a bit confusing
> > (and maybe I am just confused now). Could you add a comment about that,
> > please? elf_symbol_add() seems like a good place.
>
> Yes, that was a horrible thing to do :/ And you understood it right.
>
> Looking at it again, I'm not sure it is actually correct tho; shouldn't
> elf_create_undef_symbol() also look at gelf_getsymshndx() of symtab ?
>
> What toolchain generates these extended sections and how? That is, how
> do I test this crud..
SHN_XINDEX is basically a special-case extension to original ELF for
supporting more than 64k sections.
--
Josh
Re: [PATCH 6/9] objtool: Add elf_create_undef_symbol()
On Wed, Mar 17, 2021 at 7:40 AM Miroslav Benes wrote:
>
> On Wed, 17 Mar 2021, Peter Zijlstra wrote:
>
> > On Wed, Mar 17, 2021 at 02:52:23PM +0100, Miroslav Benes wrote:
> >
> > > > + if (!elf_symbol_add(elf, sym, SHN_XINDEX)) {
> > > > + WARN("elf_symbol_add");
> > > > + return NULL;
> > > > + }
> > >
> > > SHN_XINDEX means that the extended section index is used. Above you seem
> > > to use it in the opposite sense too (assigning to shndx when shndx_data is
> > > NULL). While it makes the code easier to handle, it is a bit confusing
> > > (and maybe I am just confused now). Could you add a comment about that,
> > > please? elf_symbol_add() seems like a good place.
> >
> > Yes, that was a horrible thing to do :/ And you understood it right.
> >
> > Looking at it again, I'm not sure it is actually correct tho; shouldn't
> > elf_create_undef_symbol() also look at gelf_getsymshndx() of symtab ?
>
> Probably yes.
>
> > What toolchain generates these extended sections and how? That is, how
> > do I test this crud..
>
> Sami might know.
Clang generates these with LTO for vmlinux.o, but I'm guessing gcc
will do the same with -ffunction-sections -fdata-sections.
Sami
Re: [PATCH 6/9] objtool: Add elf_create_undef_symbol()
On Wed, 17 Mar 2021, Peter Zijlstra wrote:
> On Wed, Mar 17, 2021 at 02:52:23PM +0100, Miroslav Benes wrote:
>
> > > + if (!elf_symbol_add(elf, sym, SHN_XINDEX)) {
> > > + WARN("elf_symbol_add");
> > > + return NULL;
> > > + }
> >
> > SHN_XINDEX means that the extended section index is used. Above you seem
> > to use it in the opposite sense too (assigning to shndx when shndx_data is
> > NULL). While it makes the code easier to handle, it is a bit confusing
> > (and maybe I am just confused now). Could you add a comment about that,
> > please? elf_symbol_add() seems like a good place.
>
> Yes, that was a horrible thing to do :/ And you understood it right.
>
> Looking at it again, I'm not sure it is actually correct tho; shouldn't
> elf_create_undef_symbol() also look at gelf_getsymshndx() of symtab ?
Probably yes.
> What toolchain generates these extended sections and how? That is, how
> do I test this crud..
Sami might know.
Miroslav
Re: [PATCH 6/9] objtool: Add elf_create_undef_symbol()
On Wed, Mar 17, 2021 at 02:52:23PM +0100, Miroslav Benes wrote:
> > + if (!elf_symbol_add(elf, sym, SHN_XINDEX)) {
> > + WARN("elf_symbol_add");
> > + return NULL;
> > + }
>
> SHN_XINDEX means that the extended section index is used. Above you seem
> to use it in the opposite sense too (assigning to shndx when shndx_data is
> NULL). While it makes the code easier to handle, it is a bit confusing
> (and maybe I am just confused now). Could you add a comment about that,
> please? elf_symbol_add() seems like a good place.
Yes, that was a horrible thing to do :/ And you understood it right.
Looking at it again, I'm not sure it is actually correct tho; shouldn't
elf_create_undef_symbol() also look at gelf_getsymshndx() of symtab ?
What toolchain generates these extended sections and how? That is, how
do I test this crud..
Re: [PATCH 6/9] objtool: Add elf_create_undef_symbol()
[ also correcting my e-mail address ]
On Fri, 12 Mar 2021, Peter Zijlstra wrote:
Just a remark regarding SHN_XINDEX...
> +static bool elf_symbol_add(struct elf *elf, struct symbol *sym, Elf32_Word
> shndx)
> +{
> + struct list_head *entry;
> + struct rb_node *pnode;
> +
> + sym->type = GELF_ST_TYPE(sym->sym.st_info);
> + sym->bind = GELF_ST_BIND(sym->sym.st_info);
> +
> + if ((sym->sym.st_shndx > SHN_UNDEF &&
> + sym->sym.st_shndx < SHN_LORESERVE) ||
> + (shndx != SHN_XINDEX && sym->sym.st_shndx == SHN_XINDEX)) {
> + if (sym->sym.st_shndx != SHN_XINDEX)
> + shndx = sym->sym.st_shndx;
> +
> + sym->sec = find_section_by_index(elf, shndx);
> + if (!sym->sec) {
> + WARN("couldn't find section for symbol %s",
> + sym->name);
> + return false;
> + }
...
> @@ -366,47 +414,11 @@ static int read_symbols(struct elf *elf)
> goto err;
> }
>
> - sym->type = GELF_ST_TYPE(sym->sym.st_info);
> - sym->bind = GELF_ST_BIND(sym->sym.st_info);
> + if (!shndx_data)
> + shndx = SHN_XINDEX;
>
> - if ((sym->sym.st_shndx > SHN_UNDEF &&
> - sym->sym.st_shndx < SHN_LORESERVE) ||
> - (shndx_data && sym->sym.st_shndx == SHN_XINDEX)) {
> - if (sym->sym.st_shndx != SHN_XINDEX)
> - shndx = sym->sym.st_shndx;
> -
> - sym->sec = find_section_by_index(elf, shndx);
> - if (!sym->sec) {
> - WARN("couldn't find section for symbol %s",
> - sym->name);
> - goto err;
...
> + if (!elf_symbol_add(elf, sym, SHN_XINDEX)) {
> + WARN("elf_symbol_add");
> + return NULL;
> + }
SHN_XINDEX means that the extended section index is used. Above you seem
to use it in the opposite sense too (assigning to shndx when shndx_data is
NULL). While it makes the code easier to handle, it is a bit confusing
(and maybe I am just confused now). Could you add a comment about that,
please? elf_symbol_add() seems like a good place.
Miroslav
[PATCH 6/9] objtool: Add elf_create_undef_symbol()
Allow objtool to create undefined symbols; this allows creating
relocations to symbols not currently in the symbol table.
Signed-off-by: Peter Zijlstra (Intel)
---
tools/objtool/elf.c | 180 +++-
tools/objtool/include/objtool/elf.h |1
2 files changed, 139 insertions(+), 42 deletions(-)
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -316,12 +316,60 @@ static int read_sections(struct elf *elf
return 0;
}
+static bool elf_symbol_add(struct elf *elf, struct symbol *sym, Elf32_Word
shndx)
+{
+ struct list_head *entry;
+ struct rb_node *pnode;
+
+ sym->type = GELF_ST_TYPE(sym->sym.st_info);
+ sym->bind = GELF_ST_BIND(sym->sym.st_info);
+
+ if ((sym->sym.st_shndx > SHN_UNDEF &&
+sym->sym.st_shndx < SHN_LORESERVE) ||
+ (shndx != SHN_XINDEX && sym->sym.st_shndx == SHN_XINDEX)) {
+ if (sym->sym.st_shndx != SHN_XINDEX)
+ shndx = sym->sym.st_shndx;
+
+ sym->sec = find_section_by_index(elf, shndx);
+ if (!sym->sec) {
+ WARN("couldn't find section for symbol %s",
+sym->name);
+ return false;
+ }
+ if (sym->type == STT_SECTION) {
+ sym->name = sym->sec->name;
+ sym->sec->sym = sym;
+ }
+ } else
+ sym->sec = find_section_by_index(elf, 0);
+
+ sym->offset = sym->sym.st_value;
+ sym->len = sym->sym.st_size;
+
+ rb_add(&sym->node, &sym->sec->symbol_tree, symbol_to_offset);
+ pnode = rb_prev(&sym->node);
+ if (pnode)
+ entry = &rb_entry(pnode, struct symbol, node)->list;
+ else
+ entry = &sym->sec->symbol_list;
+ list_add(&sym->list, entry);
+ elf_hash_add(elf->symbol_hash, &sym->hash, sym->idx);
+ elf_hash_add(elf->symbol_name_hash, &sym->name_hash,
str_hash(sym->name));
+
+ /*
+* Don't store empty STT_NOTYPE symbols in the rbtree. They
+* can exist within a function, confusing the sorting.
+*/
+ if (!sym->len)
+ rb_erase(&sym->node, &sym->sec->symbol_tree);
+
+ return true;
+}
+
static int read_symbols(struct elf *elf)
{
struct section *symtab, *symtab_shndx, *sec;
struct symbol *sym, *pfunc;
- struct list_head *entry;
- struct rb_node *pnode;
int symbols_nr, i;
char *coldstr;
Elf_Data *shndx_data = NULL;
@@ -366,47 +414,11 @@ static int read_symbols(struct elf *elf)
goto err;
}
- sym->type = GELF_ST_TYPE(sym->sym.st_info);
- sym->bind = GELF_ST_BIND(sym->sym.st_info);
+ if (!shndx_data)
+ shndx = SHN_XINDEX;
- if ((sym->sym.st_shndx > SHN_UNDEF &&
-sym->sym.st_shndx < SHN_LORESERVE) ||
- (shndx_data && sym->sym.st_shndx == SHN_XINDEX)) {
- if (sym->sym.st_shndx != SHN_XINDEX)
- shndx = sym->sym.st_shndx;
-
- sym->sec = find_section_by_index(elf, shndx);
- if (!sym->sec) {
- WARN("couldn't find section for symbol %s",
-sym->name);
- goto err;
- }
- if (sym->type == STT_SECTION) {
- sym->name = sym->sec->name;
- sym->sec->sym = sym;
- }
- } else
- sym->sec = find_section_by_index(elf, 0);
-
- sym->offset = sym->sym.st_value;
- sym->len = sym->sym.st_size;
-
- rb_add(&sym->node, &sym->sec->symbol_tree, symbol_to_offset);
- pnode = rb_prev(&sym->node);
- if (pnode)
- entry = &rb_entry(pnode, struct symbol, node)->list;
- else
- entry = &sym->sec->symbol_list;
- list_add(&sym->list, entry);
- elf_hash_add(elf->symbol_hash, &sym->hash, sym->idx);
- elf_hash_add(elf->symbol_name_hash, &sym->name_hash,
str_hash(sym->name));
-
- /*
-* Don't store empty STT_NOTYPE symbols in the rbtree. They
-* can exist within a function, confusing the sorting.
-*/
- if (!sym->len)
- rb_erase(&sym->node, &sym->sec->symbol_tree);
+ if (!elf_symbol_add(elf, sym, shndx))
+ goto err;
}
if (stats)
@@ -640,6 +652,90 @@ struct elf *elf_open_read(const char *na
return NULL;
}
+struct symbol *elf_create_undef_symbol(struct elf *elf, const char *name)
+{
+ struct
