From: Peter Zijlstra <peterz@infradead.org>
To: x86@kernel.org
Cc: linux-kernel@vger.kernel.org, peterz@infradead.org,
djwong@kernel.org, yujie.liu@intel.com, tglx@linutronix.de,
jpoimboe@kernel.org, joao.moreira@intel.com,
samitolvanen@google.com
Subject: [PATCH 3/5] objtool: Avoid O(bloody terrible) behaviour -- an ode to libelf
Date: Fri, 28 Oct 2022 21:40:25 +0200 [thread overview]
Message-ID: <20221028194453.461658986@infradead.org> (raw)
In-Reply-To: <20221028194022.388521751@infradead.org>
Due to how gelf_update_sym*() requires an Elf_Data pointer, and how
libelf keeps Elf_Data in a linked list per section,
elf_update_symbol() ends up having to iterate this list on each
update to find the correct Elf_Data for the index'ed symbol.
By allocating one Elf_Data per new symbol, the list grows per new
symbol, giving an effective O(n^2) insertion time. This is obviously
bloody terrible.
Therefore over-allocate the Elf_Data when an extention is needed.
Except it turns out libelf disregards Elf_Scn::sh_size in favour of
the sum of Elf_Data::d_size. IOW it will happily write out all the
unused space and fill it with:
0000000000000000 0 NOTYPE LOCAL DEFAULT UND
entries (aka zeros). Which obviously violates the STB_LOCAL placement
rule, and is a general pain in the backside for not being the desired
behaviour.
Manually fix-up the Elf_Data size to avoid this problem before calling
elf_update().
This significantly improves performance when adding a significant
number of symbols.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
tools/objtool/elf.c | 89 +++++++++++++++++++++++++++++++++---
tools/objtool/include/objtool/elf.h | 2
2 files changed, 84 insertions(+), 7 deletions(-)
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -634,6 +634,12 @@ static int elf_update_symbol(struct elf
/* end-of-list */
if (!symtab_data) {
+ /*
+ * Over-allocate to avoid O(n^2) symbol creation
+ * behaviour. The down side is that libelf doesn't
+ * like this; see elf_truncate_section() for the fixup.
+ */
+ int num = max(1U, sym->idx/3);
void *buf;
if (idx) {
@@ -647,28 +653,34 @@ static int elf_update_symbol(struct elf
if (t)
shndx_data = elf_newdata(t);
- buf = calloc(1, entsize);
+ buf = calloc(num, entsize);
if (!buf) {
WARN("malloc");
return -1;
}
symtab_data->d_buf = buf;
- symtab_data->d_size = entsize;
+ symtab_data->d_size = num * entsize;
symtab_data->d_align = 1;
symtab_data->d_type = ELF_T_SYM;
- symtab->sh.sh_size += entsize;
symtab->changed = true;
+ symtab->truncate = true;
if (t) {
- shndx_data->d_buf = &sym->sec->idx;
- shndx_data->d_size = sizeof(Elf32_Word);
+ buf = calloc(num, sizeof(Elf32_Word));
+ if (!buf) {
+ WARN("malloc");
+ return -1;
+ }
+
+ shndx_data->d_buf = buf;
+ shndx_data->d_size = num * sizeof(Elf32_Word);
shndx_data->d_align = sizeof(Elf32_Word);
shndx_data->d_type = ELF_T_WORD;
- symtab_shndx->sh.sh_size += sizeof(Elf32_Word);
symtab_shndx->changed = true;
+ symtab_shndx->truncate = true;
}
break;
@@ -770,6 +782,14 @@ __elf_create_symbol(struct elf *elf, str
return NULL;
}
+ symtab->sh.sh_size += symtab->sh.sh_entsize;
+ symtab->changed = true;
+
+ if (symtab_shndx) {
+ symtab_shndx->sh.sh_size += sizeof(Elf32_Word);
+ symtab_shndx->changed = true;
+ }
+
return sym;
}
@@ -1286,6 +1306,60 @@ int elf_write_reloc(struct elf *elf, str
return 0;
}
+/*
+ * When Elf_Scn::sh_size is smaller than the combined Elf_Data::d_size
+ * do you:
+ *
+ * A) adhere to the section header and truncate the data, or
+ * B) ignore the section header and write out all the data you've got?
+ *
+ * Yes, libelf sucks and we need to manually truncate if we over-allocate data.
+ */
+static int elf_truncate_section(struct elf *elf, struct section *sec)
+{
+ u64 size = sec->sh.sh_size;
+ bool truncated = false;
+ Elf_Data *data = NULL;
+ Elf_Scn *s;
+
+ s = elf_getscn(elf->elf, sec->idx);
+ if (!s) {
+ WARN_ELF("elf_getscn");
+ return -1;
+ }
+
+ for (;;) {
+ /* get next data descriptor for the relevant section */
+ data = elf_getdata(s, data);
+
+ if (!data) {
+ if (size) {
+ WARN("end of section data but non-zero size left\n");
+ return -1;
+ }
+ return 0;
+ }
+
+ if (truncated) {
+ /* when we remove symbols */
+ WARN("truncated; but more data\n");
+ return -1;
+ }
+
+ if (!data->d_size) {
+ WARN("zero size data");
+ return -1;
+ }
+
+ if (data->d_size > size) {
+ truncated = true;
+ data->d_size = size;
+ }
+
+ size -= data->d_size;
+ }
+}
+
int elf_write(struct elf *elf)
{
struct section *sec;
@@ -1296,6 +1370,9 @@ int elf_write(struct elf *elf)
/* Update changed relocation sections and section headers: */
list_for_each_entry(sec, &elf->sections, list) {
+ if (sec->truncate)
+ elf_truncate_section(elf, sec);
+
if (sec->changed) {
s = elf_getscn(elf->elf, sec->idx);
if (!s) {
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -38,7 +38,7 @@ struct section {
Elf_Data *data;
char *name;
int idx;
- bool changed, text, rodata, noinstr, init;
+ bool changed, text, rodata, noinstr, init, truncate;
};
struct symbol {
next prev parent reply other threads:[~2022-10-28 19:48 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-28 19:40 [PATCH 0/5] x86/kallsyms: Fix the call-thunk kallsyms fail Peter Zijlstra
2022-10-28 19:40 ` [PATCH 1/5] kallsyms: Revert "Take callthunks into account" Peter Zijlstra
2022-10-28 19:40 ` [PATCH 2/5] objtool: Slice up elf_create_section_symbol() Peter Zijlstra
2022-11-02 9:20 ` [tip: x86/core] " tip-bot2 for Peter Zijlstra
2022-10-28 19:40 ` Peter Zijlstra [this message]
2022-11-02 9:20 ` [tip: x86/core] objtool: Avoid O(bloody terrible) behaviour -- an ode to libelf tip-bot2 for Peter Zijlstra
2022-11-02 22:22 ` [PATCH 3/5] " Josh Poimboeuf
2022-11-03 8:46 ` Peter Zijlstra
2022-10-28 19:40 ` [PATCH 4/5] objtool: Add option to generate prefix symbols Peter Zijlstra
2022-11-02 9:20 ` [tip: x86/core] " tip-bot2 for Peter Zijlstra
2022-11-02 23:41 ` [PATCH 4/5] " Josh Poimboeuf
2022-10-28 19:40 ` [PATCH 5/5] x86: Add prefix symbols for function padding Peter Zijlstra
2022-11-02 9:20 ` [tip: x86/core] " tip-bot2 for Peter Zijlstra
2022-10-30 9:15 ` [PATCH 0/5] x86/kallsyms: Fix the call-thunk kallsyms fail Peter Zijlstra
2022-10-31 6:18 ` Yujie Liu
2022-11-02 21:46 ` [PATCH 6/5] objtool: Optimize elf_dirty_reloc_sym() Peter Zijlstra
2022-11-05 10:36 ` [tip: x86/core] " tip-bot2 for Peter Zijlstra
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=20221028194453.461658986@infradead.org \
--to=peterz@infradead.org \
--cc=djwong@kernel.org \
--cc=joao.moreira@intel.com \
--cc=jpoimboe@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=samitolvanen@google.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
--cc=yujie.liu@intel.com \
/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
all inboxes | Powered by JetHome®