Bug 21644 - abipkgdiff does not emit diagnostics about comparison errors

When abipkgdiff compares binaries of a package, it doesn't report
errors like debug info or symbol not found.

This patch fixes that.

	* include/abg-dwarf-reader.h (status_to_diagnostic_string):
	Declare new function.
	* src/abg-dwarf-reader.cc (status_to_diagnostic_string): Define
	new function.
	* tools/abipkgdiff.cc (compare): Take a new detailed_error_status
	parameter.
	(compare_task::perform): Get the details of the error, in case the
	status of the comparison is ABIDIFF_ERROR.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>

diff --git a/include/abg-dwarf-reader.h b/include/abg-dwarf-reader.h
index 14b82e569c38024d75b637b029e95e4ea5729a8f..8e261408f96b369f72542a8d42b96792573d7368 100644 (file)
--- a/include/abg-dwarf-reader.h
+++ b/include/abg-dwarf-reader.h
@@ -60,6 +60,9 @@ enum status
   STATUS_NO_SYMBOLS_FOUND = 1 << 2,
 };
 
+string
+status_to_diagnostic_string(status s);
+
 status
 operator|(status, status);
 

diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index d6f3736f74eff2cf56fa366896ada7df4d05b258..337fbb8ef690503a49259912c0b29e2a382048bd 100644 (file)
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -15475,6 +15475,29 @@ operator&=(status& l, status r)
   return l;
 }
 
+/// Emit a diagnostic status with english sentences to describe the
+/// problems encoded in a given abigail::dwarf_reader::status, if
+/// there is an error.
+///
+/// @param status the status to diagnose
+///
+/// @return a string containing sentences that describe the possible
+/// errors encoded in @p s.  If there is no error to encode, then the
+/// empty string is returned.
+string
+status_to_diagnostic_string(status s)
+{
+  string str;
+
+  if (s & STATUS_DEBUG_INFO_NOT_FOUND)
+    str += "could not find debug info\n";
+
+  if (s & STATUS_NO_SYMBOLS_FOUND)
+    str += "could not load ELF symbols\n";
+
+  return str;
+}
+
 /// Create a dwarf_reader::read_context.
 ///
 /// @param elf_path the path to the elf file the context is to be used for.

diff --git a/tools/abipkgdiff.cc b/tools/abipkgdiff.cc
index 2e5c17cf27440dfe437374dcddee71aad424805a..a82fafa1b404a18f92ca77ad0fd97d71e0a87f75 100644 (file)
--- a/tools/abipkgdiff.cc
+++ b/tools/abipkgdiff.cc
@@ -1017,6 +1017,11 @@ set_diff_context_from_opts(diff_context_sptr ctxt,
 ///
 /// @param diff the shared pointer to be set to the result of the comparison.
 ///
+/// @param detailed_error_status is this pointer is non-null and if
+/// the function returns ABIDIFF_ERROR, then the function sets the
+/// pointed-to parameter to the abigail::dwarf_reader::status value
+/// that gives details about the rror.
+///
 /// @return the status of the comparison.
 static abidiff_status
 compare(const elf_file& elf1,
@@ -1028,7 +1033,8 @@ compare(const elf_file& elf1,
        const options&  opts,
        abigail::ir::environment_sptr   &env,
        corpus_diff_sptr        &diff,
-       diff_context_sptr       &ctxt)
+       diff_context_sptr       &ctxt,
+       abigail::dwarf_reader::status *detailed_error_status = 0)
 {
   char *di_dir1 = (char*) debug_dir1.c_str(),
        *di_dir2 = (char*) debug_dir2.c_str();
@@ -1098,6 +1104,10 @@ compare(const elf_file& elf1,
            << "Could not read file '"
            << elf1.path
            << "' properly\n";
+
+      if (detailed_error_status)
+       *detailed_error_status = c1_status;
+
        return abigail::tools_utils::ABIDIFF_ERROR;
       }
   }
@@ -1111,6 +1121,9 @@ compare(const elf_file& elf1,
       else
        emit_prefix("abipkgdiff", cerr) << "\n";
 
+      if (detailed_error_status)
+       *detailed_error_status = c1_status;
+
       return abigail::tools_utils::ABIDIFF_ERROR;
     }
 
@@ -1144,6 +1157,10 @@ compare(const elf_file& elf1,
            << "Could not find the read file '"
            << elf2.path
            << "' properly\n";
+
+       if (detailed_error_status)
+         *detailed_error_status = c2_status;
+
        return abigail::tools_utils::ABIDIFF_ERROR;
       }
   }
@@ -1159,6 +1176,9 @@ compare(const elf_file& elf1,
       else
        emit_prefix("abipkgdiff", cerr) << "\n";
 
+      if (detailed_error_status)
+       *detailed_error_status = c2_status;
+
       return abigail::tools_utils::ABIDIFF_ERROR;
     }
 
@@ -1459,29 +1479,51 @@ public:
     diff_context_sptr ctxt;
     corpus_diff_sptr diff;
 
+    abigail::dwarf_reader::status detailed_status =
+      abigail::dwarf_reader::STATUS_UNKNOWN;
+
     status |= compare(args->elf1, args->debug_dir1, args->private_types_suppr1,
                      args->elf2, args->debug_dir2, args->private_types_suppr2,
-                     args->opts, env, diff, ctxt);
+                     args->opts, env, diff, ctxt, &detailed_status);
 
+    // If there is an ABI change, tell the user about it.
     if ((status & abigail::tools_utils::ABIDIFF_ABI_CHANGE)
-       || (args->opts.verbose && diff->has_changes()))
+       ||( diff && diff->has_net_changes()))
+      {
        diff->report(out, /*prefix=*/"  ");
+       string name = args->elf1.name;
+
+       pretty_output +=
+         string("================ changes of '") + name + "'===============\n"
+         + out.str()
+         + "================ end of changes of '"
+         + name + "'===============\n\n";
+      }
     else
       {
        if (args->opts.show_identical_binaries)
          out << "No ABI change detected\n";
       }
 
-    if (status != abigail::tools_utils::ABIDIFF_OK)
+    // If an error happened while comparing the twobinaries, tell the
+    // user about it.
+    if (status & abigail::tools_utils::ABIDIFF_ERROR)
       {
-       string name = args->elf1.name;
+       string diagnostic =
+         abigail::dwarf_reader::status_to_diagnostic_string(detailed_status);
 
-       pretty_output =
-         string("================ changes of '") + name + "'===============\n"
-         + out.str()
+       if (!diagnostic.empty())
+         {
+           string name = args->elf1.name;
 
-         + "================ end of changes of '"
-         + name + "'===============\n\n";
+           pretty_output +=
+             "==== Error happened during processing of " + name + ": ====\n";
+
+           pretty_output += diagnostic;
+
+           pretty_output +=
+             "==== End of error for " + name + " ====\n";
+         }
       }
   }
 }; // end class compare_task