* [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).