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>

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);
 

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.

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