Skip to content

Commit 13f60e8

Browse files
author
Peter Zijlstra
committed
objtool: Avoid O(bloody terrible) behaviour -- an ode to libelf
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> Tested-by: Yujie Liu <yujie.liu@intel.com> Link: https://lkml.kernel.org/r/20221028194453.461658986@infradead.org
1 parent 4c91be8 commit 13f60e8

File tree

2 files changed

+84
-7
lines changed

2 files changed

+84
-7
lines changed

tools/objtool/elf.c

Lines changed: 83 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -634,6 +634,12 @@ static int elf_update_symbol(struct elf *elf, struct section *symtab,
634634

635635
/* end-of-list */
636636
if (!symtab_data) {
637+
/*
638+
* Over-allocate to avoid O(n^2) symbol creation
639+
* behaviour. The down side is that libelf doesn't
640+
* like this; see elf_truncate_section() for the fixup.
641+
*/
642+
int num = max(1U, sym->idx/3);
637643
void *buf;
638644

639645
if (idx) {
@@ -647,28 +653,34 @@ static int elf_update_symbol(struct elf *elf, struct section *symtab,
647653
if (t)
648654
shndx_data = elf_newdata(t);
649655

650-
buf = calloc(1, entsize);
656+
buf = calloc(num, entsize);
651657
if (!buf) {
652658
WARN("malloc");
653659
return -1;
654660
}
655661

656662
symtab_data->d_buf = buf;
657-
symtab_data->d_size = entsize;
663+
symtab_data->d_size = num * entsize;
658664
symtab_data->d_align = 1;
659665
symtab_data->d_type = ELF_T_SYM;
660666

661-
symtab->sh.sh_size += entsize;
662667
symtab->changed = true;
668+
symtab->truncate = true;
663669

664670
if (t) {
665-
shndx_data->d_buf = &sym->sec->idx;
666-
shndx_data->d_size = sizeof(Elf32_Word);
671+
buf = calloc(num, sizeof(Elf32_Word));
672+
if (!buf) {
673+
WARN("malloc");
674+
return -1;
675+
}
676+
677+
shndx_data->d_buf = buf;
678+
shndx_data->d_size = num * sizeof(Elf32_Word);
667679
shndx_data->d_align = sizeof(Elf32_Word);
668680
shndx_data->d_type = ELF_T_WORD;
669681

670-
symtab_shndx->sh.sh_size += sizeof(Elf32_Word);
671682
symtab_shndx->changed = true;
683+
symtab_shndx->truncate = true;
672684
}
673685

674686
break;
@@ -770,6 +782,14 @@ __elf_create_symbol(struct elf *elf, struct symbol *sym)
770782
return NULL;
771783
}
772784

785+
symtab->sh.sh_size += symtab->sh.sh_entsize;
786+
symtab->changed = true;
787+
788+
if (symtab_shndx) {
789+
symtab_shndx->sh.sh_size += sizeof(Elf32_Word);
790+
symtab_shndx->changed = true;
791+
}
792+
773793
return sym;
774794
}
775795

@@ -1286,6 +1306,60 @@ int elf_write_reloc(struct elf *elf, struct reloc *reloc)
12861306
return 0;
12871307
}
12881308

1309+
/*
1310+
* When Elf_Scn::sh_size is smaller than the combined Elf_Data::d_size
1311+
* do you:
1312+
*
1313+
* A) adhere to the section header and truncate the data, or
1314+
* B) ignore the section header and write out all the data you've got?
1315+
*
1316+
* Yes, libelf sucks and we need to manually truncate if we over-allocate data.
1317+
*/
1318+
static int elf_truncate_section(struct elf *elf, struct section *sec)
1319+
{
1320+
u64 size = sec->sh.sh_size;
1321+
bool truncated = false;
1322+
Elf_Data *data = NULL;
1323+
Elf_Scn *s;
1324+
1325+
s = elf_getscn(elf->elf, sec->idx);
1326+
if (!s) {
1327+
WARN_ELF("elf_getscn");
1328+
return -1;
1329+
}
1330+
1331+
for (;;) {
1332+
/* get next data descriptor for the relevant section */
1333+
data = elf_getdata(s, data);
1334+
1335+
if (!data) {
1336+
if (size) {
1337+
WARN("end of section data but non-zero size left\n");
1338+
return -1;
1339+
}
1340+
return 0;
1341+
}
1342+
1343+
if (truncated) {
1344+
/* when we remove symbols */
1345+
WARN("truncated; but more data\n");
1346+
return -1;
1347+
}
1348+
1349+
if (!data->d_size) {
1350+
WARN("zero size data");
1351+
return -1;
1352+
}
1353+
1354+
if (data->d_size > size) {
1355+
truncated = true;
1356+
data->d_size = size;
1357+
}
1358+
1359+
size -= data->d_size;
1360+
}
1361+
}
1362+
12891363
int elf_write(struct elf *elf)
12901364
{
12911365
struct section *sec;
@@ -1296,6 +1370,9 @@ int elf_write(struct elf *elf)
12961370

12971371
/* Update changed relocation sections and section headers: */
12981372
list_for_each_entry(sec, &elf->sections, list) {
1373+
if (sec->truncate)
1374+
elf_truncate_section(elf, sec);
1375+
12991376
if (sec->changed) {
13001377
s = elf_getscn(elf->elf, sec->idx);
13011378
if (!s) {

tools/objtool/include/objtool/elf.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ struct section {
3838
Elf_Data *data;
3939
char *name;
4040
int idx;
41-
bool changed, text, rodata, noinstr, init;
41+
bool changed, text, rodata, noinstr, init, truncate;
4242
};
4343

4444
struct symbol {

0 commit comments

Comments
 (0)