* [PATCH] libdw: Don't free uninitialized Dwarf_Abbrev_Hash's of "fake" CUs.
@ 2019-10-31 2:54 Jonathon Anderson
2019-10-31 23:00 ` Mark Wielaard
0 siblings, 1 reply; 6+ messages in thread
From: Jonathon Anderson @ 2019-10-31 2:54 UTC (permalink / raw)
To: elfutils-devel; +Cc: Mark Wielaard, Jonathon Anderson
fake_{loc,loclists,addr}_cu are Dwarf_CUs that are created separate from
all the others, so their contents are minimal and mostly initialized by
a calloc. On dwarf_end however, they are freed through the same code path
as all the others, so they call DAH_free like all the others. This changes
that so that these three are exempt from DAH and split-DWARF matters, and
swaps the calloc for a malloc so Memcheck will catch any others.
---
Sorry to keep adding patches, but ran across this while tinkering about.
Its not a current issue, but when the concurrent hash table is integrated this
could cause some issues (on some system somewhere, maybe). In that case there
are enough moving internals that a calloc may not initialize it properly.
Its also an error waiting to happen, so this is this just cleans it up with
some semblance of logical correctness.
(In other news, I finally figured out why git send-email wasn't working for me!
Thanks for everyone's patience, especially Wielaard's :) )
libdw/dwarf_begin_elf.c | 15 ++++++++++++---
libdw/dwarf_end.c | 22 +++++++++++++---------
2 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/libdw/dwarf_begin_elf.c b/libdw/dwarf_begin_elf.c
index 8d137414..1b2c5a45 100644
--- a/libdw/dwarf_begin_elf.c
+++ b/libdw/dwarf_begin_elf.c
@@ -223,7 +223,7 @@ valid_p (Dwarf *result)
inside the .debug_loc or .debug_loclists section. */
if (result != NULL && result->sectiondata[IDX_debug_loc] != NULL)
{
- result->fake_loc_cu = (Dwarf_CU *) calloc (1, sizeof (Dwarf_CU));
+ result->fake_loc_cu = (Dwarf_CU *) malloc (sizeof (Dwarf_CU));
if (unlikely (result->fake_loc_cu == NULL))
{
Dwarf_Sig8_Hash_free (&result->sig8_hash);
@@ -240,12 +240,15 @@ valid_p (Dwarf *result)
result->fake_loc_cu->endp
= (result->sectiondata[IDX_debug_loc]->d_buf
+ result->sectiondata[IDX_debug_loc]->d_size);
+ result->fake_loc_cu->locs = NULL;
+ result->fake_loc_cu->address_size = 0;
+ result->fake_loc_cu->version = 0;
}
}
if (result != NULL && result->sectiondata[IDX_debug_loclists] != NULL)
{
- result->fake_loclists_cu = (Dwarf_CU *) calloc (1, sizeof (Dwarf_CU));
+ result->fake_loclists_cu = (Dwarf_CU *) malloc (sizeof (Dwarf_CU));
if (unlikely (result->fake_loclists_cu == NULL))
{
Dwarf_Sig8_Hash_free (&result->sig8_hash);
@@ -263,6 +266,9 @@ valid_p (Dwarf *result)
result->fake_loclists_cu->endp
= (result->sectiondata[IDX_debug_loclists]->d_buf
+ result->sectiondata[IDX_debug_loclists]->d_size);
+ result->fake_loclists_cu->locs = NULL;
+ result->fake_loclists_cu->address_size = 0;
+ result->fake_loclists_cu->version = 0;
}
}
@@ -272,7 +278,7 @@ valid_p (Dwarf *result)
inside the .debug_addr section, if it exists. */
if (result != NULL && result->sectiondata[IDX_debug_addr] != NULL)
{
- result->fake_addr_cu = (Dwarf_CU *) calloc (1, sizeof (Dwarf_CU));
+ result->fake_addr_cu = (Dwarf_CU *) malloc (sizeof (Dwarf_CU));
if (unlikely (result->fake_addr_cu == NULL))
{
Dwarf_Sig8_Hash_free (&result->sig8_hash);
@@ -291,6 +297,9 @@ valid_p (Dwarf *result)
result->fake_addr_cu->endp
= (result->sectiondata[IDX_debug_addr]->d_buf
+ result->sectiondata[IDX_debug_addr]->d_size);
+ result->fake_addr_cu->locs = NULL;
+ result->fake_addr_cu->address_size = 0;
+ result->fake_addr_cu->version = 0;
}
}
diff --git a/libdw/dwarf_end.c b/libdw/dwarf_end.c
index a2e94436..a32a149e 100644
--- a/libdw/dwarf_end.c
+++ b/libdw/dwarf_end.c
@@ -52,18 +52,22 @@ cu_free (void *arg)
{
struct Dwarf_CU *p = (struct Dwarf_CU *) arg;
- Dwarf_Abbrev_Hash_free (&p->abbrev_hash);
-
tdestroy (p->locs, noop_free);
- /* Free split dwarf one way (from skeleton to split). */
- if (p->unit_type == DW_UT_skeleton
- && p->split != NULL && p->split != (void *)-1)
+ if(p != p->dbg->fake_loc_cu && p != p->dbg->fake_loclists_cu
+ && p != p->dbg->fake_addr_cu)
{
- /* The fake_addr_cu might be shared, only release one. */
- if (p->dbg->fake_addr_cu == p->split->dbg->fake_addr_cu)
- p->split->dbg->fake_addr_cu = NULL;
- INTUSE(dwarf_end) (p->split->dbg);
+ Dwarf_Abbrev_Hash_free (&p->abbrev_hash);
+
+ /* Free split dwarf one way (from skeleton to split). */
+ if (p->unit_type == DW_UT_skeleton
+ && p->split != NULL && p->split != (void *)-1)
+ {
+ /* The fake_addr_cu might be shared, only release one. */
+ if (p->dbg->fake_addr_cu == p->split->dbg->fake_addr_cu)
+ p->split->dbg->fake_addr_cu = NULL;
+ INTUSE(dwarf_end) (p->split->dbg);
+ }
}
}
--
2.24.0.rc1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libdw: Don't free uninitialized Dwarf_Abbrev_Hash's of "fake" CUs.
2019-10-31 2:54 [PATCH] libdw: Don't free uninitialized Dwarf_Abbrev_Hash's of "fake" CUs Jonathon Anderson
@ 2019-10-31 23:00 ` Mark Wielaard
2019-10-31 23:43 ` Jonathon Anderson
0 siblings, 1 reply; 6+ messages in thread
From: Mark Wielaard @ 2019-10-31 23:00 UTC (permalink / raw)
To: Jonathon Anderson; +Cc: elfutils-devel
Hi Jonathon,
On Wed, Oct 30, 2019 at 09:29:02PM -0500, Jonathon Anderson wrote:
> fake_{loc,loclists,addr}_cu are Dwarf_CUs that are created separate from
> all the others, so their contents are minimal and mostly initialized by
> a calloc. On dwarf_end however, they are freed through the same code path
> as all the others, so they call DAH_free like all the others. This changes
> that so that these three are exempt from DAH and split-DWARF matters, and
> swaps the calloc for a malloc so Memcheck will catch any others.
Thanks for catching that!
> ---
> Sorry to keep adding patches, but ran across this while tinkering about.
>
> Its not a current issue, but when the concurrent hash table is
> integrated this could cause some issues (on some system somewhere,
> maybe). In that case there are enough moving internals that a calloc
> may not initialize it properly.
>
> Its also an error waiting to happen, so this is this just cleans it
> up with some semblance of logical correctness.
Yes, this is very good. But... see below.
> (In other news, I finally figured out why git send-email wasn't
> working for me! Thanks for everyone's patience, especially
> Wielaard's :) )
Works perfectly.
> diff --git a/libdw/dwarf_begin_elf.c b/libdw/dwarf_begin_elf.c
> index 8d137414..1b2c5a45 100644
> --- a/libdw/dwarf_begin_elf.c
> +++ b/libdw/dwarf_begin_elf.c
> @@ -223,7 +223,7 @@ valid_p (Dwarf *result)
> inside the .debug_loc or .debug_loclists section. */
> if (result != NULL && result->sectiondata[IDX_debug_loc] != NULL)
> {
> - result->fake_loc_cu = (Dwarf_CU *) calloc (1, sizeof (Dwarf_CU));
> + result->fake_loc_cu = (Dwarf_CU *) malloc (sizeof (Dwarf_CU));
> if (unlikely (result->fake_loc_cu == NULL))
> {
> Dwarf_Sig8_Hash_free (&result->sig8_hash);
> @@ -240,12 +240,15 @@ valid_p (Dwarf *result)
> result->fake_loc_cu->endp
> = (result->sectiondata[IDX_debug_loc]->d_buf
> + result->sectiondata[IDX_debug_loc]->d_size);
> + result->fake_loc_cu->locs = NULL;
> + result->fake_loc_cu->address_size = 0;
> + result->fake_loc_cu->version = 0;
> }
> }
>
> if (result != NULL && result->sectiondata[IDX_debug_loclists] != NULL)
> {
> - result->fake_loclists_cu = (Dwarf_CU *) calloc (1, sizeof (Dwarf_CU));
> + result->fake_loclists_cu = (Dwarf_CU *) malloc (sizeof (Dwarf_CU));
> if (unlikely (result->fake_loclists_cu == NULL))
> {
> Dwarf_Sig8_Hash_free (&result->sig8_hash);
> @@ -263,6 +266,9 @@ valid_p (Dwarf *result)
> result->fake_loclists_cu->endp
> = (result->sectiondata[IDX_debug_loclists]->d_buf
> + result->sectiondata[IDX_debug_loclists]->d_size);
> + result->fake_loclists_cu->locs = NULL;
> + result->fake_loclists_cu->address_size = 0;
> + result->fake_loclists_cu->version = 0;
> }
> }
>
> @@ -272,7 +278,7 @@ valid_p (Dwarf *result)
> inside the .debug_addr section, if it exists. */
> if (result != NULL && result->sectiondata[IDX_debug_addr] != NULL)
> {
> - result->fake_addr_cu = (Dwarf_CU *) calloc (1, sizeof (Dwarf_CU));
> + result->fake_addr_cu = (Dwarf_CU *) malloc (sizeof (Dwarf_CU));
> if (unlikely (result->fake_addr_cu == NULL))
> {
> Dwarf_Sig8_Hash_free (&result->sig8_hash);
> @@ -291,6 +297,9 @@ valid_p (Dwarf *result)
> result->fake_addr_cu->endp
> = (result->sectiondata[IDX_debug_addr]->d_buf
> + result->sectiondata[IDX_debug_addr]->d_size);
> + result->fake_addr_cu->locs = NULL;
> + result->fake_addr_cu->address_size = 0;
> + result->fake_addr_cu->version = 0;
> }
> }
This all looks correct.
> diff --git a/libdw/dwarf_end.c b/libdw/dwarf_end.c
> index a2e94436..a32a149e 100644
> --- a/libdw/dwarf_end.c
> +++ b/libdw/dwarf_end.c
> @@ -52,18 +52,22 @@ cu_free (void *arg)
> {
> struct Dwarf_CU *p = (struct Dwarf_CU *) arg;
>
> - Dwarf_Abbrev_Hash_free (&p->abbrev_hash);
> -
> tdestroy (p->locs, noop_free);
>
> - /* Free split dwarf one way (from skeleton to split). */
> - if (p->unit_type == DW_UT_skeleton
> - && p->split != NULL && p->split != (void *)-1)
> + if(p != p->dbg->fake_loc_cu && p != p->dbg->fake_loclists_cu
> + && p != p->dbg->fake_addr_cu)
> {
> - /* The fake_addr_cu might be shared, only release one. */
> - if (p->dbg->fake_addr_cu == p->split->dbg->fake_addr_cu)
> - p->split->dbg->fake_addr_cu = NULL;
> - INTUSE(dwarf_end) (p->split->dbg);
> + Dwarf_Abbrev_Hash_free (&p->abbrev_hash);
> +
> + /* Free split dwarf one way (from skeleton to split). */
> + if (p->unit_type == DW_UT_skeleton
> + && p->split != NULL && p->split != (void *)-1)
> + {
> + /* The fake_addr_cu might be shared, only release one. */
> + if (p->dbg->fake_addr_cu == p->split->dbg->fake_addr_cu)
> + p->split->dbg->fake_addr_cu = NULL;
> + INTUSE(dwarf_end) (p->split->dbg);
> + }
> }
The Dwarf_Abbrev_Hash_free looks in the correct place. But I don't
believe the Free split dwarf hunk should not be under the same if
not-fake block.
Thanks,
Mark
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] libdw: Don't free uninitialized Dwarf_Abbrev_Hash's of "fake" CUs.
2019-10-31 23:00 ` Mark Wielaard
@ 2019-10-31 23:43 ` Jonathon Anderson
2019-11-01 9:00 ` Mark Wielaard
0 siblings, 1 reply; 6+ messages in thread
From: Jonathon Anderson @ 2019-10-31 23:43 UTC (permalink / raw)
To: elfutils-devel; +Cc: Mark Wielaard, Jonathon Anderson
fake_{loc,loclists,addr}_cu are Dwarf_CUs that are created separate from
all the others, so their contents are minimal and mostly initialized by
a calloc. On dwarf_end however, they are freed through the same code path
as all the others, so they call DAH_free like all the others. This changes
that so that these three are exempt from DAH and split-DWARF matters, and
swaps the calloc for a malloc so Memcheck will catch any others.
---
> The Dwarf_Abbrev_Hash_free looks in the correct place. But I don't
> believe the Free split dwarf hunk should not be under the same if
> not-fake block.
That can be fixed.
I don't really like `unit_type = 0`, but its the only value that isn't one
of the DW_UT values. And its what the calloc would've done.
libdw/dwarf_begin_elf.c | 21 ++++++++++++++++++---
libdw/dwarf_end.c | 9 +++++++--
2 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/libdw/dwarf_begin_elf.c b/libdw/dwarf_begin_elf.c
index 8d137414..ebb3eaea 100644
--- a/libdw/dwarf_begin_elf.c
+++ b/libdw/dwarf_begin_elf.c
@@ -223,7 +223,7 @@ valid_p (Dwarf *result)
inside the .debug_loc or .debug_loclists section. */
if (result != NULL && result->sectiondata[IDX_debug_loc] != NULL)
{
- result->fake_loc_cu = (Dwarf_CU *) calloc (1, sizeof (Dwarf_CU));
+ result->fake_loc_cu = (Dwarf_CU *) malloc (sizeof (Dwarf_CU));
if (unlikely (result->fake_loc_cu == NULL))
{
Dwarf_Sig8_Hash_free (&result->sig8_hash);
@@ -240,12 +240,17 @@ valid_p (Dwarf *result)
result->fake_loc_cu->endp
= (result->sectiondata[IDX_debug_loc]->d_buf
+ result->sectiondata[IDX_debug_loc]->d_size);
+ result->fake_loc_cu->locs = NULL;
+ result->fake_loc_cu->address_size = 0;
+ result->fake_loc_cu->version = 0;
+ result->fake_loc_cu->unit_type = 0;
+ result->fake_loc_cu->split = NULL;
}
}
if (result != NULL && result->sectiondata[IDX_debug_loclists] != NULL)
{
- result->fake_loclists_cu = (Dwarf_CU *) calloc (1, sizeof (Dwarf_CU));
+ result->fake_loclists_cu = (Dwarf_CU *) malloc (sizeof (Dwarf_CU));
if (unlikely (result->fake_loclists_cu == NULL))
{
Dwarf_Sig8_Hash_free (&result->sig8_hash);
@@ -263,6 +268,11 @@ valid_p (Dwarf *result)
result->fake_loclists_cu->endp
= (result->sectiondata[IDX_debug_loclists]->d_buf
+ result->sectiondata[IDX_debug_loclists]->d_size);
+ result->fake_loclists_cu->locs = NULL;
+ result->fake_loclists_cu->address_size = 0;
+ result->fake_loclists_cu->version = 0;
+ result->fake_loclists_cu->unit_type = 0;
+ result->fake_loclists_cu->split = NULL;
}
}
@@ -272,7 +282,7 @@ valid_p (Dwarf *result)
inside the .debug_addr section, if it exists. */
if (result != NULL && result->sectiondata[IDX_debug_addr] != NULL)
{
- result->fake_addr_cu = (Dwarf_CU *) calloc (1, sizeof (Dwarf_CU));
+ result->fake_addr_cu = (Dwarf_CU *) malloc (sizeof (Dwarf_CU));
if (unlikely (result->fake_addr_cu == NULL))
{
Dwarf_Sig8_Hash_free (&result->sig8_hash);
@@ -291,6 +301,11 @@ valid_p (Dwarf *result)
result->fake_addr_cu->endp
= (result->sectiondata[IDX_debug_addr]->d_buf
+ result->sectiondata[IDX_debug_addr]->d_size);
+ result->fake_addr_cu->locs = NULL;
+ result->fake_addr_cu->address_size = 0;
+ result->fake_addr_cu->version = 0;
+ result->fake_addr_cu->unit_type = 0;
+ result->fake_addr_cu->split = NULL;
}
}
diff --git a/libdw/dwarf_end.c b/libdw/dwarf_end.c
index a2e94436..265239c4 100644
--- a/libdw/dwarf_end.c
+++ b/libdw/dwarf_end.c
@@ -52,8 +52,6 @@ cu_free (void *arg)
{
struct Dwarf_CU *p = (struct Dwarf_CU *) arg;
- Dwarf_Abbrev_Hash_free (&p->abbrev_hash);
-
tdestroy (p->locs, noop_free);
/* Free split dwarf one way (from skeleton to split). */
@@ -65,6 +63,13 @@ cu_free (void *arg)
p->split->dbg->fake_addr_cu = NULL;
INTUSE(dwarf_end) (p->split->dbg);
}
+
+ /* Only free the Abbrev_Hash if its not a fake CU. */
+ if(p != p->dbg->fake_loc_cu && p != p->dbg->fake_loclists_cu
+ && p != p->dbg->fake_addr_cu)
+ {
+ Dwarf_Abbrev_Hash_free (&p->abbrev_hash);
+ }
}
--
2.24.0.rc2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libdw: Don't free uninitialized Dwarf_Abbrev_Hash's of "fake" CUs.
2019-10-31 23:43 ` Jonathon Anderson
@ 2019-11-01 9:00 ` Mark Wielaard
2019-11-01 13:18 ` Jonathon Anderson
0 siblings, 1 reply; 6+ messages in thread
From: Mark Wielaard @ 2019-11-01 9:00 UTC (permalink / raw)
To: Jonathon Anderson; +Cc: elfutils-devel
On Thu, Oct 31, 2019 at 06:42:18PM -0500, Jonathon Anderson wrote:
> fake_{loc,loclists,addr}_cu are Dwarf_CUs that are created separate from
> all the others, so their contents are minimal and mostly initialized by
> a calloc. On dwarf_end however, they are freed through the same code path
> as all the others, so they call DAH_free like all the others. This changes
> that so that these three are exempt from DAH and split-DWARF matters, and
> swaps the calloc for a malloc so Memcheck will catch any others.
> ---
> > The Dwarf_Abbrev_Hash_free looks in the correct place. But I don't
> > believe the Free split dwarf hunk should not be under the same if
> > not-fake block.
>
> That can be fixed.
>
> I don't really like `unit_type = 0`, but its the only value that isn't one
> of the DW_UT values. And its what the calloc would've done.
O! Sorry. I now see your original approach was actually correct. For
some reason I believed the fake cus could have a split DWARF. But of
course they never have. The reason I gave was wrong, but I still like
this patch a little better because it more explicitly sets the needed
values. Your original approach however makes more sense if you like
valgrind memcheck to catch more issues.
Could you add a Signed-off-by and ChangeLog entry?
Either for your original patch or this one. Whichever you like best..
Thanks,
Mark
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] libdw: Don't free uninitialized Dwarf_Abbrev_Hash's of "fake" CUs.
2019-11-01 9:00 ` Mark Wielaard
@ 2019-11-01 13:18 ` Jonathon Anderson
2019-11-02 0:37 ` Mark Wielaard
0 siblings, 1 reply; 6+ messages in thread
From: Jonathon Anderson @ 2019-11-01 13:18 UTC (permalink / raw)
To: Mark Wielaard; +Cc: elfutils-devel, Jonathon Anderson
fake_{loc,loclists,addr}_cu are Dwarf_CUs that are created separate from
all the others, so their contents are minimal and mostly initialized by
a calloc. On dwarf_end however, they are freed through the same code path
as all the others, so they call DAH_free like all the others. This changes
that so that these three are exempt from DAH and split-DWARF matters, and
swaps the calloc for a malloc so Memcheck will catch any others.
Signed-off-by: Jonathon Anderson <jma14@rice.edu>
---
> O! Sorry. I now see your original approach was actually correct. For
> some reason I believed the fake cus could have a split DWARF. But of
> course they never have. The reason I gave was wrong, but I still like
> this patch a little better because it more explicitly sets the needed
> values.
How about a bit of both, I'll be happy if I don't initialize `unit_type`.
Does this work?
> Could you add a Signed-off-by and ChangeLog entry?
Sure thing.
libdw/ChangeLog | 6 ++++++
libdw/dwarf_begin_elf.c | 18 +++++++++++++++---
libdw/dwarf_end.c | 23 ++++++++++++++---------
3 files changed, 35 insertions(+), 12 deletions(-)
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 394c0df2..b1f73bc8 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,9 @@
+2019-11-01 Jonathon Anderson <jma14@rice.edu>
+
+ * dwarf_begin_elf.c (valid_p): Switch calloc for malloc for fake CUs.
+ Add explicit initialization of some fields.
+ * dwarf_end.c (cu_free): Add clause to limit freeing of CU internals.
+
2019-08-26 Jonathon Anderson <jma14@rice.edu>
* libdw_alloc.c (__libdw_allocate): Added thread-safe stack allocator.
diff --git a/libdw/dwarf_begin_elf.c b/libdw/dwarf_begin_elf.c
index 8d137414..8c116847 100644
--- a/libdw/dwarf_begin_elf.c
+++ b/libdw/dwarf_begin_elf.c
@@ -223,7 +223,7 @@ valid_p (Dwarf *result)
inside the .debug_loc or .debug_loclists section. */
if (result != NULL && result->sectiondata[IDX_debug_loc] != NULL)
{
- result->fake_loc_cu = (Dwarf_CU *) calloc (1, sizeof (Dwarf_CU));
+ result->fake_loc_cu = (Dwarf_CU *) malloc (sizeof (Dwarf_CU));
if (unlikely (result->fake_loc_cu == NULL))
{
Dwarf_Sig8_Hash_free (&result->sig8_hash);
@@ -240,12 +240,16 @@ valid_p (Dwarf *result)
result->fake_loc_cu->endp
= (result->sectiondata[IDX_debug_loc]->d_buf
+ result->sectiondata[IDX_debug_loc]->d_size);
+ result->fake_loc_cu->locs = NULL;
+ result->fake_loc_cu->address_size = 0;
+ result->fake_loc_cu->version = 0;
+ result->fake_loc_cu->split = NULL;
}
}
if (result != NULL && result->sectiondata[IDX_debug_loclists] != NULL)
{
- result->fake_loclists_cu = (Dwarf_CU *) calloc (1, sizeof (Dwarf_CU));
+ result->fake_loclists_cu = (Dwarf_CU *) malloc (sizeof (Dwarf_CU));
if (unlikely (result->fake_loclists_cu == NULL))
{
Dwarf_Sig8_Hash_free (&result->sig8_hash);
@@ -263,6 +267,10 @@ valid_p (Dwarf *result)
result->fake_loclists_cu->endp
= (result->sectiondata[IDX_debug_loclists]->d_buf
+ result->sectiondata[IDX_debug_loclists]->d_size);
+ result->fake_loclists_cu->locs = NULL;
+ result->fake_loclists_cu->address_size = 0;
+ result->fake_loclists_cu->version = 0;
+ result->fake_loclists_cu->split = NULL;
}
}
@@ -272,7 +280,7 @@ valid_p (Dwarf *result)
inside the .debug_addr section, if it exists. */
if (result != NULL && result->sectiondata[IDX_debug_addr] != NULL)
{
- result->fake_addr_cu = (Dwarf_CU *) calloc (1, sizeof (Dwarf_CU));
+ result->fake_addr_cu = (Dwarf_CU *) malloc (sizeof (Dwarf_CU));
if (unlikely (result->fake_addr_cu == NULL))
{
Dwarf_Sig8_Hash_free (&result->sig8_hash);
@@ -291,6 +299,10 @@ valid_p (Dwarf *result)
result->fake_addr_cu->endp
= (result->sectiondata[IDX_debug_addr]->d_buf
+ result->sectiondata[IDX_debug_addr]->d_size);
+ result->fake_addr_cu->locs = NULL;
+ result->fake_addr_cu->address_size = 0;
+ result->fake_addr_cu->version = 0;
+ result->fake_addr_cu->split = NULL;
}
}
diff --git a/libdw/dwarf_end.c b/libdw/dwarf_end.c
index a2e94436..7e194a55 100644
--- a/libdw/dwarf_end.c
+++ b/libdw/dwarf_end.c
@@ -52,18 +52,23 @@ cu_free (void *arg)
{
struct Dwarf_CU *p = (struct Dwarf_CU *) arg;
- Dwarf_Abbrev_Hash_free (&p->abbrev_hash);
-
tdestroy (p->locs, noop_free);
- /* Free split dwarf one way (from skeleton to split). */
- if (p->unit_type == DW_UT_skeleton
- && p->split != NULL && p->split != (void *)-1)
+ /* Only free the CU internals if its not a fake CU. */
+ if(p != p->dbg->fake_loc_cu && p != p->dbg->fake_loclists_cu
+ && p != p->dbg->fake_addr_cu)
{
- /* The fake_addr_cu might be shared, only release one. */
- if (p->dbg->fake_addr_cu == p->split->dbg->fake_addr_cu)
- p->split->dbg->fake_addr_cu = NULL;
- INTUSE(dwarf_end) (p->split->dbg);
+ Dwarf_Abbrev_Hash_free (&p->abbrev_hash);
+
+ /* Free split dwarf one way (from skeleton to split). */
+ if (p->unit_type == DW_UT_skeleton
+ && p->split != NULL && p->split != (void *)-1)
+ {
+ /* The fake_addr_cu might be shared, only release one. */
+ if (p->dbg->fake_addr_cu == p->split->dbg->fake_addr_cu)
+ p->split->dbg->fake_addr_cu = NULL;
+ INTUSE(dwarf_end) (p->split->dbg);
+ }
}
}
--
2.24.0.rc2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libdw: Don't free uninitialized Dwarf_Abbrev_Hash's of "fake" CUs.
2019-11-01 13:18 ` Jonathon Anderson
@ 2019-11-02 0:37 ` Mark Wielaard
0 siblings, 0 replies; 6+ messages in thread
From: Mark Wielaard @ 2019-11-02 0:37 UTC (permalink / raw)
To: Jonathon Anderson; +Cc: elfutils-devel
Hi Jonathon,
On Fri, 2019-11-01 at 08:14 -0500, Jonathon Anderson wrote:
> How about a bit of both, I'll be happy if I don't initialize
> `unit_type`.
> Does this work?
Looks perfect. Pushed to master.
Thanks,
Mark
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-11-02 0:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-31 2:54 [PATCH] libdw: Don't free uninitialized Dwarf_Abbrev_Hash's of "fake" CUs Jonathon Anderson
2019-10-31 23:00 ` Mark Wielaard
2019-10-31 23:43 ` Jonathon Anderson
2019-11-01 9:00 ` Mark Wielaard
2019-11-01 13:18 ` Jonathon Anderson
2019-11-02 0:37 ` 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).