Bug 29345 - abipkgdiff is confused by symlinked binaries in RPMs
authorDodji Seketeli <dodji@redhat.com>
Sun, 26 Mar 2023 22:10:21 +0000 (00:10 +0200)
committerDodji Seketeli <dodji@redhat.com>
Sun, 26 Mar 2023 23:15:56 +0000 (01:15 +0200)
Note that this issue can be reproduced by doing:

    $ fedabipkgdiff  --self-compare -a --from fc37 dx

When a binary is a symlink to another one,
create_maps_of_package_content doesn't realize it and considers the
two binaries to be different.

Later, when comes time to self compare both binaries, their abixml
files (which are the same, by virtue of symlinks) might be written at
the same time in different threads, creating a race condition, leading
to corruption of the abixml.

This patch fixes this by teaching create_maps_of_package_content to
resolve symlinks so that it can detect when two files actually point
to the same file.

* tools/abipkgdiff.cc (create_maps_of_package_content): Resolve
symlinks when mapping binaries.  Don't map a binary that has
already been seen.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
tools/abipkgdiff.cc

index 09adc837536374b98c991ed035557f166fc0fa09..2765284e1a752d3a739c781f94cb667e63dd2310 100644 (file)
@@ -2460,6 +2460,12 @@ create_maps_of_package_content(package& package, options& opts)
        ++file)
     {
       elf_file_sptr e (new elf_file(*file));
+      string resolved_e_path;
+      // The path 'e->path' might contain symlinks.  Let's resolve
+      // them so we can see if 'e->path' has already been seen before,
+      // for instance.
+      real_path(e->path, resolved_e_path);
+
       if (opts.compare_dso_only)
        {
          if (e->type != abigail::elf::ELF_TYPE_DSO)
@@ -2514,7 +2520,13 @@ create_maps_of_package_content(package& package, options& opts)
          // base name.  So let's consider the full path of the binary
          // inside the extracted directory.
          string key = e->name;
-         package.convert_path_to_unique_suffix(e->path, key);
+         package.convert_path_to_unique_suffix(resolved_e_path, key);
+         if (package.path_elf_file_sptr_map().find(key)
+             != package.path_elf_file_sptr_map().end())
+           // 'key' has already been seen before.  So we won't map it
+           // twice.
+           continue;
+
          package.path_elf_file_sptr_map()[key] = e;
          if (opts.verbose)
            emit_prefix("abipkgdiff", cerr)
@@ -2544,11 +2556,18 @@ create_maps_of_package_content(package& package, options& opts)
                }
            }
 
-         if (package.convert_path_to_unique_suffix(e->path, key))
+         if (package.convert_path_to_unique_suffix(resolved_e_path, key))
            {
              dir_name(key, key);
              key += string("/@soname:") + e->soname;
            }
+
+         if (package.path_elf_file_sptr_map().find(key)
+             != package.path_elf_file_sptr_map().end())
+           // 'key' has already been seen before.  So we won't do itl
+           // twice.
+           continue;
+
          package.path_elf_file_sptr_map()[key] = e;
          if (opts.verbose)
            emit_prefix("abipkgdiff", cerr)