Commit Graph

8 Commits

Author SHA1 Message Date
Dodji Seketeli
56d958641c Bug 17649 Avoid endless looping on diff graph with cycles
Bug URL: https://sourceware.org/bugzilla/show_bug.cgi?id=17649.

abidiff stumbled accross a diff graph with cycles.  And it kept
walking that graph endlessly.  Of course.

It turned out on such graphs with cycles, the categorizing code that
uses abigail::comparison::diff::traverse() to walk the graph and
categorize the diff nodes was traversing the same class of equivalence
of certain diff nodes more than once without even noticing.

This patch changes the logic of the diff graph traversing code to make
it always call diff_node_visitor::visit_begin() on the visitor for a
diff node prior to visiting it (visiting means calling
diff_node_visitor::visit()) and diff_node_visitor::visit_end() after
visiting it.

But when the diff node has already been visited and it's reached again
by the traversing code (in case of a cycle) then the
diff_node_visitor::visit_begin() is called, but
diff_node_visitor::visit() is *NOT*.  Then
diff_node_visitor::visit_end() is called.  In other words, even when
the diff node is not visited (because it's already been visited) the
pair diff_node_visitor::{visit_begin,visit_end}() is called.

This avoids traversing the diff node (or rather the equivalence class
of the diff node) more than once even in presence of cycles, but still
gives a chance to custom visitors to detect that they are seeing a
cycle and act accordingly if need be.  This is a kind of cycle
detection feature.

Then the code of the (harmless and harmful categorization) filters has
been adapted to always rely on the cycle detection feature.  The code
of the category propagation visitor has also been adapted to propagate
the category of a given diff node to and from its canonical diff node.

	* include/abg-comp-filter.h (harm{less,ful}_filter::visit_end):
	Declare new methods.
	* include/abg-comparison.h (diff_context::maybe_apply_filters):
	Remove the traverse_nodes_once flag.
	* src/abg-comp-filter.cc (apply_filter): Force the traversing to
	operate in cycle avoidance mode.
	(harm{less,ful}_filter::visit): Update the category of the
	canonical node too.
	(harm{less,ful}_filter::visit_end): Define new method.
	* src/abg-comparison.cc (diff_context::maybe_apply_filters):
	Remove the traverse_nodes_once flag.  Adjust.  Simplify logic.
	(diff::traverse): Always call diff_node_visitor::{begin,end}.  If
	the node has already been visited previously then do not call
	diff_node_visitor::visit() and do not visit the children nodes.
	(category_propagation_visitor::visit_end):  If the node has
	already been visited, then propagate the category from the
	canonical nodes of the children nodes.
	(propagate_categories):  Force the traversing to operate in cycle
	avoidance mode.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2015-02-21 15:16:48 +01:00
Dodji Seketeli
76837d1cbf Update copyright years
* include/abg-comp-filter.h: Update copyright years.
	* include/abg-comparison.h: Likewise.
	* include/abg-config.h: Likewise.
	* include/abg-corpus.h: Likewise.
	* include/abg-diff-utils.h: Likewise.
	* include/abg-dwarf-reader.h: Likewise.
	* include/abg-fwd.h: Likewise.
	* include/abg-hash.h: Likewise.
	* include/abg-ini.h: Likewise.
	* include/abg-ir.h: Likewise.
	* include/abg-libxml-utils.h: Likewise.
	* include/abg-libzip-utils.h: Likewise.
	* include/abg-reader.h: Likewise.
	* include/abg-sptr-utils.h: Likewise.
	* include/abg-traverse.h: Likewise.
	* include/abg-viz-common.h: Likewise.
	* include/abg-viz-dot.h: Likewise.
	* include/abg-viz-svg.h: Likewise.
	* include/abg-writer.h: Likewise.
	* src/abg-comp-filter.cc: Likewise.
	* src/abg-comparison.cc: Likewise.
	* src/abg-config.cc: Likewise.
	* src/abg-corpus.cc: Likewise.
	* src/abg-diff-utils.cc: Likewise.
	* src/abg-dwarf-reader.cc: Likewise.
	* src/abg-hash.cc: Likewise.
	* src/abg-ini.cc: Likewise.
	* src/abg-ir.cc: Likewise.
	* src/abg-libxml-utils.cc: Likewise.
	* src/abg-libzip-utils.cc: Likewise.
	* src/abg-reader.cc: Likewise.
	* src/abg-traverse.cc: Likewise.
	* src/abg-viz-common.cc: Likewise.
	* src/abg-viz-dot.cc: Likewise.
	* src/abg-viz-svg.cc: Likewise.
	* src/abg-writer.cc: Likewise.
	* tests/print-diff-tree.cc: Likewise.
	* tests/test-abidiff.cc: Likewise.
	* tests/test-alt-dwarf-file.cc: Likewise.
	* tests/test-core-diff.cc: Likewise.
	* tests/test-diff-dwarf.cc: Likewise.
	* tests/test-diff-filter.cc: Likewise.
	* tests/test-diff-suppr.cc: Likewise.
	* tests/test-diff2.cc: Likewise.
	* tests/test-ir-walker.cc: Likewise.
	* tests/test-lookup-syms.cc: Likewise.
	* tests/test-read-dwarf.cc: Likewise.
	* tests/test-read-write.cc: Likewise.
	* tests/test-utils.cc: Likewise.
	* tests/test-utils.h: Likewise.
	* tests/test-write-read-archive.cc: Likewise.
	* tools/abg-tools-utils.cc: Likewise.
	* tools/abg-tools-utils.h: Likewise.
	* tools/abiar.cc: Likewise.
	* tools/abidiff.cc: Likewise.
	* tools/abidw.cc: Likewise.
	* tools/abilint.cc: Likewise.
	* tools/abisym.cc: Likewise.
	* tools/binilint.cc: Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2015-01-07 17:52:10 +01:00
Dodji Seketeli
2d51a6429f Remove useless redundant_filter
* include/abg-comp-filter.h (class redundant_filter): Remove this
	now useless type declaration
	* src/abg-comparison.cc (filtering::redundant_filter::visit):
	Remove this useless member function definition.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2014-10-13 17:39:26 +02:00
Dodji Seketeli
2a77bead11 Implement generic diff tree walking and port categorization over it
* include/abg-comp-filter.h (apply_filter): Declare new overload
	that takes a corpus_diff_sptr ...
	* src/abg-comp-filter.cc (apply_filter): ... and define it.  On
	the existing overload for diff_sptr, make sure to traverse all
	diff nodes, even those that have already been traversed.
	* include/abg-comparison.h (enum diff_category): Remove
	NOT_REDUNDANT_CATEGORY, add REDUNDANT_CATEGORY.
	(operator&=, +operator<<): Declare new operators for enum diff_category.
	(diff_context::{forbid_traversing_a_node_twice,
	traversing_a_node_twice_is_forbidden):
	(diff_context::categorizing_redundancy): Remove this declaration.
	(diff_context::maybe_apply_filters): Declare a new overload that
	takes a corpus_diff_sptr.  And a take a new flag that says if it
	should visit all nodes including those that have already been
	visited.
	(diff::priv_): Make this data member protected.
	(diff::{begin_traversing, is_traversing, end_traversing,
	finish_diff_type, children_nodes, append_child_node,
	get_pretty_representation, chain_into_hierarchy, traverse}):
	Declare new member functions.
	(distinct_diff::{finish_diff_type, get_pretty_representation,
	chain_into_hierarchy}): Likewise.
	(distinct_diff::traverse): Remove.
	(pointer_diff::pointer_diff): Take the underlying type diff in
	parameter.
	(pointer_diff::{finish_diff_type, get_pretty_representation,
	chain_into_hierarchy}): Declare new member functions.
	(pointer_diff::traverse): Remove.
	(reference_type_def::reference_type_def): Take the underlying type
	diff in parameter.
	({array_type_def, reference_type_def}::{finish_diff_type,
	get_pretty_representation, chain_into_hierarchy}): Declare new
	member functions.
	({array_type_diff, reference_type_def}::traverse): Remove.
	(qualified_type_diff::qualified_type_diff): Take the underlying
	type diff in parameter.
	({enum_diff, qualified_type_diff, class_diff}::{finish_diff_type,
	get_pretty_representation, chain_into_hierarchy}): Declare new
	member functions.
	({enum_diff, qualified_type_diff, class_diff}::traverse): Remove.
	(is_class_diff): Declare new function.
	(base_diff::base_diff): Take the underlying type diff in
	parameter.
	({scope_diff, base_diff}::{finish_diff_type, get_pretty_representation,
	chain_into_hierarchy}): Declare new member functions.
	({scope_diff, base_diff}::traverse): Remove.
	(function_decl_diff::function_decl_diff): Take the return type
	diff as parameter.
	({function_decl_diff, type_decl_diff}::{finish_diff_type,
	get_pretty_representation, chain_into_hierarchy}): Declare new
	member functions.
	({function_decl_diff, type_decl_diff}::traverse): Remove.
	(typedef_diff::typedef_diff): Take the underlying type diff as
	parameter.
	(typedef::{finish_diff_type, get_pretty_representation,
	chain_into_hierarchy}): Declare new member functions.
	({typedef, translation_unit_diff}::traverse): Remove member
	function.
	(corpus_diff::{finish_diff_type, children_nodes,
	append_child_node, changed_variables, get_pretty_representation,
	chain_into_hierarchy}): Declare new member functions.
	(class diff_node_visitor::{visit_begin, visit_end}): Declare new
	member functions.
	(propagate_categories, print_diff_tree, categorizing_redundancy)
	(clear_redundancy_categorization, apply_filters): New functions
	and function overloads.
	* src/abg-comparison.cc (TRY_PRE_VISIT, TRY_PRE_VISIT_CLASS_DIFF)
	(TRY_POST_VISIT, TRY_POST_VISIT_CLASS_DIFF)
	(CATEGORIZE_REDUNDANCY_FROM_CHILD_NODE)
	(UPDATE_REDUNDANCY_CATEGORIZATION_FROM_NODE_SUBTREE)
	(TRAVERSE_DIFF_NODE_AND_PROPAGATE_CATEGORY)
	(TRAVERSE_MEM_DIFF_NODE_AND_PROPAGATE_CATEGORY)
	(TRAVERSE_MEM_FN_DIFF_NODE_AND_PROPAGATE_CATEGORY)
	(ENSURE_DIFF_NODE_TRAVERSED_ONCE)
	(ENSURE_MEM_DIFF_NODE_TRAVERSED_ONCE): Remove these macros.
	Hurrah.
	(diff_context::priv::categorizing_redundancy_): Remove.
	(diff_context::priv::forbid_traversing_a_node_twice_): Add new
	data member.
	(diff_context::priv::priv): Adjust.
	(diff_context::{forbid_traversing_a_node_twice,
	traversing_a_node_twice_is_forbidden}): Define new member
	functions.
	(diff_context::maybe_apply_filters): Once filters are applied (and
	categories are set to the relevant diff tree nodes, run a pass
	over the diff tree to propagate the categories to the relevant
	diff tree parent nodes.  Add an overload for corpus_diff_sptr.
	(diff_context::categorizing_redundancy): Remove member function.
	(diff_context::maybe_apply_filters): Define a new overload for
	corpus_diff_sptr
	(struct diff::priv::{finished_, traversing_, children_,
	pretty_representation_}):  New data members.
	(diff::priv::priv): Adjust.
	(diff::{begin_traversing, is_traversing, end_traversing,
	finish_diff_type, children_nodes, append_child_node, traverse,
	set_category, get_pretty_representation, chain_into_hierarchy}):
	Define new member functions.
	(diff::is_filtered_out): Do not refer to NOT_REDUNDANT_CATEGORY
	anymore.  Rather, use the new REDUNDANT_CATEGORY.
	({distinct_diff, var_diff, pointer_diff, array_diff,
	reference_diff, qualified_type_diff, enum_diff, class_diff,
	base_diff, scope_diff, function_decl_diff, type_decl_diff,
	typedef_diff}::{get_pretty_representation, chain_into_hierarchy,
	finish_diff_type}): Define new member functions.
	({distinct_diff, var_diff, pointer_diff, array_diff,
	reference_diff, qualified_type_diff, enum_diff, class_diff,
	base_diff, scope_diff, function_decl_diff, type_decl_diff,
	typedef_diff, translation_unit_diff}::traverse): Remove member
	functions.
	(operator&=, operator<<): Define new operators for diff_category.
	({function_decl_diff, typedef_diff}::priv::priv): Add a new
	constructor.
	(pointer_diff::{priv::priv, pointer_diff})
	(reference_diff::{priv::priv, reference_diff})
	(qualified_type_diff::{priv::priv, qualified_type_diff})
	(enum_diff::{priv::priv, enum_diff}, base_diff::{priv::priv,
	base_diff}, function_decl_diff::function_decl_diff): Take the
	underlying type diff in parameter.
	(compute_diff): Adjust the pointer_diff, reference_diff,
	qualified_type_diff, base_diff, function_decl_diff overloads.
	(class_diff::priv::{count_filtered_bases,
	count_filtered_subtype_changed_dm, count_filtered_changed_dm,
	count_filtered_changed_mem_fns, count_filtered_inserted_mem_fns,
	count_filtered_deleted_mem_fns}): Adjust for the call to
	diff_context::maybe_apply_filters.
	(corpus_diff::priv::{finished_, pretty_representation_}): New data
	member.
	(corpus_diff::priv::priv): New constructor.
	(corpus_diff::priv::clear_redundancy_categorization): Define new
	member function.
	(corpus_diff::priv::apply_filters_and_compute_diff_stats):
	Adjust for call to diff_context::maybe_apply_filters.  Also, call
	clear_redundancy_categorization at the end.
	(corpus_diff::priv::categorize_redundant_changed_sub_nodes):
	Revisit logic.
	(corpus_diff::{chain_into_hierarchy, finish_diff_type,
	children_nodes, append_child_node, changed_variables,
	get_pretty_representation}): Define new member functions.
	(corpus_diff::report): Categorize redundancy for every top level
	function/variable diff.
	(corpus_diff::traverse): Adjust to the new traversing interface.
	(diff_node_visitor::{visit_begin, visit_end}): Define new member
	functions.
	(struct category_propagation_visitor, struct diff_node_printer)
	(struct redundancy_marking_visitor, struct
	redundancy_clearing_visitor): New diff tree node visitors.
	(propagate_categories, print_diff_tree, categorize_redundancy)
	(clear_redundancy_categorization, apply_filters): Define new
	functions.
	* tests/Makefile.am: Add the new tests/print-diff-tree.cc to the
	source distribution.  Build it into a tests/printdifftree binary.
	* tools/abidiff.cc (print_diff_tree): Add debugging functions to
	call from within the debugger.  By default, this function and its
	overloads are not compiled.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2014-10-10 13:18:04 +02:00
Dodji Seketeli
b6d7fcc515 A builtin type name change is not harmless - fix that
* include/abg-comp-filter.h (has_harmless_name_change): New
	function declaration.
	* include/abg-comparison.h
	(diff_category::DECL_NAME_CHANGE_CATEGORY): Renamed this into
	HARMLESS_DECL_NAME_CHANGE_CATEGORY.
	(diff_category::EVERYTHING_CATEGORY): Update.
	* include/abg-fwd.h (is_enum): New function declaration.
	(is_var_decl): Return the shared_ptr<var_decl> rather than a bool.
	(is_data_member): New overload that takes a shared_ptr<decl_base>.
	* src/abg-comp-filter.cc (decl_name_changed): Consider the
	qualified name here.
	(has_harmless_name_change): Define new function declaration.
	(harmless_filter::visit): Use the new has_harmless_name_change
	function.
	* src/abg-comparison.cc (represent)
	(report_name_size_and_alignment_changes, enum_diff::report)
	(typedef_diff::report, is_data_member): Use the new
	filtering::has_harmless_name_change function to simplify logic of
	emitting the name change related diff
	* tools/bidiff.cc (set_diff_context_from_opts): Adjust
	DECL_NAME_CHANGE_CATEGORY -> HARMLESS_DECL_NAME_CHANGE_CATEGORY.
	* src/abg-ir.cc (is_data_member, is_enum): New function definitions.
	(is_var_decl): Return the var_decl_sptr rather than just a bool.
	* tests/data/test-diff-filter/test13-report.txt: Adjust.
	* tests/data/test-diff-filter/test6-report.txt: Adjust.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2014-08-22 17:19:27 +02:00
Dodji Seketeli
591014bf40 Avoid reporting diff nodes that have already been reported
* include/abg-comp-filter.h (class harmful_filter): Update
	comment.
	(class redundant_filter): Declare new filter.
	* include/abg-comparison.h (enum
	diff_category::NOT_REDUNDANT_CATEGORY): New category.  Update the
	values of the other enumerators.
	(diff_context::{add_diff, diff_has_been_traversed}): New overloads.
	(diff_context::{categorizing_redundancy, show_redundant_changes}):
	Declare new methods.
	(diff_context::remove_from_category): Define new inline method.
	* src/abg-comparison.cc (noop_deleter::operator()): Constify the
	parameter.
	(CATEGORIZE_REDUNDANCY_FROM_CHILD_NODE)
	(UPDATE_REDUNDANCY_CATEGORIZATION_FROM_NODE_SUBTREE): New macros.
	(TRAVERSE_DIFF_NODE_AND_PROPAGATE_CATEGORY)
	(TRAVERSE_MEM_DIFF_NODE_AND_PROPAGATE_CATEGORY)
	(TRAVERSE_MEM_FN_DIFF_NODE_AND_PROPAGATE_CATEGORY): Use the new
	CATEGORIZE_REDUNDANCY_FROM_CHILD_NODE and
	UPDATE_REDUNDANCY_CATEGORIZATION_FROM_NODE_SUBTREE macros above.
	(ENSURE_DIFF_NODE_TRAVERSED_ONCE)
	(ENSURE_MEM_DIFF_NODE_TRAVERSED_ONCE): If the (type_decl or class)
	node hasn't been yet traversed, mark it as non-redundant.
	(diff_context::priv::categorizing_redundancy): New member.
	(diff_context::priv::priv): Initialize it.
	(diff_context::{add_diff, diff_has_been_traversed): Define new
	overloads.
	(diff_context::mark_diff_as_traversed): Intern a diff node that is
	marked as being traversed.
	(diff_context::{categorizing_redundancy, show_redundant_changes}):
	Define new methods.
	(diff::is_filtered_out): A redundant function or top-level
	variable is considered filtered-out.  Otherwise, the new
	NOT_REDUNDANT_CATEGORY doesn't play any role when comparing
	allowed categories with the set of categories a diff node belongs
	to.
	(corpus::priv::categorize_redundant_changed_sub_nodes): Define
	new member function.
	(corpus_diff::priv::apply_filters_and_compute_diff_stats): Change
	this to first walk the changed functions and variables to apply
	filters, then categorize redundant changed functions, and then
	walk the changed functions and variables again to count
	filtered-out diff nodes.
	(filtering::redundant_filter::visit): Define new member function.
	* tools/bidiff.cc (options::show_redundant_changes): New data
	member.
	(options::options): Initialize it.
	(display_usage): Add help string for the --redundant command line
	option.
	(parse_command_line): Add support for the --redundant command line
	option.
	(set_diff_context_from_opts): Take the --redundant command line
	option in account.
	* tests/test-diff-filter.cc: Update this to add new test inputs.
	* tests/data/test-diff-filter/test14-0-report.txt: New test input.
	* tests/data/test-diff-filter/test14-1-report.txt: Likewise.
	* tests/data/test-diff-filter/test14-v0.cc: Likewise.
	* tests/data/test-diff-filter/test14-v0.o: Likewise.
	* tests/data/test-diff-filter/test14-v1.cc: Likewise.
	* tests/data/test-diff-filter/test14-v1.o: Likewise.
	* tests/data/test-diff-filter/test15-0-report.txt: Likewise.
	* tests/data/test-diff-filter/test15-1-report.txt: Likewise.
	* tests/data/test-diff-filter/test15-v0.cc: Likewise.
	* tests/data/test-diff-filter/test15-v0.o: Likewise.
	* tests/data/test-diff-filter/test15-v1.cc: Likewise.
	* tests/data/test-diff-filter/test15-v1.o: Likewise.
	* tests/Makefile.am: Add the above to the build system.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2014-06-23 13:43:42 +02:00
Dodji Seketeli
f3f2382396 Update copyright notice for a bunch of files
* include/abg-comp-filter.h: Update copyright notice.
	* include/abg-comparison.h: Likewise.
	* src/abg-comparison.cc: Likewise.
	* src/abg-ir.cc: Likewise.
	* tools/bidiff.cc: Likewise.
	* tests/test-diff-filter.cc: Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2014-06-23 13:43:07 +02:00
Dodji Seketeli
802f7cbc92 Initial implementation of diff tree node filtering
* include/abg-comp-filter.h: New file.
	* include/Makefile.am: Add the new include/abg-comp-filter.h to
	the source distribution.
	* include/abg-comparison.h (enum visiting_kind, diff_category): New enums.
	(operator|): Declare new operator declaration for the new
	visiting_kind enum.
	(operator{|,^,&,~}): Declare new operator decl for the new
	diff_category enum.
	(diff_context::{get_allowed_category, set_allowed_category,
	switch_categories_on, switch_categories_off, diff_filters,
	add_diff_filter, maybe_apply_filters}): Declare new member functions.
	(diff::{parent_, category_}): New members.
	(diff::diff): Adjust.
	(diff::{get_parent, set_parent, get_category, add_to_category,
	is_filtered_out, to_be_reported}):  New members.
	(diff_node_visitor::{get_visiting_kind, set_visiting_kind}): New
	members.
	(diff_node_visitor::visit): Add a new flag to saying if the
	visitor is being called in post or pre children traversing mode.
	* src/abg-comparison.cc (operator|): Declare new operator
	declaration for the new visiting_kind enum.
	(operator{|,^,&,~}): Declare new operator decl for the new
	diff_category enum.
	(diff_context::priv::{allowed_category_, filters_}): New members.
	(diff_context::diff_context): Add all known filters.
	(diff_context::{get_allowed_category, set_allowed_category,
	switch_categories_on, switch_categories_off, diff_filters,
	add_diff_filter, maybe_apply_filters}): Define new member
	functions.
	(diff::{is_filtered_out, to_be_reported}): Define new members.
	(*::report): Use the new diff::to_be_reported function.
	(*::traverse): Adjust for pre/post visiting.
	(var_diff::var_diff): Chain the type diff node to its parent.
	({pointer_diff, reference_diff, qualified_type_diff,
	typedef_diff}::underlying_type_diff): Chain the underlying type
	diff node to its parent.
	(enum_diff::enum_diff): Likewise.
	(base_diff::underlying_class_diff): Likewise.
	({class_diff, corpus_diff}::report): Do not report changed
	(member) functions that have been filtered out.  Rather report
	that they have been filtered out.
	({function_decl_diff, class_diff}::traverse): Chain underlying
	type diff nodes to their parent.
	(diff_node_visitor::visit): Add a new flag to saying if the
	visitor is being called in post or pre children traversing mode.
	Make sure to call the default diff::visit overload.
	* src/abg-comp-filter.cc: New file.
	* src/Makefile.am: Add the new abg-comp-filter.cc to the source
	distribution.
	* tools/bidiff.cc (options::show_harm{ful,less}_changes): New
	members.
	(display_usage): Add usage strings for --no-harmless and
	--no-harmful options.
	(parse_command_line): Parse --no-harmless and --no-harmful command
	line options.
	(set_diff_context_from_opts): Populate the diff context
	accordingly.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2014-03-27 13:01:18 +01:00