Messages in this thread Patch in this message | | | Date | Tue, 26 Jun 2018 13:44:05 -0500 | From | Josh Poimboeuf <> | Subject | Re: [PATCH] objtool: Fix GCC 8 cold function processing without -freorder-functions |
| |
On Tue, Jun 26, 2018 at 07:31:18PM +0100, Allan Xavier wrote: > Hi Peter, > > On 26/06/18 17:43, Peter Zijlstra wrote: > > On Tue, Jun 26, 2018 at 05:20:45PM +0100, Allan Xavier wrote: > >> 0000000000000500 g F .text 0000000000000034 nmi_panic > >> 0000000000000528 l F .text 000000000000000c nmi_panic.cold.7 > >> > >> This doesn't happen with -freorder-functions in the first example as the > >> symbols don't overlap. > > > > Urgh and I don't suppose we can 'fix' the overlap in read_symbols() ? > > > > It should be fixable in read_symbols too if you don't mind sym->len not being > the same as sym->st_size in the ELF. > > Is there a particular concern you're trying to address by having the logic there > instead? > > Will have a look into reworking the patch in any case.
Thanks for the patch and the excellent problem description. I think the concern is that the overlap might cause other unforeseen issues (now or in the future). How about something like this? (The diff also includes a cleanup to reduce the indent level.)
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c index 4e60e105583e..36518393a960 100644 --- a/tools/objtool/elf.c +++ b/tools/objtool/elf.c @@ -302,19 +302,32 @@ static int read_symbols(struct elf *elf) continue; sym->pfunc = sym->cfunc = sym; coldstr = strstr(sym->name, ".cold."); - if (coldstr) { - coldstr[0] = '\0'; - pfunc = find_symbol_by_name(elf, sym->name); - coldstr[0] = '.'; - - if (!pfunc) { - WARN("%s(): can't find parent function", - sym->name); - goto err; - } - - sym->pfunc = pfunc; - pfunc->cfunc = sym; + if (!coldstr) + continue; + + coldstr[0] = '\0'; + pfunc = find_symbol_by_name(elf, sym->name); + coldstr[0] = '.'; + + if (!pfunc) { + WARN("%s(): can't find parent function", + sym->name); + goto err; + } + + sym->pfunc = pfunc; + pfunc->cfunc = sym; + + /* + * Unfortunately, -fnoreorder-functions puts the child + * inside the parent. Remove the overlap so we can + * have sane assumptions. + */ + if (sym->sec == pfunc->sec && + sym->offset >= pfunc->offset && + sym->offset < pfunc->offset + pfunc->len && + sym->offset + sym->len == pfunc->offset + pfunc->len) { + pfunc->len -= sym->len; } } }
| |