[applied] Bug 30329 - Fix regression in support for absolute path to alt DWARF

Message ID 87v7p4orp2.fsf@redhat.com
State New
Headers
Series [applied] Bug 30329 - Fix regression in support for absolute path to alt DWARF |

Commit Message

Dodji Seketeli June 10, 2025, 9:45 a.m. UTC
  Hello,

The previous fix for this bug was the commit
https://sourceware.org/git/?p=libabigail.git;a=commit;h=6ba26ed6ae74cf54c87cde1df163e5f086bbcfce.

Support for that fix was later "improved" by this recent commit:

    commit f17f76e2f94e50fb03e832a6193f7dfe0bed890a
    Author: Dodji Seketeli <dodji@redhat.com>
    Date:   Fri Jun 6 11:08:41 2025 +0200

	elf-reader: Fix elfutils initialization of debuginfo lookup paths

	The ELF reader was doing too much work unnecessary (and flat out
	wrong) work to setup the paths to where debug info should be looked
	for.  This prevents clients like abipkgdiff to properly rely on the
	transparent handling of debuginfod by elfutils when possible.

	This patch fixes the logic of that initialization by following the
	comments of dwfl_build_id_find_elf in /usr/include/elfutils/libdwfl.h.

		* src/abg-elf-reader.cc (find_alt_dwarf_debug_info_path)
		(find_alt_dwarf_debug_info): Remove static functions.
		(reader::priv::find_alt_dwarf_debug_info): Remove member function.h
		(reader::priv::{formated_di_root_paths,
		raw_formated_di_root_paths}): Add new data members.
		(reader::priv::initialize): Initialize the new data members.
		(reader::priv::initialize_debug_info_root_paths): Define new
		member function.
		(reader::priv::crack_open_elf_file): Invoke the new
		initialize_debug_info_root_paths above.  Adjust the call to
		elf_helpers::initialize_dwfl_callbacks to pass the new
		raw_formated_di_root_paths.
		(reader::priv::locate_dwarf_debug_info): Remove all the
		unnecessary cruft.  Just rely on elfutils finding the debug info,
		now that it's been properly initialized.  Use the dwarf_getalt
		function that is now always present in the versions of elfutils
		that we use.

That later fix actually temporarily broke the initial fix for "Bug
30329".

This patch handles the case of a DWZ-produced alternate DWARF being
referenced by an absolute path which happens to be under the root
debug info dir (e.g,
"/usr/lib/debug/dwz/components/sqlite.bst/x86_64-unknown-linux-gnu",
under the root dir "/usr/lib/debug").

The idea of the fix is to automatically add a
"/usr/lib/debug/dwz/components/sqlite.bst" as a debuginfo dir because
it is under the root dir /usr/lib/debug and because it contains a
file.

	* include/abg-tools-utils.h (get_file_path_dirs_under_dir):
	Declare ...
	* src/abg-tools-utils.cc (get_file_path_dirs_under_dir): ... new
	function.
	(is_file): Define new static function.
	* src/abg-elf-reader.cc
	(reader::priv::initialize_debug_info_root_paths): Add
	sub-directories (that contain files) of root debuginfo dirs to the
	set of debug info root paths to be considered by elfutils.
	* tests/test-abidiff-exit.cc (in_out_specs): Add a new test case
	for
	data/test-abidiff-exit/PR30329/old-image/usr/lib/x86_64-linux-gnu/libsqlite3.so.0.8.6
	where two debuginfo root dirs are added.
	(main): Support adding several comma-separated debuginfo root
	dirs to InOutSpec::in_elfv[01]_debug_dir.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Applied to the mainline.
---
 include/abg-tools-utils.h  |  3 ++
 src/abg-elf-reader.cc      | 18 ++++++++++++
 src/abg-tools-utils.cc     | 58 ++++++++++++++++++++++++++++++++++++++
 tests/test-abidiff-exit.cc | 40 +++++++++++++++++++++-----
 4 files changed, 112 insertions(+), 7 deletions(-)
  

Patch

diff --git a/include/abg-tools-utils.h b/include/abg-tools-utils.h
index 1762de96..f12c1907 100644
--- a/include/abg-tools-utils.h
+++ b/include/abg-tools-utils.h
@@ -99,6 +99,9 @@  suppr::suppressions_type
 gen_suppr_spec_from_kernel_abi_whitelists
    (const vector<string>& abi_whitelist_paths);
 
+bool
+get_file_path_dirs_under_dir(const string& root_dir, vector<string>& dirs);
+
 bool
 get_vmlinux_path_from_kernel_dist(const string&	from,
 				  string&		vmlinux_path);
diff --git a/src/abg-elf-reader.cc b/src/abg-elf-reader.cc
index 13d497ff..8642f701 100644
--- a/src/abg-elf-reader.cc
+++ b/src/abg-elf-reader.cc
@@ -203,7 +203,25 @@  struct reader::priv
   void
   initialize_debug_info_root_paths()
   {
+    vector<string> root_paths = debug_info_root_paths;
+
     for (auto path : debug_info_root_paths)
+      if (tools_utils::string_begins_with(path, "/"))
+	{
+	  // For absolute root directories, let's add all the
+	  // sub-directories of these directories that contain a
+	  // (debug info) file.  This can be helpful to help elfutils
+	  // find alternate debuginfo files when the altdebuginfolink
+	  // is itself an absolute file path that is contained under
+	  // the root directory.  This what we see in PR30329 and its
+	  // associated regression test in test-abidiff-exit.cc
+	  vector<string> additional_subdirs;
+	  tools_utils::get_file_path_dirs_under_dir(path, additional_subdirs);
+	  for (auto& subdir : additional_subdirs)
+	    root_paths.push_back(subdir);
+	}
+
+    for (auto path : root_paths)
       {
 	if (formated_di_root_paths.empty())
 	  formated_di_root_paths = "-";
diff --git a/src/abg-tools-utils.cc b/src/abg-tools-utils.cc
index b296f8a0..68585998 100644
--- a/src/abg-tools-utils.cc
+++ b/src/abg-tools-utils.cc
@@ -2979,6 +2979,20 @@  is_kernel_module(const FTSENT *entry)
   return false;
 }
 
+/// Test if a given file denoted by a FTSENT* is a regular file or a
+/// symlink.
+///
+/// @param entry returned by the combo fts_open/fts_read.
+///
+/// @return true iff @p entry is for a regular file or a symlink.
+static bool
+is_file(const FTSENT *entry)
+{
+  return (entry
+	  && (entry->fts_info == FTS_F
+	      || entry->fts_info == FTS_SL));
+}
+
 /// Find a vmlinux and its kernel modules in a given directory tree.
 ///
 /// @param from the directory tree to start looking from.
@@ -3068,6 +3082,50 @@  find_vmlinux_path(const string&	from,
   return found_vmlinux;
 }
 
+/// Get all the sub-directories (which contain a regular file) of a
+/// given directory.
+///
+/// @param root_dir the root directory to consider.
+///
+/// @param dirs the sub-directories of @p root_dir which contain a
+/// file.
+bool
+get_file_path_dirs_under_dir(const string& root_dir, vector<string>& dirs)
+{
+  char* paths[] = {const_cast<char*>(root_dir.c_str()), 0};
+  FTS *file_hierarchy = fts_open(paths,
+				 FTS_PHYSICAL|FTS_NOCHDIR|FTS_XDEV, 0);
+  if (!file_hierarchy)
+    return false;
+
+  string r = root_dir;
+  if (!string_ends_with(r, "/"))
+    r += "/";
+
+  bool found_file = false;
+  FTSENT *entry;
+  while ((entry = fts_read(file_hierarchy)))
+    {
+      // Skip descendents of symbolic links.
+      if (entry->fts_info == FTS_SL || entry->fts_info == FTS_SLNONE)
+	{
+	  fts_set(file_hierarchy, entry, FTS_SKIP);
+	  continue;
+	}
+
+      if (is_file(entry))
+	found_file = true;
+
+      string path = entry->fts_path;
+      dir_name(path, path);
+      dirs.push_back(path);
+    }
+
+  fts_close(file_hierarchy);
+
+  return found_file;
+}
+
 /// Get the paths of the vmlinux and kernel module binaries under
 /// given directory.
 ///
diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
index 584ced34..f3cf4107 100644
--- a/tests/test-abidiff-exit.cc
+++ b/tests/test-abidiff-exit.cc
@@ -997,6 +997,21 @@  InOutSpec in_out_specs[] =
     "data/test-abidiff-exit/PR30329/PR30329-report-1.txt",
     "output/test-abidiff-exit/PR30329/PR30329-report-1.txt"
   },
+  {
+    "data/test-abidiff-exit/PR30329/old-image/usr/lib/x86_64-linux-gnu/libsqlite3.so.0.8.6",
+    "data/test-abidiff-exit/PR30329/new-image/usr/lib/x86_64-linux-gnu/libsqlite3.so.0.8.6",
+    "",
+    "",
+    "",
+    "data/test-abidiff-exit/PR30329/old-image/usr/lib/debug,data/test-abidiff-exit/PR30329/old-image/usr/lib/debug/dwz/components/sqlite.bst",
+    "data/test-abidiff-exit/PR30329/new-image/usr/lib/debug,data/test-abidiff-exit/PR30329/old-image/usr/lib/debug/dwz/components/sqlite.bst",
+    "",
+    "",
+    "--no-default-suppression",
+    abigail::tools_utils::ABIDIFF_ABI_CHANGE,
+    "data/test-abidiff-exit/PR30329/PR30329-report-1.txt",
+    "output/test-abidiff-exit/PR30329/PR30329-report-1.txt"
+  },
   {
     "data/test-abidiff-exit/PR30503/libsdl/1.2.60/lib64/libSDL-1.2.so.1.2.60",
     "data/test-abidiff-exit/PR30503/libsdl/1.2.64/lib64/libSDL-1.2.so.1.2.64",
@@ -1692,7 +1707,8 @@  main()
     in_suppression_path, abidiff_options, abidiff, cmd, diff_cmd,
     ref_diff_report_path, out_diff_report_path, in_elfv0_debug_dir,
     in_elfv1_debug_dir, in_elfv0_added_bins_dir, in_elfv1_added_bins_dir;
-  vector<string> in_elfv0_headers_dirs, in_elfv1_headers_dirs;
+  vector<string> in_elfv0_headers_dirs, in_elfv1_headers_dirs,
+    in_elfv0_debug_dirs, in_elfv1_debug_dirs;
   string source_dir_prefix = string(get_src_dir()) + "/tests/";
   string build_dir_prefix = string(get_build_dir()) + "/tests/";
 
@@ -1704,9 +1720,19 @@  main()
 	in_elfv0_debug_dir.clear();
 	in_elfv1_debug_dir.clear();
 	if (s->in_elfv0_debug_dir && strcmp(s->in_elfv0_debug_dir, ""))
-	  in_elfv0_debug_dir = source_dir_prefix + s->in_elfv0_debug_dir;
+	  {
+	    if (!split_string(s->in_elfv0_debug_dir, ",", in_elfv0_debug_dirs))
+	      in_elfv0_debug_dirs.push_back(s->in_elfv0_debug_dir);
+	    for (auto& dd : in_elfv0_debug_dirs)
+	      dd = source_dir_prefix + dd;
+	  }
 	if (s->in_elfv1_debug_dir && strcmp(s->in_elfv1_debug_dir, ""))
-	  in_elfv1_debug_dir = source_dir_prefix + s->in_elfv1_debug_dir;
+	  {
+	    if (!split_string(s->in_elfv1_debug_dir, ",", in_elfv1_debug_dirs))
+	      in_elfv1_debug_dirs.push_back(s->in_elfv1_debug_dir);
+	    for (auto& dd : in_elfv1_debug_dirs)
+	      dd = source_dir_prefix + dd;
+	  }
 	in_elfv0_headers_dirs.clear();
 	in_elfv1_headers_dirs.clear();
 	in_elfv0_added_bins_dir.clear();
@@ -1762,11 +1788,11 @@  main()
 	if (!in_elfv1_added_bins_dir.empty())
 	  abidiff += " --added-binaries-dir2 " + in_elfv1_added_bins_dir;
 
-	if (!in_elfv0_debug_dir.empty())
-	  abidiff += " --debug-info-dir1 " + in_elfv0_debug_dir;
+	for (auto& dd : in_elfv0_debug_dirs)
+	  abidiff += " --debug-info-dir1 " + dd;
 
-	if (!in_elfv1_debug_dir.empty())
-	  abidiff += " --debug-info-dir2 " + in_elfv1_debug_dir;
+	for (auto& dd : in_elfv1_debug_dirs)
+	  abidiff += " --debug-info-dir2 " + dd;
 
 	if (!in_elfv0_headers_dirs.empty())
 	  for (vector<string>::const_iterator s = in_elfv0_headers_dirs.begin();