On Thu, May 31, 2018 at 01:02:44PM +0200, Mark Wielaard wrote:
> Add explicit test in get-units-invalid for dwarf_cuoffset and
> dwarf_dieoffset.
And that test caught another bug on 32bit systems!
> Dwarf_Off
> dwarf_dieoffset (Dwarf_Die *die)
> {
> - return (die == NULL
> + return ((die == NULL || die->cu == NULL)
> ? ~0ul
> : (Dwarf_Off) (die->addr - die->cu->startp + die->cu->start));
Note that ~0ul != (Dwarf_Off) -1 on 32bit systems.
So error detection was always broken.
The reason we didn't notice before was because we had a similar bug
in eu-readelf...
I am checking in the attached patch which fixes both.
>From aa02fb9028abcadaa18440b86b1ed085e029956c Mon Sep 17 00:00:00 2001
From: Mark Wielaard <m...@klomp.org>
Date: Thu, 31 May 2018 13:01:39 +0200
Subject: [PATCH] libdw: Don't crash on invalid die in dwarf_dieoffset.
Add explicit test in get-units-invalid for dwarf_cuoffset and
dwarf_dieoffset. Make sure dwarf_dieoffset returns (Dwarf_Off) -1
on failure.
Signed-off-by: Mark Wielaard <m...@klomp.org>
---
libdw/ChangeLog | 5 +++++
libdw/dwarf_dieoffset.c | 4 ++--
src/ChangeLog | 4 ++++
src/readelf.c | 2 +-
tests/ChangeLog | 5 +++++
tests/get-units-invalid.c | 14 +++++++++++++-
6 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 5a33d9c..38b45ba 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,8 @@
+2018-05-31 Mark Wielaard <m...@klomp.org>
+
+ * dwarf_dieoffset.c: Check die->cu != NULL. Return -1, not ~0ul
+ on failure.
+
2018-05-29 Mark Wielaard <m...@klomp.org>
* dwarf_cuoffset.c (dwarf_cuoffset): Check die->cu is not NULL.
diff --git a/libdw/dwarf_dieoffset.c b/libdw/dwarf_dieoffset.c
index 8028f6d..3a8e2cb 100644
--- a/libdw/dwarf_dieoffset.c
+++ b/libdw/dwarf_dieoffset.c
@@ -38,8 +38,8 @@
Dwarf_Off
dwarf_dieoffset (Dwarf_Die *die)
{
- return (die == NULL
- ? ~0ul
+ return ((die == NULL || die->cu == NULL)
+ ? (Dwarf_Off) -1
: (Dwarf_Off) (die->addr - die->cu->startp + die->cu->start));
}
INTDEF(dwarf_dieoffset)
diff --git a/src/ChangeLog b/src/ChangeLog
index f424fb7..03ed5aa 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,7 @@
+2018-05-31 Mark Wielaard <m...@klomp.org>
+
+ * readelf.c (print_debug_units): Check offset against -1 not ~0ul.
+
2018-05-29 Mark Wielaard <m...@klomp.org>
* readelf.c (print_debug_loc_section): Handle GNU DebugFission list
diff --git a/src/readelf.c b/src/readelf.c
index 2ccbea5..470a94e 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -7588,7 +7588,7 @@ print_debug_units (Dwfl_Module *dwflmod,
do
{
Dwarf_Off offset = dwarf_dieoffset (&dies[level]);
- if (unlikely (offset == ~0ul))
+ if (unlikely (offset == (Dwarf_Off) -1))
{
if (!silent)
error (0, 0, gettext ("cannot get DIE offset: %s"),
diff --git a/tests/ChangeLog b/tests/ChangeLog
index b656bee..521df52 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,8 @@
+2018-05-31 Mark Wielaard <m...@klomp.org>
+
+ * get-units-invalid.c (main): Check dwarf_cuoffset and
+ dwarf_dieoffset.
+
2018-05-29 Mark Wielaard <m...@klomp.org>
* dwarf-die-addr-die.c (check_dbg): Also check subdies, split or
diff --git a/tests/get-units-invalid.c b/tests/get-units-invalid.c
index 58b32c0..ba0f818 100644
--- a/tests/get-units-invalid.c
+++ b/tests/get-units-invalid.c
@@ -83,7 +83,19 @@ main (int argc, char *argv[])
if (dwarf_ranges (&subdie, 0, &base, &start, &end) != -1)
{
printf ("Should NOT have a ranges: %s\n",
- dwarf_diename (&result));
+ dwarf_diename (&subdie));
+ return -1;
+ }
+ if (dwarf_cuoffset (&subdie) != (Dwarf_Off) -1)
+ {
+ printf ("Should NOT have a cuoffset: %s\n",
+ dwarf_diename (&subdie));
+ return -1;
+ }
+ if (dwarf_dieoffset (&subdie) != (Dwarf_Off) -1)
+ {
+ printf ("Should NOT have a die offset: %s\n",
+ dwarf_diename (&subdie));
return -1;
}
}
--
1.8.3.1
