This is a partial fix of PR23700.
Conceptually, there are two kinds of type suppression specifications:
1/ a generic user-provided suppression specification that is meant to
suppress changes on types specified by the user
2/ a private type suppression specification that is automatically
generated from the path to public header files provided by the user.
Technically, one difference between 1 and 2 lays in the way we
propagate categories of changes matched by those suppression
specifications.
If a class type change of category SUPPRESSED_CATEGORY is referenced
in a typedef change, then the typedef change is also considered to be
of category SUPPRESSED_CATEGORY. In other words, the
SUPPRESSED_CATEGORY category is propagated to the typedef change.
That means that if a change to a class type is suppressed, a (changed)
typedef to that class is considered to be suppressed too.
But then that is not true if the class type was changed because it's
private. In that, a typedef to that class can be *public*, because
the said typedef is defined in a public header. In that case the
typedef change should *NOT* be considered suppressed just because the
class type change was suppressed.
The problem we have here is that we don't make any difference between
1/ and 2/. So we need to introduce different propagation rules for 1/
and 2/.
So this patch introduces a new PRIVATE_TYPE_CATEGORY category for
types suppression specification that are automatically generated for
private types. That new category has its own propagation rule which
is basically "no propagation"; every type must be matched by the
private type suppression specification to be considered as private.
* include/abg-comp-filter.h (has_harmful_name_change): Declare new
function overloads.
* include/abg-comparison.h (PRIVATE_TYPE_CATEGORY): New enumerator
for diff_category;
(EVERYTHING_CATEGORY): Adjust this enumerator in diff_category;
(is_suppressed): Take an output parameter to say if the
suppression is a private type suppression.
* include/abg-suppression.h (is_private_type_suppr_spec): Take a
const reference parameter and add an overload for a shared
pointer.
* src/abg-comp-filter.cc (has_harmful_name_change): Define new
function.
* src/abg-comparison-priv.h (diff::priv::is_filtered_out): Diffs
of category PRIVATE_TYPE_CATEGORY are also considered filtered
out.
* src/abg-comparison.cc (diff::is_filtered_out): Adjust to account
for canonical diffs of category PRIVATE_TYPE_CATEGORY.
(diff::is_suppressed): Add an overload that takes a
is_private_type output parameter. Re-write the old overload in
terms of the new one.
(operator<<(ostream& o, diff_category c)): Handle
PRIVATE_TYPE_CATEGORY.
(category_propagation_visitor::visit_end): Do not propagate
PRIVATE_TYPE_CATEGORY here. Do not propagate
HARMLESS_DECL_NAME_CHANGE_CATEGORY either, when the class does
have a harmful decl name change.
(suppression_categorization_visitor::visit_begin): Set the new
PRIVATE_TYPE_CATEGORY category but do not propagate it.
(suppression_categorization_visitor::visit_end): Add some
comments.
* src/abg-default-reporter.cc (default_reporter::report): Avoid
reporting typedef underlying types that are in the
PRIVATE_TYPE_CATEGORY category.
* src/abg-suppression.cc (type_suppression::suppresses_diff): Do
not peel typedefs if we are a private type suppression.
(is_private_type_suppr_spec): Take a const reference.
* tests/data/Makefile.am: Add the new test material below to
source distribution.
* tests/test-diff-suppr.cc: Use new test binary input.
* tests/data/test-diff-filter/test7-report.txt: Adjust.
* tests/data/test-diff-suppr/test39-opaque-type-report-0.txt: New
test reference output.
* tests/data/test-diff-suppr/test39-opaque-type-v{0,1}.c: Source
code of new test binary input.
* tests/data/test-diff-suppr/test39-opaque-type-v{0,1}.o: New test
binary input.
* tests/data/test-diff-suppr/test39-public-headers-dir/test39-header-v{0,1}.h:
Source code of new test binary input.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
has_harmless_name_change(const decl_base_sptr& f, const decl_base_sptr& s);
bool
+has_harmful_name_change(const decl_base_sptr& f, const decl_base_sptr& s);
+
+bool
+has_harmful_name_change(const diff* dif);
+
+bool
has_virtual_mem_fn_change(const function_decl_diff* diff);
bool
/// user-provided suppression specification.
SUPPRESSED_CATEGORY = 1 << 7,
+ /// This means that a diff node was warked as being for a private
+ /// type. That is, the diff node is meant to be suppressed by a
+ /// suppression specification that was auto-generated to filter out
+ /// changes to private types.
+ PRIVATE_TYPE_CATEGORY = 1 << 8,
+
/// This means the diff node (or at least one of its descendant
/// nodes) carries a change that modifies the size of a type or an
/// offset of a type member. Removal or changes of enumerators in a
/// enum fall in this category too.
- SIZE_OR_OFFSET_CHANGE_CATEGORY = 1 << 8,
+ SIZE_OR_OFFSET_CHANGE_CATEGORY = 1 << 9,
/// This means that a diff node in the sub-tree carries an
/// incompatible change to a vtable.
- VIRTUAL_MEMBER_CHANGE_CATEGORY = 1 << 9,
+ VIRTUAL_MEMBER_CHANGE_CATEGORY = 1 << 10,
/// A diff node in this category is redundant. That means it's
/// present as a child of a other nodes in the diff tree.
- REDUNDANT_CATEGORY = 1 << 10,
+ REDUNDANT_CATEGORY = 1 << 11,
/// This means that a diff node in the sub-tree carries a class type
/// that was declaration-only and that is now defined, or vice
/// versa.
- CLASS_DECL_ONLY_DEF_CHANGE_CATEGORY = 1 << 11,
+ CLASS_DECL_ONLY_DEF_CHANGE_CATEGORY = 1 << 12,
/// A diff node in this category is a function parameter type which
/// top cv-qualifiers change.
- FN_PARM_TYPE_TOP_CV_CHANGE_CATEGORY = 1 << 12,
+ FN_PARM_TYPE_TOP_CV_CHANGE_CATEGORY = 1 << 13,
/// A diff node in this category is a function parameter type which
/// cv-qualifiers change.
- FN_PARM_TYPE_CV_CHANGE_CATEGORY = 1 << 13,
+ FN_PARM_TYPE_CV_CHANGE_CATEGORY = 1 << 14,
/// A special enumerator that is the logical 'or' all the
/// enumerators above.
| HARMLESS_ENUM_CHANGE_CATEGORY
| HARMLESS_SYMBOL_ALIAS_CHANGE_CATEORY
| SUPPRESSED_CATEGORY
+ | PRIVATE_TYPE_CATEGORY
| SIZE_OR_OFFSET_CHANGE_CATEGORY
| VIRTUAL_MEMBER_CHANGE_CATEGORY
| REDUNDANT_CATEGORY
is_suppressed() const;
bool
+ is_suppressed(bool &is_private_type) const;
+
+ bool
to_be_reported() const;
bool
get_private_types_suppr_spec_label();
bool
-is_private_type_suppr_spec(suppr::type_suppression&);
+is_private_type_suppr_spec(const type_suppression&);
+
+bool
+is_private_type_suppr_spec(const suppression_sptr& s);
} // end namespace suppr
} // end namespace abigail
0))));
}
+/// Test if two decls represents a harmful name change.
+///
+/// A harmful name change is a name change that is not harmless, so
+/// this function uses the function has_harmless_name_change.
+///
+/// @param f the first decl to consider in the comparison.
+///
+/// @param s the second decl to consider in the comparison.
+///
+/// @return true iff decl @p s represents a harmful name change over
+/// @p f.
+bool
+has_harmful_name_change(const decl_base_sptr& f, const decl_base_sptr& s)
+{return decl_name_changed(f, s) && ! has_harmless_name_change(f, s);}
+
+/// Test if a diff node represents a harmful name change.
+///
+/// A harmful name change is a name change that is not harmless, so
+/// this function uses the function has_harmless_name_change.
+///
+/// @param f the first decl to consider in the comparison.
+///
+/// @param s the second decl to consider in the comparison.
+///
+/// @return true iff decl @p s represents a harmful name change over
+/// @p f.
+bool
+has_harmful_name_change(const diff* dif)
+{
+ decl_base_sptr f = is_decl(dif->first_subject()),
+ s = is_decl(dif->second_subject());
+
+ return has_harmful_name_change(f, s);
+}
+
/// Test if a class_diff node has non-static members added or
/// removed.
///
return false;
/// We don't want to display nodes suppressed by a user-provided
- /// suppression specification.
- if (category & SUPPRESSED_CATEGORY)
+ /// suppression specification or by a "private type" suppression
+ /// specification.
+ if (category & (SUPPRESSED_CATEGORY | PRIVATE_TYPE_CATEGORY))
return true;
// We don't want to display redundant diff nodes, when the user
diff::is_filtered_out() const
{
if (diff * canonical = get_canonical_diff())
- if (canonical->get_category() & SUPPRESSED_CATEGORY)
- // The canonical type was suppressed. This means all the class
- // of equivalence of that canonical type was suppressed. So
- // this node should be suppressed too.
+ if (canonical->get_category() & SUPPRESSED_CATEGORY
+ || canonical->get_category() & PRIVATE_TYPE_CATEGORY)
+ // The canonical type was suppressed either by a user-provided
+ // suppression specification or by a "private-type" suppression
+ // specification.. This means all the class of equivalence of
+ // that canonical type was suppressed. So this node should be
+ // suppressed too.
return true;
return priv_->is_filtered_out(get_category());
}
bool
diff::is_suppressed() const
{
+ bool is_private = false;
+ return is_suppressed(is_private);
+}
+
+/// Test if the current diff node has been suppressed by a
+/// user-provided suppression specification or by an auto-generated
+/// "private type" suppression specification.
+///
+/// Note that private type suppressions are auto-generated from the
+/// path to where public headers are, as given by the user.
+///
+/// @param is_private_type out parameter if the current diff node was
+/// suppressed because it's a private type then this parameter is set
+/// to true.
+///
+/// @return true if the current diff node has been suppressed by a
+/// user-provided suppression list.
+bool
+diff::is_suppressed(bool &is_private_type) const
+{
const suppressions_type& suppressions = context()->suppressions();
- for (suppressions_type::const_iterator i = suppressions.begin();
+ for (suppressions_type::const_iterator i = suppressions.begin();
i != suppressions.end();
++i)
- if ((*i)->suppresses_diff(this))
- return true;
+ {
+ if ((*i)->suppresses_diff(this))
+ {
+ if (is_private_type_suppr_spec(*i))
+ is_private_type = true;
+ return true;
+ }
+ }
return false;
}
o << "STATIC_DATA_MEMBER_CHANGE_CATEGORY";
emitted_a_category |= true;
}
- else if (c & HARMLESS_ENUM_CHANGE_CATEGORY)
+
+ if (c & HARMLESS_ENUM_CHANGE_CATEGORY)
{
if (emitted_a_category)
o << "|";
emitted_a_category |= true;
}
+ if (c & SUPPRESSED_CATEGORY)
+ {
+ if (emitted_a_category)
+ o << "|";
+ o << "SUPPRESSED_CATEGORY";
+ emitted_a_category |= true;
+ }
+
+ if (c & PRIVATE_TYPE_CATEGORY)
+ {
+ if (emitted_a_category)
+ o << "|";
+ o << "PRIVATE_TYPE_CATEGORY";
+ emitted_a_category |= true;
+ }
+
if (c & SIZE_OR_OFFSET_CHANGE_CATEGORY)
{
if (emitted_a_category)
emitted_a_category |= true;
}
-
- if (c & SUPPRESSED_CATEGORY)
+ if (c & FN_PARM_TYPE_CV_CHANGE_CATEGORY)
{
if (emitted_a_category)
o << "|";
- o << "SUPPRESSED_CATEGORY";
+ o << "FN_PARM_TYPE_CV_CHANGE_CATEGORY";
emitted_a_category |= true;
}
assert(diff);
diff_category c = diff->get_category();
- c &= ~(REDUNDANT_CATEGORY|SUPPRESSED_CATEGORY);
+ // Do not propagate redundant and suppressed categories. Those
+ // are propagated in a specific pass elsewhere.
+ c &= ~(REDUNDANT_CATEGORY
+ | SUPPRESSED_CATEGORY
+ | PRIVATE_TYPE_CATEGORY);
+ // Also, if a (class) type has got a harmful name change, do not
+ // propagate harmless name changes coming from its sub-types
+ // (i.e, data members) to the class itself.
+ if (filtering::has_harmful_name_change(d))
+ c &= ~HARMLESS_DECL_NAME_CHANGE_CATEGORY;
+
d->add_to_category(c);
if (!already_visited && canonical)
if (update_canonical)
virtual void
visit_begin(diff* d)
{
- if (d->is_suppressed())
+ bool is_private_type = false;
+ if (d->is_suppressed(is_private_type))
{
- d->add_to_local_and_inherited_categories(SUPPRESSED_CATEGORY);
+ diff_category c = is_private_type
+ ? PRIVATE_TYPE_CATEGORY
+ : SUPPRESSED_CATEGORY;
+ d->add_to_local_and_inherited_categories(c);
// If a node was suppressed, all the other nodes of its class
// of equivalence are suppressed too.
diff *canonical_diff = d->get_canonical_diff();
if (canonical_diff != d)
- canonical_diff->add_to_category(SUPPRESSED_CATEGORY);
+ canonical_diff->add_to_category(c);
}
}
&& (!d->has_local_changes()
|| is_pointer_diff(d)))
{
+ // Note that we handle private diff nodes differently from
+ // generally suppressed diff nodes. E.g, it's not because a
+ // type is private (and suppressed because of that; i.e, in
+ // the category PRIVATE_TYPE_CATEGORY) that a typedef to that
+ // type should also be private and so suppressed. Private
+ // diff nodes thus have different propagation rules than
+ // generally suppressed rules.
for (vector<diff*>::const_iterator i = d->children_nodes().begin();
i != d->children_nodes().end();
++i)
// want to just know about the local changes of the typedef,
// but also about the changes on the underlying type.
diff_category c = dif->get_category();
- if (!(c & SUPPRESSED_CATEGORY))
+ if (!(c & (SUPPRESSED_CATEGORY | PRIVATE_TYPE_CATEGORY)))
{
out << indent
<< "underlying type '"
if (!suppresses_type(ft, d->context())
&& !suppresses_type(st, d->context()))
{
- ft = peel_typedef_type(ft);
- st = peel_typedef_type(st);
+ // A private type suppression specification considers tha a type
+ // can be private and yet some typedefs of that type can be
+ // public -- depending on, e.g, if the typedef is defined in a
+ // public header or not. So if we are in the context of a
+ // private type suppression let's *NOT* peel typedefs away.
+ if (!is_private_type_suppr_spec(*this))
+ {
+ ft = peel_typedef_type(ft);
+ st = peel_typedef_type(st);
+ }
if (!suppresses_type(ft, d->context())
&& !suppresses_type(st, d->context()))
///
/// @return true iff @p s is a private type suppr spec.
bool
-is_private_type_suppr_spec(suppr::type_suppression& s)
+is_private_type_suppr_spec(const type_suppression& s)
{return s.get_label() == get_private_types_suppr_spec_label();}
+/// Test if a type suppression specification represents a private type
+/// suppression automatically generated by libabigail from the user
+/// telling us where public headers are.
+///
+/// @param s the suppression specification we are looking at.
+///
+/// @return true iff @p s is a private type suppr spec.
+bool
+is_private_type_suppr_spec(const suppression_sptr& s)
+{
+ type_suppression_sptr type_suppr = is_type_suppression(s);
+ return (type_suppr
+ && type_suppr->get_label() == get_private_types_suppr_spec_label());
+}
+
// </file_suppression stuff>
}// end namespace suppr
} // end namespace abigail
test-diff-suppr/test38-char-class-in-ini-v1.c \
test-diff-suppr/test38-char-class-in-ini-v1.o \
test-diff-suppr/test38-char-class-in-ini.abignore \
+test-diff-suppr/test39-opaque-type-report-0.txt \
+test-diff-suppr/test39-opaque-type-v0.c \
+test-diff-suppr/test39-opaque-type-v1.c \
+test-diff-suppr/test39-opaque-type-v0.o \
+test-diff-suppr/test39-opaque-type-v1.o \
+test-diff-suppr/test39-public-headers-dir/test39-header-v0.h \
+test-diff-suppr/test39-public-headers-dir/test39-header-v1.h \
\
test-diff-dwarf-abixml/test0-pr19026-libvtkIOSQL-6.1.so.1 \
test-diff-dwarf-abixml/test0-pr19026-libvtkIOSQL-6.1.so.1.abi \
-Functions changes summary: 0 Removed, 0 Changed (1 filtered out), 0 Added function
+Functions changes summary: 0 Removed, 1 Changed, 0 Added function
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+1 function with some indirect sub-type change:
+
+ [C]'function return_type foo()' has some indirect sub-type changes:
+ return type changed:
+ type name changed from 'return_type' to 'other_return_type'
+ type size hasn't changed
+
+ no data member change (1 filtered);
+
+
--- /dev/null
+Functions changes summary: 0 Removed, 1 Changed (1 filtered out), 0 Added functions
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+
+1 function with some indirect sub-type change:
+
+ [C]'function void bar(StructType*)' at test39-opaque-type-v1.c:20:1 has some indirect sub-type changes:
+ parameter 1 of type 'StructType*' changed:
+ in pointed to type 'typedef StructType' at test39-header-v1.h:2:1:
+ typedef name changed from StructType to AnotherStructType at test39-header-v1.h:2:1
+
+
+
--- /dev/null
+#include "test39-public-headers-dir/test39-header-v0.h"
+
+struct _StructType
+{
+ int m0;
+};
+
+void
+foo(StructType* a __attribute__((unused)))
+{
+}
+
+void bar(StructType* a __attribute__((unused)))
+{
+}
--- /dev/null
+#include "test39-public-headers-dir/test39-header-v1.h"
+
+struct _StructType
+{
+ int m0;
+ char m1;
+};
+
+struct _AnotherStructType
+{
+ int m1;
+};
+
+
+void
+foo(StructType* a __attribute__((unused)))
+{
+}
+
+void bar(AnotherStructType* a __attribute__((unused)))
+{
+}
--- /dev/null
+typedef struct _StructType StructType;
+
+void foo(StructType*);
+void bar(StructType*);
--- /dev/null
+typedef struct _StructType StructType;
+typedef struct _AnotherStructType AnotherStructType;
+
+void foo(StructType*);
+void bar(AnotherStructType*);
"data/test-diff-suppr/test38-char-class-in-ini-report-0.txt",
"output/test-diff-suppr/test38-char-class-in-ini-report-0.txt"
},
+ {
+ "data/test-diff-suppr/test39-opaque-type-v0.o",
+ "data/test-diff-suppr/test39-opaque-type-v1.o",
+ "data/test-diff-suppr/test39-public-headers-dir",
+ "data/test-diff-suppr/test39-public-headers-dir",
+ "",
+ "--no-default-suppression",
+ "data/test-diff-suppr/test39-opaque-type-report-0.txt",
+ "output/test-diff-suppr/test39-opaque-type-report-0.txt"
+ },
// This should be the last entry
{NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL}
};
projects / platform / upstream / libabigail.git / commitdiff