Messages in this thread Patch in this message | | | Date | Thu, 18 Mar 2021 19:19:31 -0500 | From | Josh Poimboeuf <> | Subject | Re: [PATCH 5/9] objtool: Rework rebuild_reloc logic |
| |
On Thu, Mar 18, 2021 at 12:38:42PM -0500, Josh Poimboeuf wrote: > On Thu, Mar 18, 2021 at 06:04:25PM +0100, Peter Zijlstra wrote: > > On Thu, Mar 18, 2021 at 11:36:40AM -0500, Josh Poimboeuf wrote: > > > > I was thinking you could get a section changed without touching > > > > relocations, but while that is theoretically possible, it is exceedingly > > > > unlikely (and objtool doesn't do that). > > > > > > Hm? This is a *relocation* section, not a normal one. So by > > > definition, it only changes when its relocations change. > > > > The way I read this code: > > > > list_for_each_entry(sec, &elf->sections, list) { > > if (sec->changed) { > > + if (sec->reloc && > > + elf_rebuild_reloc_section(elf, sec->reloc)) { > > + WARN_ELF("elf_rebuild_reloc_section"); > > + return -1; > > + } > > > > is that we iterate the regular sections (which could be dirtied because > > we changed some data), and if that section has a relocation section, we > > rebuild that for good measure (even though it might not have altered > > relocations). > > > > Or am I just totally confused ? > > Ah, you're right. I'm the one that's confused. I guess I was also > confused when I wrote that hunk, but it just happens to work anyway. > > It would be cleaner to do something like > > if ((is_reloc_sec(sec) && > elf_rebuild_reloc_section(elf, sec)) { > > so we process the changed reloc section directly, instead of relying on > the (most likely) fact that the corresponding text section also changed.
i.e., in actual code:
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c index 66c49c2e20a6..3b3d19a5e626 100644 --- a/tools/objtool/elf.c +++ b/tools/objtool/elf.c @@ -948,8 +948,7 @@ int elf_write(struct elf *elf) /* Update changed relocation sections and section headers: */ list_for_each_entry(sec, &elf->sections, list) { if (sec->changed) { - if (sec->reloc && - elf_rebuild_reloc_section(elf, sec->reloc)) { + if (sec->base && elf_rebuild_reloc_section(elf, sec)) { WARN_ELF("elf_rebuild_reloc_section"); return -1; }
| |