[applied] Add --enable-debug-type-canonicalization to configure

Message ID 87ee8id8xs.fsf@redhat.com
State New
Headers
Series [applied] Add --enable-debug-type-canonicalization to configure |

Commit Message

Dodji Seketeli Oct. 18, 2021, 8:38 a.m. UTC
  Hello,

This configure option adds the possibility to debug the type
canonicalization process specifically.

When this new configure option is turned on, in
ir::get_canonical_type_for, when the type T, candidate for
canonicalization is compared to a given canonical type C, the
comparison is done twice; once using structural equality and once
using canonical equality whenever it's possible.  For all the
sub-types of T and C, structural equality and canonical equality must
yield the same result.  Otherwise, an error message is emitted and the
process aborts.

This all happens when using the abidw program with the --enable-tc or
--enable-type-canonicalization option.

This has proven to be very helpful to detect type canonicalization issues.

For instance, here is a trace of canonicalization issue that was
detected thanks to this patch:

    $ build/tools/abidw --debug-tc /usr/lib64/libwiretap.so.11.0.8
    structural & canonical equality different for type: function type void (wtap*)
    in compare_types_during_canonicalization at: /home/dodji/git/libabigail/PR28364/src/abg-ir.cc:13575: execution should not have reached this point!
    Abandon (core dumped)

This means that right after canonicalizing the type "void (wtap*)",
structural and canonical equality yield different results.  So it
means there is a problem with that type specifically that makes its
canonicalization "go wrong".  This requires further debugging to
understand, but at least, we are super close to the root cause of the
canonicalization problem.

	* configure.ac: Support the new
	--enable-debug-type-canonicalization option.  Define macro
	WITH_DEBUG_TYPE_CANONICALIZATION accordingly.
	* doc/manuals/abidw.rst: Update documentation.
	* include/abg-ir.h
	(environment::debug_type_canonicalization_is_on): Declare new
	member function if WITH_DEBUG_TYPE_CANONICALIZATION is defined.
	* src/abg-ir-priv.h
	(environment::priv::{use_canonical_type_comparison_,
	debug_type_canonicalization_}): Define new data members if
	WITH_DEBUG_TYPE_CANONICALIZATION is defined.
	(environment::priv::priv): Initialize them.
	* src/abg-ir.cc (try_canonical_compare): When
	WITH_DEBUG_TYPE_CANONICALIZATION is defined, perform comparison
	using either structural or canonical equality depending on the
	environment::priv::use_canonical_type_comparison_ flag.
	(environment::debug_type_canonicalization_is_on): Define member
	function when WITH_DEBUG_TYPE_CANONICALIZATION is defined.
	(compare_types_during_canonicalization): Define new function.
	(type_base::get_canonical_type_for): Use the new function
	compare_types_during_canonicalization.
	* tools/abidw.cc (options::debug_type_canonicalization): Define
	new data member.
	(option::option): Initialize it.
	(display_usage): Add help string for --debug-tc.
	(parse_command_line): Support new option --debug-tc or
	--debug-type-canonicalization.
	(load_corpus_and_write_abixml): Turn type canonicalization
	debugging on if --enable-tc is provided.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Applied to master.
---
 configure.ac          |  19 ++++++++
 doc/manuals/abidw.rst |  13 +++++
 include/abg-ir.h      |   8 +++
 src/abg-ir-priv.h     |  20 ++++++++
 src/abg-ir.cc         | 110 +++++++++++++++++++++++++++++++++++++++++-
 tools/abidw.cc        |  19 ++++++++
 6 files changed, 187 insertions(+), 2 deletions(-)
  

Patch

diff --git a/configure.ac b/configure.ac
index 9e91f496..950e2704 100644
--- a/configure.ac
+++ b/configure.ac
@@ -72,6 +72,12 @@  AC_ARG_ENABLE(debug-self-comparison,
 	      ENABLE_DEBUG_SELF_COMPARISON=$enableval,
 	      ENABLE_DEBUG_SELF_COMPARISON=no)
 
+AC_ARG_ENABLE(debug-type-canonicalization,
+	      AS_HELP_STRING([--enable-debug-type-canonicalization=yes|no],
+			     [enable debugging of type canonicalization 'abidw --debug-tc'(default is no)]),
+	      ENABLE_DEBUG_TYPE_CANONICALIZATION=$enableval,
+	      ENABLE_DEBUG_TYPE_CANONICALIZATION=no)
+
 AC_ARG_ENABLE(deb,
 	      AS_HELP_STRING([--enable-deb=yes|no|auto],
 			     [enable the support of deb in abipkgdiff (default is auto)]),
@@ -313,6 +319,18 @@  fi
 
 AM_CONDITIONAL(ENABLE_DEBUG_SELF_COMPARISON, test x$ENABLE_DEBUG_SELF_COMPARISON = xyes)
 
+dnl enable the debugging of type canonicalization when doing abidw --debug-tc <binary>
+if test x$ENABLE_DEBUG_TYPE_CANONICALIZATION = xyes; then
+   AC_DEFINE([WITH_DEBUG_TYPE_CANONICALIZATION],
+	     1,
+	     [compile support of debugging type canonicalization while using abidw --debug-tc])
+   AC_MSG_NOTICE([support of debugging type canonicalization is enabled])
+else
+   AC_MSG_NOTICE([support of debugging type canonicalization is disabled])
+fi
+
+AM_CONDITIONAL(ENABLE_DEBUG_TYPE_CANONICALIZATION, test x$ENABLE_DEBUG_TYPE_CANONICALIZATION = xyes)
+
 dnl Check for the dpkg program
 if test x$ENABLE_DEB = xauto -o x$ENABLE_DEB = xyes; then
    AC_CHECK_PROG(HAS_DPKG, dpkg, yes, no)
@@ -931,6 +949,7 @@  AC_MSG_NOTICE([
     Enable rpm support in abipkgdiff               : ${ENABLE_RPM}
     Enable rpm 4.15 support in abipkgdiff tests    : ${ENABLE_RPM415}
     Enable self comparison debugging               : ${ENABLE_DEBUG_SELF_COMPARISON}
+    Enable type canonicalization debugging         : ${ENABLE_DEBUG_TYPE_CANONICALIZATION}
     Enable deb support in abipkgdiff               : ${ENABLE_DEB}
     Enable GNU tar archive support in abipkgdiff   : ${ENABLE_TAR}
     Enable bash completion	                   : ${ENABLE_BASH_COMPLETION}
diff --git a/doc/manuals/abidw.rst b/doc/manuals/abidw.rst
index 4b110d6b..d7545899 100644
--- a/doc/manuals/abidw.rst
+++ b/doc/manuals/abidw.rst
@@ -250,6 +250,19 @@  Options
     it the libabigail package needs to be configured with
     the --enable-debug-self-comparison option.
 
+    *  ``--debug-type-canonicalization | --debug-tc``
+
+    Debug the type canonicalization process.  This is done by using
+    structural and canonical equality when canonicalizing every single
+    type.  Structural and canonical equality should yield the same
+    result.  If they don't yield the same result for a given type,
+    then it means that the canonicalization of that type went wrong.
+    In that case, an error message is emitted and the execution of the
+    program is aborted.
+
+    This option is available only if the package was configured with
+    the --enable-debug-type-canonicalization option.
+
 
   *  ``--annotate``
 
diff --git a/include/abg-ir.h b/include/abg-ir.h
index 81147144..a651d1b7 100644
--- a/include/abg-ir.h
+++ b/include/abg-ir.h
@@ -217,6 +217,14 @@  public:
   self_comparison_debug_is_on() const;
 #endif
 
+#ifdef WITH_DEBUG_TYPE_CANONICALIZATION
+  void
+  debug_type_canonicalization_is_on(bool flag);
+
+  bool
+  debug_type_canonicalization_is_on() const;
+#endif
+
   vector<type_base_sptr>* get_canonical_types(const char* name);
 
   type_base* get_canonical_type(const char* name, unsigned index);
diff --git a/src/abg-ir-priv.h b/src/abg-ir-priv.h
index 18623ceb..a01a1b2c 100644
--- a/src/abg-ir-priv.h
+++ b/src/abg-ir-priv.h
@@ -389,6 +389,21 @@  struct environment::priv
 #ifdef WITH_DEBUG_SELF_COMPARISON
   bool					self_comparison_debug_on_;
 #endif
+#ifdef WITH_DEBUG_TYPE_CANONICALIZATION
+  // This controls whether to use canonical type comparison during
+  // type comparison or not.  This is only used for debugging, when we
+  // want to ensure that comparing types using canonical or structural
+  // comparison yields the same result.
+  bool					use_canonical_type_comparison_;
+  // Whether we are debugging type canonicalization or not.  When
+  // debugging type canonicalization, a type is compared to its
+  // potential canonical type twice: The first time with canonical
+  // comparison activated, and the second time with structural
+  // comparison activated.  The two comparison should yield the same
+  // result, otherwise, canonicalization is "broken" for that
+  // particular type.
+  bool					debug_type_canonicalization_;
+#endif
 
   priv()
     : canonicalization_is_done_(),
@@ -398,6 +413,11 @@  struct environment::priv
 #ifdef WITH_DEBUG_SELF_COMPARISON
     ,
       self_comparison_debug_on_(false)
+#endif
+#ifdef WITH_DEBUG_TYPE_CANONICALIZATION
+    ,
+      use_canonical_type_comparison_(true),
+      debug_type_canonicalization_(false)
 #endif
   {}
 
diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index 3a1fbdd1..a79dca92 100644
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -864,10 +864,41 @@  template<typename T>
 bool
 try_canonical_compare(const T *l, const T *r)
 {
+#if WITH_DEBUG_TYPE_CANONICALIZATION
+  // We are debugging the canonicalization of a type down the stack.
+  // 'l' is a subtype of a canonical type and 'r' is a subtype of the
+  // type being canonicalized.  We are at a point where we can compare
+  // 'l' and 'r' either using canonical comparison (if 'l' and 'r'
+  // have canonical types) or structural comparison.
+  //
+  // Because we are debugging the process of type canonicalization, we
+  // want to compare 'l' and 'r' canonically *AND* structurally.  Both
+  // kinds of comparison should yield the same result, otherwise type
+  // canonicalization just failed for the subtype 'r' of the type
+  // being canonicalized.
+  //
+  // In concrete terms, this function is going to be called twice with
+  // the same pair {'l', 'r'} to compare: The first time with
+  // environment::priv_->use_canonical_type_comparison_ set to true,
+  // instructing us to compare them canonically, and the second time
+  // with that boolean set to false, instructing us to compare them
+  // structurally.
+  const environment *env = l->get_environment();
+  if (env->priv_->use_canonical_type_comparison_)
+    {
+      if (const type_base *lc = l->get_naked_canonical_type())
+	if (const type_base *rc = r->get_naked_canonical_type())
+	  ABG_RETURN_EQUAL(lc, rc);
+    }
+  return equals(*l, *r, 0);
+#else
   if (const type_base *lc = l->get_naked_canonical_type())
     if (const type_base *rc = r->get_naked_canonical_type())
       ABG_RETURN_EQUAL(lc, rc);
   return equals(*l, *r, 0);
+#endif
+
+
 }
 
 /// Detect if a recursive comparison cycle is detected while
@@ -3667,6 +3698,26 @@  environment::self_comparison_debug_is_on() const
 {return priv_->self_comparison_debug_on_;}
 #endif
 
+#ifdef WITH_DEBUG_TYPE_CANONICALIZATION
+/// Set the "type canonicalization debugging" mode, triggered by using
+/// the command: "abidw --debug-tc".
+///
+/// @param flag if true then the type canonicalization debugging mode
+/// is enabled.
+void
+environment::debug_type_canonicalization_is_on(bool flag)
+{priv_->debug_type_canonicalization_ = flag;}
+
+/// Getter of the "type canonicalization debugging" mode, triggered by
+/// using the command: "abidw --debug-tc".
+///
+/// @return true iff the type canonicalization debugging mode is
+/// enabled.
+bool
+environment::debug_type_canonicalization_is_on() const
+{return priv_->debug_type_canonicalization_;}
+#endif // WITH_DEBUG_TYPE_CANONICALIZATION
+
 /// Get the vector of canonical types which have a given "string
 /// representation".
 ///
@@ -13535,6 +13586,61 @@  types_defined_same_linux_kernel_corpus_public(const type_base& t1,
   return false;
 }
 
+
+/// Compare a type T against a canonical type.
+///
+/// This function is called during the canonicalization process of the
+/// type T.  T is called the "candidate type" because it's in the
+/// process of being canonicalized.  Meaning, it's going to be
+/// compared to a canonical type C.  If T equals C, then the canonical
+/// type of T is C.
+///
+/// The purpose of this function is to allow the debugging of the
+/// canonicalization of T, if that debugging is activated by
+/// configuring the libabigail package with
+/// --enable-debug-type-canonicalization and by running "abidw
+/// --debug-tc".  In that case, T is going to be compared to C twice:
+/// once with canonical equality and once with structural equality.
+/// The two comparisons must be equal.  Otherwise, the
+/// canonicalization process is said to be faulty and this function
+/// aborts.
+///
+/// This is a sub-routine of type_base::get_canonical_type_for.
+///
+/// @param canonical_type the canonical type to compare the candidate
+/// type against.
+///
+/// @param candidate_type the candidate type to compare against the
+/// canonical type.
+///
+/// @return true iff @p canonical_type equals @p candidate_type.
+///
+static bool
+compare_types_during_canonicalization(const type_base_sptr& canonical_type,
+				      const type_base_sptr& candidate_type)
+{
+#ifdef WITH_DEBUG_TYPE_CANONICALIZATION
+  environment *env = canonical_type->get_environment();
+  if (env->debug_type_canonicalization_is_on())
+    {
+      bool canonical_equality = false, structural_equality = false;
+      env->priv_->use_canonical_type_comparison_ = true;
+      canonical_equality = canonical_type == candidate_type;
+      env->priv_->use_canonical_type_comparison_ = false;
+      structural_equality = canonical_type == candidate_type;
+      if (canonical_equality != structural_equality)
+	{
+	  std::cerr << "structural & canonical equality different for type: "
+		    << canonical_type->get_pretty_representation(true, true)
+		    << std::endl;
+	  ABG_ASSERT_NOT_REACHED;
+	}
+      return structural_equality;
+    }
+#endif //end WITH_DEBUG_TYPE_CANONICALIZATION
+  return canonical_type == candidate_type;
+}
+
 /// Compute the canonical type for a given instance of @ref type_base.
 ///
 /// Consider two types T and T'.  The canonical type of T, denoted
@@ -13665,8 +13771,8 @@  type_base::get_canonical_type_for(type_base_sptr t)
 	  // Compare types by considering that decl-only classes don't
 	  // equal their definition.
 	  env->decl_only_class_equals_definition(false);
-	  bool equal = types_defined_same_linux_kernel_corpus_public(**it, *t)
-		       || *it == t;
+	  bool equal = (types_defined_same_linux_kernel_corpus_public(**it, *t)
+			|| compare_types_during_canonicalization(*it, t));
 	  // Restore the state of the on-the-fly-canonicalization and
 	  // the decl-only-class-being-equal-to-a-matching-definition
 	  // flags.
diff --git a/tools/abidw.cc b/tools/abidw.cc
index 54bf3aa4..240ed095 100644
--- a/tools/abidw.cc
+++ b/tools/abidw.cc
@@ -102,6 +102,9 @@  struct options
   bool			abidiff;
 #ifdef WITH_DEBUG_SELF_COMPARISON
   bool			debug_abidiff;
+#endif
+#ifdef WITH_DEBUG_TYPE_CANONICALIZATION
+  bool			debug_type_canonicalization;
 #endif
   bool			annotate;
   bool			do_log;
@@ -132,6 +135,9 @@  struct options
       abidiff(),
 #ifdef WITH_DEBUG_SELF_COMPARISON
       debug_abidiff(),
+#endif
+#ifdef WITH_DEBUG_TYPE_CANONICALIZATION
+      debug_type_canonicalization(),
 #endif
       annotate(),
       do_log(),
@@ -195,6 +201,9 @@  display_usage(const string& prog_name, ostream& out)
     << "  --abidiff  compare the loaded ABI against itself\n"
 #ifdef WITH_DEBUG_SELF_COMPARISON
     << "  --debug-abidiff  debug the process of comparing the loaded ABI against itself\n"
+#endif
+#ifdef WITH_DEBUG_TYPE_CANONICALIZATION
+    << "  --debug-tc  debug the type canonicalization process\n"
 #endif
     << "  --annotate  annotate the ABI artifacts emitted in the output\n"
     << "  --stats  show statistics about various internal stuff\n"
@@ -348,6 +357,11 @@  parse_command_line(int argc, char* argv[], options& opts)
 	  opts.abidiff = true;
 	  opts.debug_abidiff = true;
 	}
+#endif
+#ifdef WITH_DEBUG_TYPE_CANONICALIZATION
+      else if (!strcmp(argv[i], "--debug-tc")
+	       || !strcmp(argv[i], "debug-type-canonicalization"))
+	opts.debug_type_canonicalization = true;
 #endif
       else if (!strcmp(argv[i], "--annotate"))
 	opts.annotate = true;
@@ -498,6 +512,11 @@  load_corpus_and_write_abixml(char* argv[],
     env->self_comparison_debug_is_on(true);
 #endif
 
+#ifdef WITH_DEBUG_TYPE_CANONICALIZATION
+  if (opts.debug_type_canonicalization)
+    env->debug_type_canonicalization_is_on(true);
+#endif
+
   read_context& ctxt = *context;
   corpus_sptr corp;
   dwarf_reader::status s = dwarf_reader::STATUS_UNKNOWN;