lkml.org 
[lkml]   [2010]   [Dec]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH v4] perf symbols: fix symbol offset breakage with separated debug
    Date
    v4: remove assert() calls

    * Simply removed the assert() to validate section indices
    returned by libelf: libelf should validate the ELF file
    anyway.

    * Replaced the assert() on arguments to dso__load_sym
    with a run-time check.

    v3: Fixed a minor error in v2 where a check was erronueously
    done twice.

    Original description follows:

    The new patch loads the ELF section headers from a separate
    file if necessary, to avoid getting confused by the different
    section file offsets seen in debug images. Invalid section
    headers are detected by checking for the presence of non-
    writable SHT_NOBITS sections, which don't make sense under
    normal circumstances.

    In particular, this allows symbols in ET_EXEC images to get
    fixed up correctly in the presence of separated debug images.

    The image search loop is also tidied up to fix a minor bug
    which would perform the same image load attempt more than
    once in some situations.

    For non-separated images, the headers should get loaded from
    the same image as the symbols, in the usual way.
    ---
    KernelVersion: 2.6.37-rc3

    tools/perf/util/symbol.c | 188 +++++++++++++++++++++++++++++++++++++++++++---
    1 files changed, 176 insertions(+), 12 deletions(-)

    diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
    index b39f499..7d8fd34 100644
    --- a/tools/perf/util/symbol.c
    +++ b/tools/perf/util/symbol.c
    @@ -992,8 +992,105 @@ static size_t elf_addr_to_index(Elf *elf, GElf_Addr addr)
    return -1;
    }

    +/**
    + * Read all section headers, copying them into a separate array so they survive
    + * elf_end.
    + *
    + * @elf: the libelf instance to operate on.
    + * @ehdr: the elf header: this must already have been read with gelf_getehdr().
    + * @count: the number of headers read is assigned to *count on successful
    + * return. count must not be NULL.
    + *
    + * Returns a pointer to the allocated headers, which should be deallocated with
    + * free() when no longer needed.
    + */
    +static GElf_Shdr *elf_get_all_shdrs(Elf *elf, GElf_Ehdr const *ehdr,
    + unsigned *count)
    +{
    + GElf_Shdr *shdrs;
    + Elf_Scn *scn;
    + unsigned max_index = 0;
    + unsigned i;
    +
    + shdrs = malloc(ehdr->e_shnum * sizeof *shdrs);
    + if (!shdrs)
    + return NULL;
    +
    + for (i = 0; i < ehdr->e_shnum; i++)
    + shdrs[i].sh_type = SHT_NULL;
    +
    + for (scn = NULL; (scn = elf_nextscn(elf, scn)); ) {
    + size_t j;
    +
    + /*
    + * Just assuming we get section 0, 1, ... in sequence may lead
    + * to wrong section indices. Check the index explicitly:
    + */
    + j = elf_ndxscn(scn);
    +
    + if (j > max_index)
    + max_index = j;
    +
    + if (!gelf_getshdr(scn, &shdrs[j]))
    + goto error;
    + }
    +
    + *count = max_index + 1;
    + return shdrs;
    +
    +error:
    + free(shdrs);
    + return NULL;
    +}
    +
    +/**
    + * Check that the section headers @shdrs reflect accurately the file data
    + * layout of the image that was loaded during perf record. This is generally
    + * not true for separated debug images generated with e.g.,
    + * objcopy --only-keep-debug.
    + *
    + * We identify invalid files by checking for non-empty sections which are
    + * declared as having no file data (SHT_NOBITS) but are not writable.
    + *
    + * @shdrs: the full set of section headers, as loaded by elf_get_all_shdrs().
    + * @count: the number of headers present in @shdrs.
    + *
    + * Returns 1 for valid headers, 0 otherwise.
    + */
    +static int elf_check_shdrs_valid(GElf_Shdr const *shdrs, unsigned count)
    +{
    + unsigned i;
    +
    + for (i = 0; i < count; i++) {
    + if (shdrs[i].sh_type == SHT_NOBITS &&
    + !(shdrs[i].sh_flags & SHF_WRITE) &&
    + shdrs[i].sh_size != 0)
    + return 0;
    + }
    +
    + return 1;
    +}
    +
    +/*
    + * Notes:
    + *
    + * If saved_shdrs is non-NULL, the section headers will be read if found, and
    + * will be used for address fixups. saved_shdrs_count must also be non-NULL in
    + * this case. This may be needed for separated debug images, since the section
    + * headers and symbols may need to come from separate images in that case.
    + *
    + * Note: irrespective of whether this function returns successfully,
    + * *saved_shdrs may get initialised if saved_shdrs is non-NULL. It is the
    + * caller's responsibility to free() it when non longer needed.
    + *
    + * If want_symtab == 1, this function will only load symbols from .symtab
    + * sections. Otherwise (want_symtab == 0), .dynsym or .symtab symbols are
    + * loaded. This feature is used by dso__load() to search for the best image
    + * to load.
    + */
    static int dso__load_sym(struct dso *self, struct map *map, const char *name,
    int fd, symbol_filter_t filter, int kmodule,
    + GElf_Shdr **saved_shdrs, unsigned *saved_shdrs_count,
    int want_symtab)
    {
    struct kmap *kmap = self->kernel ? map__kmap(map) : NULL;
    @@ -1012,6 +1109,17 @@ static int dso__load_sym(struct dso *self, struct map *map, const char *name,
    int nr = 0;
    size_t opdidx = 0;

    + if(saved_shdrs != NULL && saved_shdrs_count == NULL) {
    + /*
    + * If you trigger this check, you're calling this function
    + * incorrectly. Refer to the notes above for details.
    + */
    + pr_debug("%s: warning: saved_shdrs_count == NULL: "
    + "ignoring the saved section headers.\n",
    + __func__);
    + saved_shdrs = NULL;
    + }
    +
    elf = elf_begin(fd, PERF_ELF_C_READ_MMAP, NULL);
    if (elf == NULL) {
    pr_debug("%s: cannot read %s ELF file.\n", __func__, name);
    @@ -1035,6 +1143,34 @@ static int dso__load_sym(struct dso *self, struct map *map, const char *name,
    goto out_elf_end;
    }

    + /*
    + * Copy all section headers from the image if requested and if not
    + * already loaded.
    + */
    + if (saved_shdrs != NULL && *saved_shdrs == NULL) {
    + GElf_Shdr *shdrs;
    + unsigned count;
    +
    + shdrs = elf_get_all_shdrs(elf, &ehdr, &count);
    + if (shdrs == NULL)
    + goto out_elf_end;
    +
    + /*
    + * Only keep the headers if they reflect the actual run-time
    + * image's file layout:
    + */
    + if (elf_check_shdrs_valid(shdrs, count)) {
    + *saved_shdrs = shdrs;
    + *saved_shdrs_count = count;
    + } else
    + free(shdrs);
    + }
    +
    + /* If no genuine ELF headers are available yet, give up: we can't
    + * adjust symbols correctly in that case: */
    + if (saved_shdrs != NULL && *saved_shdrs == NULL)
    + goto out_elf_end;
    +
    sec = elf_section_by_name(elf, &ehdr, &shdr, ".symtab", NULL);
    if (sec == NULL) {
    if (want_symtab)
    @@ -1167,12 +1303,25 @@ static int dso__load_sym(struct dso *self, struct map *map, const char *name,
    goto new_symbol;
    }

    + /*
    + * Currently, symbols for shared objects and PIE executables
    + * (i.e., ET_DYN) do not seem to get adjusted. This might need
    + * to change if file offset == virtual address is not actually
    + * guaranteed for these images. ELF doesn't provide this
    + * guarantee natively.
    + */
    if (curr_dso->adjust_symbols) {
    pr_debug4("%s: adjusting symbol: st_value: %#Lx "
    "sh_addr: %#Lx sh_offset: %#Lx\n", __func__,
    (u64)sym.st_value, (u64)shdr.sh_addr,
    (u64)shdr.sh_offset);
    - sym.st_value -= shdr.sh_addr - shdr.sh_offset;
    + if (saved_shdrs && *saved_shdrs &&
    + sym.st_shndx < *saved_shdrs_count)
    + sym.st_value -=
    + (*saved_shdrs)[sym.st_shndx].sh_addr -
    + (*saved_shdrs)[sym.st_shndx].sh_offset;
    + else
    + sym.st_value -= shdr.sh_addr - shdr.sh_offset;
    }
    /*
    * We need to figure out if the object was created from C++ sources
    @@ -1409,6 +1558,8 @@ int dso__load(struct dso *self, struct map *map, symbol_filter_t filter)
    struct machine *machine;
    const char *root_dir;
    int want_symtab;
    + GElf_Shdr *saved_shdrs = NULL;
    + unsigned saved_shdrs_count;

    dso__set_loaded(self, map->type);

    @@ -1439,13 +1590,13 @@ int dso__load(struct dso *self, struct map *map, symbol_filter_t filter)
    * On the first pass, only load images if they have a full symtab.
    * Failing that, do a second pass where we accept .dynsym also
    */
    - for (self->origin = DSO__ORIG_BUILD_ID_CACHE, want_symtab = 1;
    - self->origin != DSO__ORIG_NOT_FOUND;
    - self->origin++) {
    + self->origin = DSO__ORIG_BUILD_ID_CACHE;
    + want_symtab = 1;
    + while (1) {
    switch (self->origin) {
    case DSO__ORIG_BUILD_ID_CACHE:
    if (dso__build_id_filename(self, name, size) == NULL)
    - continue;
    + goto continue_next;
    break;
    case DSO__ORIG_FEDORA:
    snprintf(name, size, "/usr/lib/debug%s.debug",
    @@ -1459,7 +1610,7 @@ int dso__load(struct dso *self, struct map *map, symbol_filter_t filter)
    char build_id_hex[BUILD_ID_SIZE * 2 + 1];

    if (!self->has_build_id)
    - continue;
    + goto continue_next;

    build_id__sprintf(self->build_id,
    sizeof(self->build_id),
    @@ -1483,21 +1634,24 @@ int dso__load(struct dso *self, struct map *map, symbol_filter_t filter)
    default:
    /*
    * If we wanted a full symtab but no image had one,
    - * relax our requirements and repeat the search.
    + * relax our requirements and repeat the search,
    + * providing we saw some valid section headers:
    */
    - if (want_symtab) {
    + if (want_symtab && saved_shdrs != NULL) {
    want_symtab = 0;
    self->origin = DSO__ORIG_BUILD_ID_CACHE;
    - } else
    continue;
    + } else
    + goto done;
    }

    /* Name is now the name of the next image to try */
    fd = open(name, O_RDONLY);
    if (fd < 0)
    - continue;
    + goto continue_next;

    ret = dso__load_sym(self, map, name, fd, filter, 0,
    + &saved_shdrs, &saved_shdrs_count,
    want_symtab);
    close(fd);

    @@ -1506,7 +1660,7 @@ int dso__load(struct dso *self, struct map *map, symbol_filter_t filter)
    * info!?!?
    */
    if (!ret)
    - continue;
    + goto continue_next;

    if (ret > 0) {
    int nr_plt = dso__synthesize_plt_symbols(self, map, filter);
    @@ -1514,9 +1668,18 @@ int dso__load(struct dso *self, struct map *map, symbol_filter_t filter)
    ret += nr_plt;
    break;
    }
    +
    +continue_next:
    + self->origin++;
    + continue;
    }

    +done:
    free(name);
    +
    + if (saved_shdrs)
    + free(saved_shdrs);
    +
    if (ret < 0 && strstr(self->name, " (deleted)") != NULL)
    return 0;
    return ret;
    @@ -1782,7 +1945,8 @@ static int dso__load_vmlinux(struct dso *self, struct map *map,
    return -1;

    dso__set_loaded(self, map->type);
    - err = dso__load_sym(self, map, vmlinux, fd, filter, 0, 0);
    + err = dso__load_sym(self, map, vmlinux, fd, filter, 0,
    + NULL, NULL, 0);
    close(fd);

    if (err > 0)
    --
    1.7.1


    \
     
     \ /
      Last update: 2010-12-06 18:27    [W:0.041 / U:31.516 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site