* Re: [PATCH] Fix de-initialization of elf::reader::priv
2022-12-18 19:33 [PATCH] Fix de-initialization of elf::reader::priv Petr Pavlu
@ 2022-12-19 17:45 ` Dodji Seketeli
0 siblings, 0 replies; 2+ messages in thread
From: Dodji Seketeli @ 2022-12-19 17:45 UTC (permalink / raw)
To: Petr Pavlu via Libabigail; +Cc: Petr Pavlu
Hello Petr,
Petr Pavlu via Libabigail <libabigail@sourceware.org> a écrit:
> Add a destructor for elf::reader::priv which releases any allocated alt
> DWARF data and fix the initialize() method to fully reset the object.
>
> The latter fixes a crash observed when handling multiple files. For
> instance, when reading the Linux kernel tree, load_vmlinux_corpus()
> processes vmlinux and all modules. Member dwarf_handle was never reset
> after setting it for the first file which could later result in use of
> invalid DWARF data in dwarf::reader::build_die_parent_maps().
>
> * src/abg-elf-reader.cc (priv::~priv): Release alt debug
> information.
> (priv::initialize): Reset all members.
> (priv::clear_alt_dwarf_debug_info_data): New helper function.
>
> Fixes: 7bd69830 ("Make Front Ends first class citizens")
Thanks for the patch!
I have just edited slightly to fix what I think is a related dormant
issue in find_alt_dwarf_debug_info, which is that if the macro
LIBDW_HAS_DWARF_GETALT is not defined, then find_alt_dwarf_debug_info
doesn't return the proper file descriptor for the alternate DWARF debug
info file. With this additional fix, clear_alt_dwarf_debug_info_data()
will find (and clear) the expected alternate DWARF debuginfo file
descriptor.
The diff of my change is:
diff --git a/src/abg-elf-reader.cc b/src/abg-elf-reader.cc
index 4814a70e..c07f0655 100644
--- a/src/abg-elf-reader.cc
+++ b/src/abg-elf-reader.cc
@@ -225,10 +225,10 @@ find_alt_dwarf_debug_info(Dwfl_Module *elf_module,
// If we reach this point it means we have found the path to the
// alternate debuginfo file and it's in alt_file_path. So let's
// open it and read it.
- int fd = open(alt_file_path.c_str(), O_RDONLY);
- if (fd == -1)
+ alt_fd = open(alt_file_path.c_str(), O_RDONLY);
+ if (alt_fd == -1)
return result;
- result = dwarf_begin(fd, DWARF_C_READ);
+ result = dwarf_begin(alt_fd, DWARF_C_READ);
#ifdef LIBDW_HAS_DWARF_GETALT
Dwarf_Addr bias = 0;
Please find below the patch that I am applying to the master branch.
Thanks!
Cheers,
From 241c7fc7c1fb3b258815c800311f6d83869de87b Mon Sep 17 00:00:00 2001
From: Petr Pavlu <petr.pavlu@suse.com>
Date: Sun, 18 Dec 2022 20:33:40 +0100
Subject: [PATCH] Fix de-initialization of elf::reader::priv
This fixes 7bd69830 ("Make Front Ends first class citizens").
Add a destructor for elf::reader::priv which releases any allocated alt
DWARF data and fix the initialize() method to fully reset the object.
The latter fixes a crash observed when handling multiple files. For
instance, when reading the Linux kernel tree, load_vmlinux_corpus()
processes vmlinux and all modules. Member dwarf_handle was never reset
after setting it for the first file which could later result in use of
invalid DWARF data in dwarf::reader::build_die_parent_maps().
* src/abg-elf-reader.cc (priv::~priv): Release alt debug
information.
(priv::initialize): Reset all members.
(priv::clear_alt_dwarf_debug_info_data): New helper function.
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
---
src/abg-elf-reader.cc | 44 ++++++++++++++++++++++++++++++++++++++-----
1 file changed, 39 insertions(+), 5 deletions(-)
diff --git a/src/abg-elf-reader.cc b/src/abg-elf-reader.cc
index 3b1b5803..c07f0655 100644
--- a/src/abg-elf-reader.cc
+++ b/src/abg-elf-reader.cc
@@ -225,10 +225,10 @@ find_alt_dwarf_debug_info(Dwfl_Module *elf_module,
// If we reach this point it means we have found the path to the
// alternate debuginfo file and it's in alt_file_path. So let's
// open it and read it.
- int fd = open(alt_file_path.c_str(), O_RDONLY);
- if (fd == -1)
+ alt_fd = open(alt_file_path.c_str(), O_RDONLY);
+ if (alt_fd == -1)
return result;
- result = dwarf_begin(fd, DWARF_C_READ);
+ result = dwarf_begin(alt_fd, DWARF_C_READ);
#ifdef LIBDW_HAS_DWARF_GETALT
Dwarf_Addr bias = 0;
@@ -281,6 +281,11 @@ struct reader::priv
initialize(debug_info_roots);
}
+ ~priv()
+ {
+ clear_alt_dwarf_debug_info_data();
+ }
+
/// Reset the private data of @elf elf::reader.
///
/// @param debug_info_roots the vector of new directories where to
@@ -288,11 +293,23 @@ struct reader::priv
void
initialize(const vector<char**>& debug_info_roots)
{
- debug_info_root_paths = debug_info_roots;
+ clear_alt_dwarf_debug_info_data();
+
+ elf_handle = nullptr;
+ symtab_section = nullptr;
+ elf_architecture.clear();
+ dt_needed.clear();
symt.reset();
+ debug_info_root_paths = debug_info_roots;
+ offline_callbacks = {};
dwfl_handle.reset();
elf_module = nullptr;
- elf_handle = nullptr;
+ dwarf_handle = nullptr;
+ alt_dwarf_handle = nullptr;
+ alt_dwarf_path.clear();
+ alt_dwarf_fd = 0;
+ ctf_section = nullptr;
+ alt_ctf_section = nullptr;
}
/// Setup the necessary plumbing to open the ELF file and find all
@@ -348,6 +365,23 @@ struct reader::priv
return result;
}
+ /// Clear the resources related to the alternate DWARF data.
+ void
+ clear_alt_dwarf_debug_info_data()
+ {
+ if (alt_dwarf_fd)
+ {
+ if (alt_dwarf_handle)
+ {
+ dwarf_end(alt_dwarf_handle);
+ alt_dwarf_handle = nullptr;
+ }
+ close(alt_dwarf_fd);
+ alt_dwarf_fd = 0;
+ }
+ alt_dwarf_path.clear();
+ }
+
/// Locate the DWARF debug info in the ELF file.
///
/// This also knows how to locate split debug info.
--
2.31.1
--
Dodji
^ permalink raw reply [flat|nested] 2+ messages in thread