* [PATCH 2/2 v2] Generalize cu_sec_idx @ 2017-12-08 15:06 Ulf Hermann 2017-12-14 13:51 ` Mark Wielaard 0 siblings, 1 reply; 6+ messages in thread From: Ulf Hermann @ 2017-12-08 15:06 UTC (permalink / raw) To: elfutils-devel Apparently CUs can appear in other sections than IDX_debug_info and IDX_types. Rather than relying on the indirect indication provided by type_offset we compare the addresses directly to figure out which section a given CU belongs to. This fixes the dwarf-getmacros test. (Signed-off instead of Change-Id ...) Signed-off-by: Ulf Hermann <ulf.hermann@qt.io> --- libdw/ChangeLog | 4 ++++ libdw/libdwP.h | 12 +++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/libdw/ChangeLog b/libdw/ChangeLog index 996cd2e..508bf9c 100644 --- a/libdw/ChangeLog +++ b/libdw/ChangeLog @@ -1,3 +1,7 @@ +2017-12-08 Ulf Hermann <ulf.hermann@qt.io> + + * libdwP.h: Generalize cu_sec_idx to check all sections. + 2017-05-09 Ulf Hermann <ulf.hermann@qt.io> * libdwP.h: Fix check for the upper border of the range in __libdw_in_section. diff --git a/libdw/libdwP.h b/libdw/libdwP.h index e092d8e..8f3a95c 100644 --- a/libdw/libdwP.h +++ b/libdw/libdwP.h @@ -715,7 +715,17 @@ __libdw_read_offset (Dwarf *dbg, Dwarf *dbg_ret, static inline size_t cu_sec_idx (struct Dwarf_CU *cu) { - return cu->type_offset == 0 ? IDX_debug_info : IDX_debug_types; + for (int sec_index = IDX_debug_info; sec_index < IDX_last; ++sec_index) + { + Elf_Data *data = cu->dbg->sectiondata[sec_index]; + if (data != NULL && data->d_buf != NULL + && cu->startp >= data->d_buf + && cu->startp < data->d_buf + data->d_size) + { + return sec_index; + } + } + return IDX_last; } static inline bool -- 2.8.1.windows.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2 v2] Generalize cu_sec_idx 2017-12-08 15:06 [PATCH 2/2 v2] Generalize cu_sec_idx Ulf Hermann @ 2017-12-14 13:51 ` Mark Wielaard 2017-12-14 13:53 ` Ulf Hermann 0 siblings, 1 reply; 6+ messages in thread From: Mark Wielaard @ 2017-12-14 13:51 UTC (permalink / raw) To: Ulf Hermann, elfutils-devel On Fri, 2017-12-08 at 16:06 +0100, Ulf Hermann wrote: > Apparently CUs can appear in other sections than IDX_debug_info and > IDX_types. Rather than relying on the indirect indication provided by > type_offset we compare the addresses directly to figure out which section > a given CU belongs to. This is clever and indeed cu_sec_idx () is not generic enough. But this is also somewhat inefficient. I am working on DWARF5 support and there a CU can come from even more different sections (or file). So I am changing Dwarf_CU to have an explicit section to which is it is associated. This can then also be used by the "fake" CUs like created in dwarf_getmacros. Thanks, Mark ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2 v2] Generalize cu_sec_idx 2017-12-14 13:51 ` Mark Wielaard @ 2017-12-14 13:53 ` Ulf Hermann 2017-12-20 18:09 ` Mark Wielaard 0 siblings, 1 reply; 6+ messages in thread From: Ulf Hermann @ 2017-12-14 13:53 UTC (permalink / raw) To: Mark Wielaard, elfutils-devel On 12/14/2017 02:51 PM, Mark Wielaard wrote: > This is clever and indeed cu_sec_idx () is not generic enough. > But this is also somewhat inefficient. I am working on DWARF5 support > and there a CU can come from even more different sections (or file). So > I am changing Dwarf_CU to have an explicit section to which is it is > associated. This can then also be used by the "fake" CUs like created > in dwarf_getmacros. Mind that the two most common cases are 0 and 1. In fact nothing else was supported before this change. So, most of the time this will not do a lot of iteration. regards, Ulf ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2 v2] Generalize cu_sec_idx 2017-12-14 13:53 ` Ulf Hermann @ 2017-12-20 18:09 ` Mark Wielaard 2017-12-21 13:48 ` Ulf Hermann 0 siblings, 1 reply; 6+ messages in thread From: Mark Wielaard @ 2017-12-20 18:09 UTC (permalink / raw) To: Ulf Hermann, elfutils-devel [-- Attachment #1: Type: text/plain, Size: 987 bytes --] On Thu, 2017-12-14 at 14:53 +0100, Ulf Hermann wrote: > On 12/14/2017 02:51 PM, Mark Wielaard wrote: > > This is clever and indeed cu_sec_idx () is not generic enough. > > But this is also somewhat inefficient. I am working on DWARF5 support > > and there a CU can come from even more different sections (or file). So > > I am changing Dwarf_CU to have an explicit section to which is it is > > associated. This can then also be used by the "fake" CUs like created > > in dwarf_getmacros. > > Mind that the two most common cases are 0 and 1. In fact nothing else > was supported before this change. So, most of the time this will not > do a lot of iteration. yes, but the original code really was not correct. The attached patch fixes it by adding an explicit sec_idx field to the Dwarf_CU struct that is set whenever a struct Dwarf_CU is created, so that we never have to guess. This patch combined with the overflow fix makes all testcases PASS. Cheers, Mark [-- Attachment #2: 0001-libdw-Add-explicit-section-index-to-struct-Dwarf_CU.patch --] [-- Type: text/x-patch, Size: 4023 bytes --] From 51a7292b7ec7ddebcd2abddc7efff9d604494d44 Mon Sep 17 00:00:00 2001 From: Mark Wielaard <mark@klomp.org> Date: Wed, 20 Dec 2017 16:50:57 +0100 Subject: [PATCH 1/2] libdw: Add explicit section index to struct Dwarf_CU. The DIE (attribute) data might come from either the main .debug_info section or for DWARFv4 from a separate .debug_types section. Or in case of the fake_loc_cu from the .debug_loc section and in the case of macros from the .debug_macinfo or .debug_macro section. We didn't handle the last two "fake" CU cases correctly when sanity checking offsets in __libdw_read_address and __libdw_read_offset. Add an explicit sec_idx field to struct Dwarf_CU that is always set to the actual section that the data came from. Signed-off-by: Mark Wielaard <mark@klomp.org> --- libdw/ChangeLog | 11 +++++++++++ libdw/dwarf_begin_elf.c | 1 + libdw/dwarf_getmacros.c | 1 + libdw/libdwP.h | 4 +++- libdw/libdw_findcu.c | 5 +++-- 5 files changed, 19 insertions(+), 3 deletions(-) diff --git a/libdw/ChangeLog b/libdw/ChangeLog index 350230e..22b7bf4 100644 --- a/libdw/ChangeLog +++ b/libdw/ChangeLog @@ -1,3 +1,14 @@ +2017-12-20 Mark Wielaard <mark@klomp.org> + + * libdwP.h (struct Dwarf_CU): Add sec_idx field. + (cu_sec_idx): Return cu->sec_idx. + * libdw_findcu.c (__libdw_intern_next_unit): Set cu sec_idx to + IDX_debug_info or IDX_debug_types. + * dwarf_begin_elf.c (valid_p): Set fake_loc_cu->sec_idx to + IDX_debug_loc. + * dwarf_getmacros.c (read_macros): Set fake_cu->sec_idx to + IDX_debug_macro or IDX_debug_macinfo. + 2017-12-12 Mark Wielaard <mark@klomp.org> * dwarf_aggregate_size.c (dwarf_aggregate_size): Don't peel the diff --git a/libdw/dwarf_begin_elf.c b/libdw/dwarf_begin_elf.c index afa15ce..7c3fe10 100644 --- a/libdw/dwarf_begin_elf.c +++ b/libdw/dwarf_begin_elf.c @@ -201,6 +201,7 @@ valid_p (Dwarf *result) } else { + result->fake_loc_cu->sec_idx = IDX_debug_loc; result->fake_loc_cu->dbg = result; result->fake_loc_cu->startp = result->sectiondata[IDX_debug_loc]->d_buf; diff --git a/libdw/dwarf_getmacros.c b/libdw/dwarf_getmacros.c index db6582b..c456051 100644 --- a/libdw/dwarf_getmacros.c +++ b/libdw/dwarf_getmacros.c @@ -360,6 +360,7 @@ read_macros (Dwarf *dbg, int sec_index, Version 4 for the old GNU extension, version 5 for DWARF5. */ Dwarf_CU fake_cu = { .dbg = dbg, + .sec_idx = sec_index, .version = table->version, .offset_size = table->is_64bit ? 8 : 4, .startp = (void *) startp + offset, diff --git a/libdw/libdwP.h b/libdw/libdwP.h index 78c0013..f524347 100644 --- a/libdw/libdwP.h +++ b/libdw/libdwP.h @@ -293,6 +293,8 @@ struct Dwarf_CU uint8_t offset_size; uint16_t version; + size_t sec_idx; /* Normally .debug_info, could be .debug_type or "fake". */ + /* Zero if this is a normal CU. Nonzero if it is a type unit. */ size_t type_offset; uint64_t type_sig8; @@ -714,7 +716,7 @@ __libdw_read_offset (Dwarf *dbg, Dwarf *dbg_ret, static inline size_t cu_sec_idx (struct Dwarf_CU *cu) { - return cu->type_offset == 0 ? IDX_debug_info : IDX_debug_types; + return cu->sec_idx; } static inline bool diff --git a/libdw/libdw_findcu.c b/libdw/libdw_findcu.c index 082307b..4e025e2 100644 --- a/libdw/libdw_findcu.c +++ b/libdw/libdw_findcu.c @@ -93,8 +93,8 @@ __libdw_intern_next_unit (Dwarf *dbg, bool debug_types) } /* Invalid or truncated debug section data? */ - Elf_Data *data = dbg->sectiondata[debug_types - ? IDX_debug_types : IDX_debug_info]; + size_t sec_idx = debug_types ? IDX_debug_types : IDX_debug_info; + Elf_Data *data = dbg->sectiondata[sec_idx]; if (unlikely (*offsetp > data->d_size)) *offsetp = data->d_size; @@ -102,6 +102,7 @@ __libdw_intern_next_unit (Dwarf *dbg, bool debug_types) struct Dwarf_CU *newp = libdw_typed_alloc (dbg, struct Dwarf_CU); newp->dbg = dbg; + newp->sec_idx = sec_idx; newp->start = oldoff; newp->end = *offsetp; newp->address_size = address_size; -- 1.8.3.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2 v2] Generalize cu_sec_idx 2017-12-20 18:09 ` Mark Wielaard @ 2017-12-21 13:48 ` Ulf Hermann 2017-12-21 20:22 ` Mark Wielaard 0 siblings, 1 reply; 6+ messages in thread From: Ulf Hermann @ 2017-12-21 13:48 UTC (permalink / raw) To: Mark Wielaard, elfutils-devel > yes, but the original code really was not correct. The attached patch > fixes it by adding an explicit sec_idx field to the Dwarf_CU struct > that is set whenever a struct Dwarf_CU is created, so that we never > have to guess. > > This patch combined with the overflow fix makes all testcases PASS. Looks good to me. Ulf ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2 v2] Generalize cu_sec_idx 2017-12-21 13:48 ` Ulf Hermann @ 2017-12-21 20:22 ` Mark Wielaard 0 siblings, 0 replies; 6+ messages in thread From: Mark Wielaard @ 2017-12-21 20:22 UTC (permalink / raw) To: Ulf Hermann; +Cc: elfutils-devel On Thu, Dec 21, 2017 at 02:48:15PM +0100, Ulf Hermann wrote: > > yes, but the original code really was not correct. The attached patch > > fixes it by adding an explicit sec_idx field to the Dwarf_CU struct > > that is set whenever a struct Dwarf_CU is created, so that we never > > have to guess. > > > > This patch combined with the overflow fix makes all testcases PASS. > > Looks good to me. Thanks again. Pushed to master. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-12-21 20:22 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-12-08 15:06 [PATCH 2/2 v2] Generalize cu_sec_idx Ulf Hermann 2017-12-14 13:51 ` Mark Wielaard 2017-12-14 13:53 ` Ulf Hermann 2017-12-20 18:09 ` Mark Wielaard 2017-12-21 13:48 ` Ulf Hermann 2017-12-21 20:22 ` Mark Wielaard
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).