On Mon, 2014-03-10 at 14:37 -0700, Josh Stone wrote: > On 03/10/2014 02:23 PM, Mark Wielaard wrote: > > On Fri, 2014-03-07 at 17:39 -0800, Josh Stone wrote: > >> Maybe this only needs to add a check that file == &mod->main > > > > Yes, I think that would be the correct thing to do. Both find_dw and > > find_symtab call __libdwfl_getelf first. So the main ELF file will > > always be loaded through open_elf first. After mod->e_type has been set > > it should not be set or changed again by either debug of aux file > > opening. > > Ok. I attached my simple patch which still passes the testsuite, and > also fixes my issue -- let me know how it works for you.
Yes, that works for me. I added an extra comment explaining the why and added an assert to make sure our internal invariant holds. If you don't mind adding your Signed-off-by line to the attached commit (see also the CONTRIBUTING file) could you push it to master? Or let me know I should. Thanks, Mark
>From 65f8bca67b7f6b5a03b0c1789adf828b0c886220 Mon Sep 17 00:00:00 2001 From: Josh Stone <jist...@redhat.com> Date: Tue, 11 Mar 2014 11:35:30 +0100 Subject: [PATCH] libdwfl: dwfl_module_getdwarf.c (open_elf) only (re)set mod->e_type once. As noted in https://sourceware.org/bugzilla/show_bug.cgi?id=16676#c2 for systemtap the heuristic used by open_elf to set the kernel Dwfl_Module type to ET_DYN even if the underlying ELF file e_type was set to ET_EXEC could trigger erroneously for non-kernel/non-main (debug or aux) files. Make sure we only set the e_type of the module once when processing the main file (when the phdrs can be trusted). --- libdwfl/ChangeLog | 5 +++++ libdwfl/dwfl_module_getdwarf.c | 26 ++++++++++++++++++++------ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog index 12938f3..8530787 100644 --- a/libdwfl/ChangeLog +++ b/libdwfl/ChangeLog @@ -1,3 +1,8 @@ +2014-03-11 Josh Stone <jist...@redhat.com> + + * dwfl_module_getdwarf.c (open_elf): Only explicitly set + mod->e_type when processing the main ELF file. + 2014-03-04 Mark Wielaard <m...@redhat.com> * libdwflP.h (struct __libdwfl_pid_arg): Moved here and renamed from diff --git a/libdwfl/dwfl_module_getdwarf.c b/libdwfl/dwfl_module_getdwarf.c index c4bd739..dcc1adb 100644 --- a/libdwfl/dwfl_module_getdwarf.c +++ b/libdwfl/dwfl_module_getdwarf.c @@ -1,5 +1,5 @@ /* Find debugging and symbol information for a module in libdwfl. - Copyright (C) 2005-2012 Red Hat, Inc. + Copyright (C) 2005-2012, 2014 Red Hat, Inc. This file is part of elfutils. This file is free software; you can redistribute it and/or modify @@ -77,7 +77,7 @@ open_elf (Dwfl_Module *mod, struct dwfl_file *file) return DWFL_E (LIBELF, elf_errno ()); } - if (mod->e_type != ET_REL) + if (ehdr->e_type != ET_REL) { /* In any non-ET_REL file, we compute the "synchronization address". @@ -131,11 +131,25 @@ open_elf (Dwfl_Module *mod, struct dwfl_file *file) } } - mod->e_type = ehdr->e_type; + /* We only want to set the module e_type explictly once derived from + the main ELF file (it might be changed for the kernel, because + that is special, see below). open_elf is always called first for + the main ELF file because both find_dw and find_symtab call + __libdwfl_getelf first to open the main file. So don't let debug + or aux files override the module e_type. The kernel heuristic + below could otherwise trigger for non-kernel/non-main files since + for non main ELF files the phdrs might not match the actual load + addresses. */ + if (file == &mod->main) + { + mod->e_type = ehdr->e_type; - /* Relocatable Linux kernels are ET_EXEC but act like ET_DYN. */ - if (mod->e_type == ET_EXEC && file->vaddr != mod->low_addr) - mod->e_type = ET_DYN; + /* Relocatable Linux kernels are ET_EXEC but act like ET_DYN. */ + if (mod->e_type == ET_EXEC && file->vaddr != mod->low_addr) + mod->e_type = ET_DYN; + } + else + assert (mod->main.elf != NULL); return DWFL_E_NOERROR; } -- 1.7.1
