Bug 23708 - categorize void* to pointer change as harmless

Changing a void* pointer into another pointer of the same size is a
change that is harmless in terms of data layout.

This commit thus categorizes such a change as harmless.

	* include/abg-comparison.h (VOID_PTR_TO_PTR_CHANGE_CATEGORY): New
	enumerator in the diff_category enum.  Also, adjust the
	EVERYTHING_CATEGORY enumerator.
	* include/abg-fwd.h (is_void_pointer_type): Declare new function.
	* src/abg-comp-filter.cc (has_void_ptr_to_ptr_change): Define new
	static function and ...
	(categorize_harmless_diff_node): ... use it here.
	* src/abg-comparison.cc (get_default_harmless_categories_bitmap):
	Add the new abigail::comparison::VOID_PTR_TO_PTR_CHANGE_CATEGORY
	category in here.
	(operator<<(ostream& o, diff_category c)): Add support for the new
	VOID_PTR_TO_PTR_CHANGE_CATEGORY.
	* src/abg-ir.cc	(is_void_pointer_type): Define new function.
	* tests/data/Makefile.am: Add the new test material below to source distribution.
	* tests/data/test-diff-filter/test47-filter-void-ptr-change-report-0.txt:
	New test reference output.
	* tests/data/test-diff-filter/test47-filter-void-ptr-change-v{0,1}.c:
	Source code of the new binary test input below.
	* tests/data/test-diff-filter/test47-filter-void-ptr-change-v{0,1}.o:
	New binary test input.
	* tests/test-diff-filter.cc: Add the test input/output above to
	test harness.

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

diff --git a/include/abg-comparison.h b/include/abg-comparison.h
index 51c5db5..e9c6fac 100644 (file)
--- a/include/abg-comparison.h
+++ b/include/abg-comparison.h
@@ -394,6 +394,10 @@ enum diff_category
   /// cv-qualifier change.
   FN_RETURN_TYPE_CV_CHANGE_CATEGORY = 1 << 15,
 
+  /// A diff node in this category carries a change from void pointer
+  /// to non-void pointer.
+  VOID_PTR_TO_PTR_CHANGE_CATEGORY = 1 << 16,
+
   /// A special enumerator that is the logical 'or' all the
   /// enumerators above.
   ///
@@ -416,6 +420,7 @@ enum diff_category
   | FN_PARM_TYPE_TOP_CV_CHANGE_CATEGORY
   | FN_PARM_TYPE_CV_CHANGE_CATEGORY
   | FN_RETURN_TYPE_CV_CHANGE_CATEGORY
+  | VOID_PTR_TO_PTR_CHANGE_CATEGORY
 }; // enum diff_category
 
 diff_category

diff --git a/include/abg-fwd.h b/include/abg-fwd.h
index c70edf1..005fcbe 100644 (file)
--- a/include/abg-fwd.h
+++ b/include/abg-fwd.h
@@ -479,6 +479,9 @@ is_reference_type(const type_or_decl_base*);
 reference_type_def_sptr
 is_reference_type(const type_or_decl_base_sptr&);
 
+const type_base*
+is_void_pointer_type(const type_base*);
+
 qualified_type_def*
 is_qualified_type(const type_or_decl_base*);
 

diff --git a/src/abg-comp-filter.cc b/src/abg-comp-filter.cc
index b3653cb..fd3585e 100644 (file)
--- a/src/abg-comp-filter.cc
+++ b/src/abg-comp-filter.cc
@@ -1165,6 +1165,65 @@ has_fn_return_type_cv_qual_change(const diff* dif)
   return type_diff_has_cv_qual_change_only(return_type_diff);
 }
 
+/// Test if a diff node carries a void* to pointer type change.
+///
+/// Note that this function looks through typedef and qualifier types
+/// to find the void pointer.
+///
+/// @param dif the diff node to consider.
+///
+/// @return true iff @p dif carries a void* to pointer type change.
+static bool
+has_void_ptr_to_ptr_change(const diff* dif)
+{
+  dif = peel_typedef_diff(dif);
+
+  if (const distinct_diff *d = is_distinct_diff(dif))
+    {
+      const type_base *f = is_type(d->first().get());
+      const type_base *s = is_type(d->second().get());
+
+      f = peel_qualified_or_typedef_type(f);
+      s = peel_qualified_or_typedef_type(s);
+
+      if (is_void_pointer_type(f)
+         && is_pointer_type(s)
+         && !is_void_pointer_type(s)
+         && f->get_size_in_bits() == s->get_size_in_bits())
+       return true;
+    }
+  else if (const pointer_diff *d = is_pointer_diff(dif))
+    {
+      const type_base *f = is_type(d->first_pointer()).get();
+      const type_base *s = is_type(d->second_pointer()).get();
+
+      f = peel_qualified_or_typedef_type(f);
+      s = peel_qualified_or_typedef_type(s);
+
+      if (is_void_pointer_type(f)
+         && is_pointer_type(s)
+         && !is_void_pointer_type(s)
+         && f->get_size_in_bits() == s->get_size_in_bits())
+       return true;
+    }
+  else if (const qualified_type_diff *d = is_qualified_type_diff(dif))
+    {
+      const type_base *f = is_type(d->first_qualified_type()).get();
+      const type_base *s = is_type(d->second_qualified_type()).get();
+
+      f = peel_qualified_or_typedef_type(f);
+      s = peel_qualified_or_typedef_type(s);
+
+      if (is_void_pointer_type(f)
+         && is_pointer_type(s)
+         && !is_void_pointer_type(s)
+         && f->get_size_in_bits() == s->get_size_in_bits())
+       return true;
+    }
+
+  return false;
+}
+
 /// Detect if the changes carried by a given diff node are deemed
 /// harmless and do categorize the diff node accordingly.
 ///
@@ -1224,6 +1283,9 @@ categorize_harmless_diff_node(diff *d, bool pre)
       if (has_fn_return_type_cv_qual_change(d))
        category |= FN_RETURN_TYPE_CV_CHANGE_CATEGORY;
 
+      if (has_void_ptr_to_ptr_change(d))
+       category |= VOID_PTR_TO_PTR_CHANGE_CATEGORY;
+
       if (category)
        {
          d->add_to_local_and_inherited_categories(category);

diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
index 012c0eb..2afba7a 100644 (file)
--- a/src/abg-comparison.cc
+++ b/src/abg-comparison.cc
@@ -2923,7 +2923,8 @@ get_default_harmless_categories_bitmap()
          | abigail::comparison::CLASS_DECL_ONLY_DEF_CHANGE_CATEGORY
          | abigail::comparison::FN_PARM_TYPE_TOP_CV_CHANGE_CATEGORY
          | abigail::comparison::FN_PARM_TYPE_CV_CHANGE_CATEGORY
-         | abigail::comparison::FN_RETURN_TYPE_CV_CHANGE_CATEGORY);
+         | abigail::comparison::FN_RETURN_TYPE_CV_CHANGE_CATEGORY
+         | abigail::comparison::VOID_PTR_TO_PTR_CHANGE_CATEGORY);
 }
 
 /// Getter of a bitmap made of the set of change categories that are
@@ -3084,6 +3085,14 @@ operator<<(ostream& o, diff_category c)
       emitted_a_category |= true;
     }
 
+    if (c & VOID_PTR_TO_PTR_CHANGE_CATEGORY)
+    {
+      if (emitted_a_category)
+       o << "|";
+      o << "VOID_PTR_TO_PTR_CHANGE_CATEGORY";
+      emitted_a_category |= true;
+    }
+
   return o;
 }
 

diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index c49070a..9fca36e 100644 (file)
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -7061,6 +7061,30 @@ reference_type_def_sptr
 is_reference_type(const type_or_decl_base_sptr& t)
 {return dynamic_pointer_cast<reference_type_def>(t);}
 
+/// Test if a type is a pointer to void type.
+///
+/// Note that this looks trough typedefs or CV qualifiers to look for
+/// the void pointer.
+///
+/// @param type the type to consider.
+///
+/// @return the actual void pointer if @p is a void pointer or NULL if
+/// it's not.
+const type_base*
+is_void_pointer_type(const type_base* type)
+{
+  type = peel_qualified_or_typedef_type(type);
+
+  const pointer_type_def * t = is_pointer_type(type);
+  if (!t)
+    return 0;
+
+  if (t->get_environment()->is_void_type(t->get_pointed_to_type()))
+    return t;
+
+  return 0;
+}
+
 /// Test whether a type is a reference_type_def.
 ///
 /// @param t the type to test.

diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index b0e78dd..dd455c4 100644 (file)
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -678,6 +678,11 @@ test-diff-filter/test46-fn-return-qual-change-v0.c \
 test-diff-filter/test46-fn-return-qual-change-v1.c \
 test-diff-filter/test46-fn-return-qual-change-v0.o \
 test-diff-filter/test46-fn-return-qual-change-v1.o \
+test-diff-filter/test47-filter-void-ptr-change-report-0.txt \
+test-diff-filter/test47-filter-void-ptr-change-v0.c \
+test-diff-filter/test47-filter-void-ptr-change-v1.c \
+test-diff-filter/test47-filter-void-ptr-change-v0.o \
+test-diff-filter/test47-filter-void-ptr-change-v1.o \
 \
 test-diff-suppr/test0-type-suppr-v0.cc \
 test-diff-suppr/test0-type-suppr-v1.cc \

diff --git a/tests/data/test-diff-filter/test47-filter-void-ptr-change-report-0.txt b/tests/data/test-diff-filter/test47-filter-void-ptr-change-report-0.txt
new file mode 100644 (file)
index 0000000..41fc64d
--- /dev/null
+++ b/tests/data/test-diff-filter/test47-filter-void-ptr-change-report-0.txt
@@ -0,0 +1,3 @@
+Functions changes summary: 0 Removed, 0 Changed (2 filtered out), 0 Added functions
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+

diff --git a/tests/data/test-diff-filter/test47-filter-void-ptr-change-v0.c b/tests/data/test-diff-filter/test47-filter-void-ptr-change-v0.c
new file mode 100644 (file)
index 0000000..b04490a
--- /dev/null
+++ b/tests/data/test-diff-filter/test47-filter-void-ptr-change-v0.c
@@ -0,0 +1,19 @@
+struct S0
+{
+  void* m0;
+};
+
+typedef const void* POINTER;
+
+struct S1
+{
+  POINTER m0;
+};
+
+void foo(struct S0* a __attribute__((unused)))
+{
+}
+
+void bar(struct S1* a __attribute__((unused)))
+{
+}

diff --git a/tests/data/test-diff-filter/test47-filter-void-ptr-change-v0.o b/tests/data/test-diff-filter/test47-filter-void-ptr-change-v0.o
new file mode 100644 (file)
index 0000000..920ff25
Binary files /dev/null and b/tests/data/test-diff-filter/test47-filter-void-ptr-change-v0.o differ

diff --git a/tests/data/test-diff-filter/test47-filter-void-ptr-change-v1.c b/tests/data/test-diff-filter/test47-filter-void-ptr-change-v1.c
new file mode 100644 (file)
index 0000000..7503313
--- /dev/null
+++ b/tests/data/test-diff-filter/test47-filter-void-ptr-change-v1.c
@@ -0,0 +1,19 @@
+struct S0
+{
+  int* m0;
+};
+
+typedef const char* POINTER;
+
+struct S1
+{
+  POINTER m0;
+};
+
+void foo(struct S0* a __attribute__((unused)))
+{
+}
+
+void bar(struct S1* a __attribute__((unused)))
+{
+}

diff --git a/tests/data/test-diff-filter/test47-filter-void-ptr-change-v1.o b/tests/data/test-diff-filter/test47-filter-void-ptr-change-v1.o
new file mode 100644 (file)
index 0000000..38796cb
Binary files /dev/null and b/tests/data/test-diff-filter/test47-filter-void-ptr-change-v1.o differ

diff --git a/tests/test-diff-filter.cc b/tests/test-diff-filter.cc
index aff569e..8184411 100644 (file)
--- a/tests/test-diff-filter.cc
+++ b/tests/test-diff-filter.cc
@@ -528,6 +528,20 @@ InOutSpec in_out_specs[] =
     "data/test-diff-filter/libtest45-basic-type-change-report-1.txt",
     "output/test-diff-filter/libtest45-basic-type-change-report-1.txt",
   },
+  {
+    "data/test-diff-filter/test46-fn-return-qual-change-v0.o",
+    "data/test-diff-filter/test46-fn-return-qual-change-v1.o",
+    "--no-default-suppression",
+    "data/test-diff-filter/test46-fn-return-qual-change-report-0.txt",
+    "output/test-diff-filter/test46-fn-return-qual-change-report-0.txt",
+  },
+  {
+    "data/test-diff-filter/test47-filter-void-ptr-change-v0.o",
+    "data/test-diff-filter/test47-filter-void-ptr-change-v1.o",
+    "--no-default-suppression",
+    "data/test-diff-filter/test47-filter-void-ptr-change-report-0.txt",
+    "output/test-diff-filter/test47-filter-void-ptr-change-report-0.txt",
+  },
   // This should be the last entry
   {NULL, NULL, NULL, NULL, NULL}
 };