Commit Graph

599 Commits

Author SHA1 Message Date
Matthias Maennich
389c1e3385 cxx-compat: add test suite for cxx-compat
This is an empty test to begin with and its sole purpose for now is to
make sure abg-cxx-compat.h can be compiled on its own.

	* tests/Makefile.am: Add new test case runtestcxxcompat.
	* tests/test-cxx-compat.cc: New test source file.

Reviewed-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Matthias Maennich <maennich@google.com>
2020-05-13 11:55:12 +02:00
Matthias Maennich
6229bb850f tests/.gitignore: ignore all files starting with runtest*
We only have runtest* executables that we can ignore there with this
prefix. Hence, stop adding each and every test and use an exclusion
pattern instead.

	* tests/.gitignore: gitignore all files named runtest*

Reviewed-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Matthias Maennich <maennich@google.com>
2020-05-13 11:26:28 +02:00
Giuliano Procida
dc0ac49157 Tidy checks for sufficient suppression properties.
Each suppression specification must have at least one of a documented
per-suppression type list of properties defined if it is to be
considered at all.

At present:

- suppression specifications which fail the check are silently ignored
- in the function suppression case, the check does not trigger an
  early return and risks a later null pointer dereference.

This commit:

- reimplements the checks using arrays and helper function calls
- adds a helper function to determine if a suppression specification
  is going to be ignored due a lack of properties
- makes the parsing functions return failure early if the check fails

Inconsistencies between suppression types (in particular, the
treatment of the "label" property) remain.

The only behavioural change may be to how present but empty properties
are handled. This is an edge case that may be worth revisiting in a
more general fashion in a later commit.

	* src/abg-suppression.cc (check_sufficient_props): New helper
	function to check for sufficient properties in a section.
	(read_type_suppression): Replace conditional logic with call
	to check_sufficient_props.
	(read_function_suppression): Ditto.
	(read_variable_suppression): Ditto.
	(read_file_suppression): Ditto.
	* tests/data/test-diff-suppr/test15-suppr-added-fn-4.suppr:
	Explain why the suppression will be ignored.
	* tests/data/test-diff-suppr/test16-suppr-removed-fn-4.suppr:
	Ditto.
	* tests/data/test-diff-suppr/test17-suppr-added-var-4.suppr:
	Ditto.
	* tests/data/test-diff-suppr/test18-suppr-removed-var-4.suppr:
	Ditto.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-05-12 18:08:33 +02:00
Matthias Maennich
246ca20049 corpus/writer: sort emitted translation units by path name
By sorting the corpora output, we achieve determinism for the emitted
XML file across multiple runs of abidw.

For that to happen, change the collection of translation units to a
std::set (instead of std::vector), sorted by absolute path name.

Test data needed adjustments, but changes are fully compatible.

	* include/abg-fwd.h: remove translation_units fwd declaration.
	* include/abg-ir.h (struct shared_translation_unit_comparator):
	Define new class.
	(translation_units): Define new typedef.
	* src/abg-corpus.cc (corpus::add): do checked insert into the
	translation_units set (rather than vector::push_back)
	* tests/data/test-annotate/test13-pr18894.so.abi: Adjust test data.
	* tests/data/test-annotate/test14-pr18893.so.abi: Likewise.
	* tests/data/test-annotate/test15-pr18892.so.abi: Likewise.
	* tests/data/test-annotate/test17-pr19027.so.abi: Likewise.
	* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise.
	* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise.
	* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise.
	* tests/data/test-annotate/test21-pr19092.so.abi: Likewise.

Signed-off-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-05-05 13:09:34 +02:00
Matthias Maennich
0b9560c19b tests: reorder test execution to optimize 'make check' runtime
Reorder the test definition to first start expensive tests and later on
start tests with short runtime. This optimizes the 'make check' runtime
by starting the tests on the critical path.

	* tests/Makefile.am(TESTS): split up in expensive and non
	expensive tests, sort the expensive ones by runime, the cheap
	ones alphabetically

Signed-off-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-05-04 15:53:45 +02:00
Matthias Maennich
c8b0327e50 test-types-stability: parallelize test case alternatives
Commit a9f5fb4089 ("Add --no-write-default-sizes option.") introduced
a new test variant for test-types-stability that is actually independent
of the original test case in terms of execution. Hence it can be
expressed as a separate test case. So, do that by parametrizing the
test_task struct with a new no_default_sizes flag and schedule a
separate test case in the main loop.

That test runs now ~twice as fast dropping from roughly 20s on my
machine to 10s. That effectively removes it from the critical path of
make check, which is now back to about 15s on my machine with my
configuration.

	* tests/test-types-stability.cc (test_task): add field no_default_sizes
	(test_task::perform) Switch on the new flag to test a different
	behaviour.
	(main): Schedule an additional test case to test with the new flag.

Cc: Mark Wielaard <mark@klomp.org>
Signed-off-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-05-04 15:34:42 +02:00
Giuliano Procida
186cc9ed3a Simplify generation of symbol whitelist regex.
The code to build the symbol whitelist regex uses things like seekp
and tellp to generate regexes like "^foo$|^bar$".

This patch simplifies the code, for further enhancement, resulting in
generated regexes like "^(foo|bar)$".

There should be no change in behaviour, unless whitelisted symbol
names contain special regex characters.

	* include/abg-regex.h (generate_from_strings): Declare new
	function to build a regex from some strings, representing a
	membership test.
	* src/abg-regex.cc (generate_from_strings): Implement new
	function to build a regex from some strings, representing a
	membership test, in a straightfoward fashion.
	* src/abg-tools-utils.cc
	(gen_suppr_spec_from_kernel_abi_whitelists): Replace
	regex-building code with a call to generate_from_strings.
	* tests/test-kmi-whitelist.cc: Update regexes in test.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-05-04 11:17:49 +02:00
Mark Wielaard
a9f5fb4089 Add --no-write-default-sizes option.
abidw will write out the exact same size-in-bits address for every
pointer type, reference type, function declaration and function type
even though it is always the same as the translation unit address
size. When giving the --no-write-default-sizes option these aren't
written out anymore. The reader is updated to set the default size
when none is given in the XML description.

Even though size and alignment are handled together in the reader,
default alignment is still set to zero, following commit a05384675

Note that this isn't backward compatible with older libabigail
readers, which will set the size to zero when none is given. So this
option isn't the default.

	* doc/manuals/abidw.rst: Document --no-write-default-sizes.
	* include/abg-writer.h (set_write_default_sizes): New function
	declaration.
	(set_common_options): Call set_write_default_sizes.
	* src/abg-reader.cc (build_function_decl): Get default size.
	(build_pointer_type_def): Likewise.
	(build_reference_type_def): Likewise.
	(build_function_type): Likewise.
	* src/abg-writer.cc (write_context): Add m_write_default_sizes
	bool.
	(get_write_default_sizes): New method.
	(set_write_default_sizes): Likewise.
	(write_size_and_alignment): Add default size and alignment
	parameters.
	(set_write_default_sizes): New function.
	(write_type_decl): Set default size and alignment.
	(write_pointer_type_def): Likewise.
	(write_reference_type_def): Likewise.
	(write_function_decl): Likewise.
	(write_function_type): Likewise.
	(write_class_decl_opening_tag): Likewise.
	(write_union_decl_opening_tag): Likewise.
	* tests/test-types-stability.cc (perform): Also test --abidiff
	with --no-write-default-sizes.
	* tools/abidw.cc (option): Add default_sizes bool.
	(parse_command_line): Parse --no-write-default-sizes.
	(display_usage): Add doc string for --no-write-default-sizes.

Signed-off-by: Mark Wielaard <mark@klomp.org>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-04-29 11:56:40 +02:00
Giuliano Procida
4e11174eae abidiff: Omit declaration-only type size 0 diffs.
When reporting declaration-only type changes, the size is reported as
changing to or from 0, which is not correct. The declaration type is
of unknown size. This patch eliminates such size reports.

	* src/abg-reporter-priv.cc (report_size_and_alignment_changes):
	Filter out declaration-only / defined type size changes
	unconditionally.
	* tests/data/test-abidiff-exit/test-decl-struct-report.txt:
	Update test.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-04-27 13:12:48 +02:00
Giuliano Procida
39ab7e8b22 abidiff: Blank line after declaration-only diff.
In the case where a type change is summarised as declaration-only to
defined or vice versa, the trailing new line was missing.

	* src/abg-default-reporter.cc (default_reporter::report): In
	the class_or_union_diff overload, emit a new line at the end
	of the declaration-only reporting path.
	* tests/data/test-abidiff-exit/test-decl-struct-report.txt:
	Add missing blank lines.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-04-27 13:02:02 +02:00
Giuliano Procida
bddb6b7d09 Add tests for declaration-only struct diffs.
There were no tests exercising this reporting path. The new test is
run with --harmless as these changes are considered harmless.

	* tests/data/Makefile.am: Add new test case files.
	* tests/data/test-abidiff-exit/test-decl-struct-report.txt:
	New test case generating "declaration-only" output.
	* tests/data/test-abidiff-exit/test-decl-struct-v0.c: Ditto.
	* tests/data/test-abidiff-exit/test-decl-struct-v0.o: Ditto.
	* tests/data/test-abidiff-exit/test-decl-struct-v1.c: Ditto.
	* tests/data/test-abidiff-exit/test-decl-struct-v1.o: Ditto.
	* tests/test-abidiff-exit.cc: Run new test case.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-04-27 12:48:58 +02:00
Giuliano Procida
037b5a1ca6 test24-soname-suppr*txt: Fix suppression syntax.
The property name "reached_through" should have been
"accessed_through" and the property value "reference_or_pointer"
should have been "reference-or-pointer".

This patch fixes these issues.

	* tests/data/test-diff-suppr/test24-soname-suppr-0.txt: Fix
        typo, change "reached_through" to "accessed_through".
	* tests/data/test-diff-suppr/test24-soname-suppr-1.txt: Ditto.
	* tests/data/test-diff-suppr/test24-soname-suppr-2.txt: Ditto.
	* tests/data/test-diff-suppr/test24-soname-suppr-3.txt: Ditto.
	* tests/data/test-diff-suppr/test24-soname-suppr-4.txt: Ditto.

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-04-22 17:05:01 +02:00
Giuliano Procida
086083734e test35-leaf.suppr: fix regex typo.
Bad regexes are silently ignored. This will be fixed in a later
commit.

	* tests/data/test-diff-suppr/test35-leaf.suppr: Fix typo in
	regex, "*." -> ".*".

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-04-22 15:53:19 +02:00
Matthias Maennich
b4d97cdd0f abg-dwarf-reader split: create abg-elf-helpers.{h,cc} and test case
abg-elf-helpers.{h,cc} shall contain the ELF related parts of the
abg-dwarf-reader. Create the stub files, an empty unit test and hook
everything up in the make system.

	* src/Makefile.am: Add new source files abg-elf-helpers.{h,cc}.
	* src/abg-elf-helpers.cc: New source file.
	* src/abg-elf-helpers.h: New header file.
	* tests/.gitignore: Exclude runtestelfhelpers from being committed.
	* tests/Makefile.am: Add new test case runtestelfhelpers.
	* tests/test-elf-helpers.cc: New test source file.

Reviewed-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-04-22 11:39:45 +02:00
Matthias Maennich
56302e9c52 tests: parallelize diff-suppr test
Parallelize test execution for diff-suppr test by using abigail's multi
threaded worker queue. To accomplish that, follow a similar pattern like
other tests: Add a test_task struct with some precomputed values and
chunk up the work in main().

The test cases needed to be adjusted to allow parallel execution. In
particular the output files can't be shared anymore. To ensure this, a
small tests has been added that checks for unique output paths.

Finally, one redundant test case has been dropped.

This reduces the test time on my machine from ~31s to ~14s. There are
still two test cases that dominate the overall runtime. Since this test
is on the critical path for 'distcheck' (it is the test with the longest
runtime), this is effectively chopped off the overal make distcheck
time.

	* tests/test-diff-suppr.cc(main): parallelize test execution.
	(test_task) new abigail::workers::task implementation to run
	test cases in this test as separate worker tasks.

Signed-off-by: Matthias Maennich <maennich@google.com>
2020-04-20 15:02:39 +02:00
Giuliano Procida
b4f1a796bd abidiff: Document and refresh tests.
Following on from giving some test file more descriptive names, this
patch actually documents in brief the purpose of these recently added
tests.

It also improves the coverage of the test-leaf-cxx-members test to
include deletion and insertion report text as well.

	* tests/data/test-abidiff-exit/test-leaf-cxx-members-v1.cc:
	Comment test. Reorder members of ops to get better coverage.
	* tests/data/test-abidiff-exit/test-leaf-fun-type-v1.cc:
	Comment test.
	* tests/data/test-abidiff-exit/test-leaf-more-v1.cc: Ditto.
	* tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc:
	Comment test. Update comment on ops2.
	* tests/data/test-abidiff-exit/test-leaf-redundant-v1.c:
	Comment test.
	* tests/data/test-abidiff-exit/test-leaf-stats-v1.cc: Ditto.
	* tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt:
	Update locations. Update report to reflect deletion,
	insertion and changed members (was previously changed only).:
	* tests/data/test-abidiff-exit/test-leaf-peeling-report.txt:
	Update locations.
	* tests/data/test-abidiff-exit/test-leaf-redundant-report.txt:
	Ditto.:
	* tests/data/test-abidiff-exit/test-leaf-cxx-members-v0.cc:
	Added one line comment referring to -v1 source file.
	* tests/data/test-abidiff-exit/test-leaf-fun-type-v0.cc: Ditto.
	* tests/data/test-abidiff-exit/test-leaf-more-v0.cc: Ditto.
	* tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc: Ditto.
	* tests/data/test-abidiff-exit/test-leaf-redundant-v0.c: Ditto.
	* tests/data/test-abidiff-exit/test-leaf-stats-v0.cc: Ditto.
	* tests/data/test-abidiff-exit/test-leaf-cxx-members-v0.o: Recompiled.
	* tests/data/test-abidiff-exit/test-leaf-cxx-members-v1.o: Ditto.
	* tests/data/test-abidiff-exit/test-leaf-fun-type-v0.o: Ditto.
	* tests/data/test-abidiff-exit/test-leaf-fun-type-v1.o: Ditto.
	* tests/data/test-abidiff-exit/test-leaf-more-v0.o: Ditto.
	* tests/data/test-abidiff-exit/test-leaf-more-v1.o: Ditto.
	* tests/data/test-abidiff-exit/test-leaf-peeling-v0.o: Ditto.
	* tests/data/test-abidiff-exit/test-leaf-peeling-v1.o: Ditto.
	* tests/data/test-abidiff-exit/test-leaf-redundant-v0.o: Ditto.
	* tests/data/test-abidiff-exit/test-leaf-redundant-v1.o: Ditto.
	* tests/data/test-abidiff-exit/test-leaf-stats-v0.o: Ditto.
	* tests/data/test-abidiff-exit/test-leaf-stats-v1.o: Ditto.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-04-14 13:23:44 +02:00
Giuliano Procida
4bbe135690 abg-reporter-priv.cc: Fix anonymous member size change reports.
Data members have names (unless anonymous), types, sizes and
offsets (if a member of a class or struct). It is the responsibility
of the represent function to detail all differences of a changed
member. The size of a member is the same as the size of its type.

A recent change to the function focused on cleaning up the formatting
logic and added a catch-all case to the end of it in case no diff
information had been emitted by then.

This catch-all triggered for anonymous structs and union diffs under
certain circumstances when these anonymous members had changed in
size.

This patch ensures size change information for a member is emitted
exactly once (unless the type change has been or is being reported
already). It also ensures anonymous members are identified
appropriately and includes a test case would otherwise reach the
catch-all case in both default and --leaf-changes-only modes.

	* src/abg-reporter-priv.cc (represent): In the var_diff_sptr
	overload, factor out some expressions as local variables, rely
	on diff_to_be_reported to decide whether to emit a change,
	fold together local/non-local change reporting using
	local_changes to preserve current name formatting differences,
	keep track explicitly of whether size information has been
	emitted and ensure it happens if needed, make offset and size
	change reporting for anonymous data members more meaningful.
	* tests/test-abidiff-exit.cc: Run new test cases.
	* tests/data/Makefile.am: Add new test files.
	* tests/data/test-abidiff-exit/test-member-size-v0.cc: New
	test.
	* tests/data/test-abidiff-exit/test-member-size-v0.o: Ditto.
	* tests/data/test-abidiff-exit/test-member-size-v1.cc: Ditto.
	* tests/data/test-abidiff-exit/test-member-size-v1.o: Ditto.
	* tests/data/test-abidiff-exit/test-member-size-report0.txt:
	New test, default mode.
	* tests/data/test-abidiff-exit/test-member-size-report1.txt:
	New test, --leaf-changes-only mode.
	* tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt:
	Eliminate duplicate reporting of member sizes.
	* tests/data/test-abidiff-exit/test-leaf-more-report.txt: Ditto.
	* tests/data/test-abidiff-exit/test-leaf-peeling-report.txt:
	Ditto.
	* tests/data/test-abidiff-exit/test-no-stray-comma-report.txt:
	Ditto.
	* tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt:
	Add size report for anonymous data member.
	* tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt:
	Ditto.
	* tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt:
	Add missing size change report.
	* tests/data/test-diff-suppr/test36-leaf-report-0.txt: Remove
	size change report for previously reported type.
	* tests/data/test-diff-suppr/test46-PR25128-report-1.txt:
	Eliminate duplicate reporting of member size change.
	* tests/data/test-diff-suppr/test46-PR25128-report-2.txt:
	Ditto.

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-04-14 12:39:59 +02:00
Giuliano Procida
22b2ba96f5 abidiff: More compact references to prior diffs.
In both the default and leaf reporting modes, when there is a repeated
or circular reference to a type difference of a member variable, some
placeholder text is emitted instead.

In the leaf reporter:

  type 'struct S' of 'foo::bar' changed as reported earlier

In the default reporter, this spans two lines:

  type of 'S foo::bar' changed:
    details were reported earlier

This patch changes the latter to the more compact:

  type of 'S foo::bar' changed, as reported earlier

More generally, this patch makes the punctuation of such placeholder
text more consistent with a comma separating the phrases in all cases.

It doesn't attempt to reconcile the different formatting of member
variable declarations between the two modes.

	* src/abg-reporter-priv.cc (represent): In the var_diff_sptr
	overload, use consistent punctuation and keep to a single line
	of output when referring back to an existing type diff report.
	Remove unnecessary braces around single line conditional
	blocks.
	* src/abg-reporter-priv.h: In the macro
	RETURN_IF_BEING_REPORTED_OR_WAS_REPORTED_EARLIER, use
	consistent punctuation when referring back to an existing type
	diff report.
	* tests/data/test-abidiff/test-PR18791-report0.txt: Adjust
	formatting of back references to existing type diff reports.
	* tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt:
	Ditto.
	* tests/data/test-diff-filter/test16-report-2.txt: Ditto.
	* tests/data/test-diff-filter/test17-1-report.txt: Ditto.
	* tests/data/test-diff-filter/test25-cyclic-type-report-1.txt:
	Ditto.
	* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
	Ditto.
	* tests/data/test-diff-suppr/test36-leaf-report-0.txt: Ditto.

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-04-14 11:47:10 +02:00
Giuliano Procida
8d559fb639 test-abidiff-exit.cc: Drop redundant --redundant.
* tests/test-abidiff-exit.cc: Drop obsolete --redundant flag
	and comment as --redundant is now implied by
	--leaf-changes-only.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-04-07 14:49:10 +02:00
Giuliano Procida
7e0d52834b Rename test-abidiff-exit/test-leaf[0-3] files.
The files tests/data/test-abidiff-exit/test-leaf[0-3]-* were
introduced in a series of changes. Numbering rather than naming the
tests turned out to be a bad choice: it caused confusion when
dealing with merge conflicts due to reordering of commits.

This patch renames the tests to give them more descriptive names,
which is good practice anyway.

	* tests/data/Makefile.am: Rename test files.
	* tests/test-abidiff-exit.cc: Rename test files.
	* tests/data/test-abidiff-exit/test-leaf-fun-type-report.txt:
	Renamed from test-leaf2-report.txt.
	* tests/data/test-abidiff-exit/test-leaf-fun-type-v0.cc:
	Renamed from test-leaf2-v0.cc.
	* tests/data/test-abidiff-exit/test-leaf-fun-type-v0.o:
	Renamed from test-leaf2-v0.o.
	* tests/data/test-abidiff-exit/test-leaf-fun-type-v1.cc:
	Renamed from test-leaf2-v1.cc.
	* tests/data/test-abidiff-exit/test-leaf-fun-type-v1.o:
	Renamed from test-leaf2-v1.o.
	* tests/data/test-abidiff-exit/test-leaf-more-report.txt:
	Renamed from test-leaf1-report.txt.
	* tests/data/test-abidiff-exit/test-leaf-more-v0.cc: Renamed
	from test-leaf1-v0.cc.
	* tests/data/test-abidiff-exit/test-leaf-more-v0.o: Renamed
	from test-leaf1-v0.o.
	* tests/data/test-abidiff-exit/test-leaf-more-v1.cc: Renamed
	from test-leaf1-v1.cc.
	* tests/data/test-abidiff-exit/test-leaf-more-v1.o: Renamed
	from test-leaf1-v1.o.
	* tests/data/test-abidiff-exit/test-leaf-redundant-report.txt:
	Renamed from test-leaf3-report.txt.
	* tests/data/test-abidiff-exit/test-leaf-redundant-v0.c:
	Renamed from test-leaf3-v0.c.
	* tests/data/test-abidiff-exit/test-leaf-redundant-v0.o:
	Renamed from test-leaf3-v0.o.
	* tests/data/test-abidiff-exit/test-leaf-redundant-v1.c:
	Renamed from test-leaf3-v1.c.
	* tests/data/test-abidiff-exit/test-leaf-redundant-v1.o:
	Renamed from test-leaf3-v1.o.
	* tests/data/test-abidiff-exit/test-leaf-stats-report.txt:
	Renamed from test-leaf0-report.txt.
	* tests/data/test-abidiff-exit/test-leaf-stats-v0.cc: Renamed
	from test-leaf0-v0.cc.
	* tests/data/test-abidiff-exit/test-leaf-stats-v0.o: Renamed
	from test-leaf0-v0.o.
	* tests/data/test-abidiff-exit/test-leaf-stats-v1.cc: Renamed
	from test-leaf0-v1.cc.
	* tests/data/test-abidiff-exit/test-leaf-stats-v1.o: Renamed
	from test-leaf0-v1.o.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-04-07 14:48:58 +02:00
Giuliano Procida
94844853fb abidiffpkg: Remove stray test report file.
The file tests/data/test-diff-pkg/dirpkg-2-report-1.txt looks like it
was committed in error. It's not used, its contents don't match the
implied directories being compared and there are other tests that
cover the same functionality, with or without --no-abignore, anyway.

	* tests/data/Makefile.am: Removed
	tests/data/test-diff-pkg/dirpkg-2-report-1.txt.
	* tests/data/test-diff-pkg/dirpkg-2-report-1.txt: Removed.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-04-01 19:24:11 +02:00
Giuliano Procida
609dc8813d test-diff-suppr.cc: Add missing tests.
Commit 79383f937c added many tests but
didn't actually execute three of them.

Commit fe9fa7a05f added many tests but
didn't actually execute one of them.

This patch corrects these issues.

	* tests/test-diff-suppr.cc: Add stanzas for
	test6-fn-suppr-report-4, test16-suppr-removed-fn-report-5 and
	test22-suppr-removed-var-sym-report-5 and
	test23-alias-filter-report-4 tests.
	* tests/data/test-diff-suppr/test6-fn-suppr-report-4.txt:
	Number parameters from 1 and update expected output to current
	formatting.
	* tests/data/test-diff-suppr/test16-suppr-removed-fn-report-5.txt:
	Update expected output to current formatting.
	* tests/data/test-diff-suppr/test22-suppr-removed-var-sym-report-5.txt:
	Update expected output to current formatting.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-04-01 18:38:23 +02:00
Giuliano Procida
ef90972a4a abidiff: Remove blank line after typedef changes.
This patch removes perhaps the last remaining cause of double blank
lines in output.

The state variable emit_nl was being set to true just after emitting a
new line which resulted in a blank line after every local typedef
change report.

	* include/abg-reporter.h
	(default_reporter::report_local_typedef_changes): Change
	return type to void.
	* src/abg-default-reporter.cc:
	(default_reporter::report_local_typedef_changes): Change
	return type to void, remove emit_nl state variable and logic.
	* tests/data/test-abidiff/test-PR18791-report0.txt: Remove
	blank lines.
	* tests/data/test-diff-dwarf/test42-PR21296-clanggcc-report0.txt:
	Ditto.
	* tests/data/test-diff-suppr/test39-opaque-type-report-0.txt:
	Ditto.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-04-01 15:54:02 +02:00
Giuliano Procida
2d5389f265 Fix size calculations for multidimensional arrays.
The code in abg-ir.cc that calculated the memory size of an array
summed, rather than multiplied, the dimensions. It also did duplicate
work for each dimension after the first.

Existing code in abg-reader.cc asserted that array size information
read from XML match freshly calculated values.

This patch corrects the calculation, eliminates the duplicate work and
updates the XML reader validation to just emit a warning if old bad
array size information is found.

	* include/abg-ir.h (array_type_def::append_subrange): Remove
	this function.
	* src/abg-ir.cc (array_type_def::set_element_type): Add a note
	about safe usage.
	(array_type_def::append_subrange): Inline this function into
	its only caller append_subranges and remove it.
	(array_type_def::append_subranges): Do correct multiplicative
	calculation of multidimensional array sizes.
	* src/abg-reader.cc (build_array_type_def): When checking
	calculated against read array sizes, warn once if value
	matches old behaviour rather than raising an assertion.
	Otherwise, before raising an assertion, emit an informative
	error message.
	* tests/data/test-annotate/test14-pr18893.so.abi: Correct
	array sizes.
	* tests/data/test-annotate/test17-pr19027.so.abi: Ditto.
	* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
	Ditto.
	* tests/data/test-annotate/test7.so.abi: Ditto.
	* tests/data/test-diff-dwarf/test10-report.txt: Ditto.
	* tests/data/test-diff-dwarf/test11-report.txt: Ditto.
	* tests/data/test-read-write/test25.xml: Ditto.

Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-03-31 19:48:35 +02:00
Giuliano Procida
88c90d14d9 abidiff: Remove new lines after parameter diffs.
These blank lines break up the output unexpectedly and often cause
double blank lines after reporting of function changes.

	* src/abg-default-reporter.cc: (report_local_function_type_changes):
	Remove unnecessary blank lines after lists of parameter changes.
	* tests/data/test-*/test*report*.txt: Remove blank lines after
	parameter change lists in 12 files.

Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-03-30 18:53:07 +02:00
Giuliano Procida
10b93d084c abidiff: Eliminate leaf mode double blank lines.
In --leaf-changes-only mode, the report currently has 2 blank lines
after each type-diff section. This patches changes this to 1.

	* src/abg-leaf-reporter.cc: (report_diffs) Emit 1 instead of 2
	new lines between sections.
	* tests/data/test-abidiff-exit/test-leaf-peeling-report.txt:
	Replace double blank lines with single ones.
	* tests/data/test-diff-filter/libtest45-basic-type-change-report-1.txt:
	Ditto.
	* tests/data/test-diff-suppr/test36-leaf-report-0.txt: Ditto.
	* tests/data/test-*/test*report*.txt:: Ditto.

Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-03-30 17:35:59 +02:00
Giuliano Procida
7070d3b65a abidiff: Fix variable declaration formatting.
The represent(var_diff_sptr) function tracks vertical "\n" and
horizontal ", " spacing using two state variables:

    - emitted - the main diff text has been emitted
    - begin_with_and - main text emitted && new line started, so
      any further description must be indented and prefixed "and ".
    - if emitted is true and begin_with_and is false, then further
      description is prefixed with ", " instead.

There was some inconsistent emission of new lines and maintenance of
the state variables. This patch documents the variables and makes sure
new lines and state variables are kept in sync.

Finally, it is theoretically possible to reach the end of the function
without emitted becoming true. This patch adds a TODO and makes sure
at least something is output should that ever happen.

	* src/abg-reporter-priv.cc: (represent) In the var_diff_sptr
	overload, make sure the state variables begin_with_and and
	emitted are updated consistently; add a TODO for one case
	which may result in the end of the function being reached
	without having emitted a report; add missing new lines
	following reporting of anonymous member changes; only emit a
	final new line if begin_with_and hasn't tracked one already;
	document state variables.
	* tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt:
	Add missing blank line after anonymous member change text.
	* tests/data/test-diff-filter/test44-anonymous-data-member-report-1.txt:
	Add missing "and " continuation.

Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-03-30 16:56:22 +02:00
Giuliano Procida
786fcba8ae abidiff: Remove member function diff blank lines.
Currently, lists of member function deletions, additions and changes
are each followed by a blank line in abidiff output.

While this formatting works reasonably well in leaf-changes-only mode
for top-level changes to types, it is inconsistent with everthing
else. In particular, when reporting member function diffs in default
mode, or more deeply nested in leaf mode, the blank lines are
jarring.

There was also no test coverage for leaf-changes-only mode.

This change removes these blank lines and associated state variables
and logic. It adds a test case for leaf-changes-only mode.

Note that the patch also modifies default mode reporting of member
types, template member functions and template member classes
analogously, but I wasn't able to exercise those code paths.

	* src/abg-default-reporter.cc (report): In the
	class_or_union_diff overload, don't emit a new line after each
	list of member function, member type, template member
	function and template member class changes.
	* src/abg-leaf-reporter.cc (report): : In the
	class_or_union_diff overload, don't emit a new line after each
	list of member function changes.
	* tests/data/Makefile.am: Add new test case files.
	* tests/data/test-abidiff-exit/test-leaf-cxx-members-v0.cc:
	New test case for --leaf-changes-only member function diffs.
	* tests/data/test-abidiff-exit/test-leaf-cxx-members-v0.o:
	Ditto.
	* tests/data/test-abidiff-exit/test-leaf-cxx-members-v1.cc:
	Ditto. Also add a TODO regarding a potentially bad diff.
	* tests/data/test-abidiff-exit/test-leaf-cxx-members-v1.o:
	Ditto.
	* tests/data/test-abidiff/test-struct1-report.txt: Remove
	blank lines after member function changes lists.
	* tests/data/test-diff-dwarf/test41-PR20476-hidden-report-0.txt:
	Ditto.
	* tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt:
	Ditto.
	* tests/test-abidiff-exit.cc: Add new test case.

Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-03-30 16:36:13 +02:00
Giuliano Procida
340b53300e abidiff: Fix enum impacted interfaces blank line.
There was a spurious new line at the end of reporting enum diffs,
before reporting impacted interfaces, if requested.

	* src/abg-default-reporter.cc (report): In the enum_diff
	overload, don't emit a blank line before a possible "impacted
	interfaces" stanza. In the class_or_union overload, change a
	stray conditional to use the emitted state variable for
	consistency.
	* tests/data/test*report*.txt: Remove blank lines after enum
	diffs in 17 files.

Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-03-30 16:27:13 +02:00
Giuliano Procida
912c436250 abidiff: Remove blank line after base class diffs.
This patch removes the blank line emitted after base class diffs.
These are jarring, particularly when the diffs are nested.

	* src/abg-default-reporter.cc (report): In the class_diff
	overload, eliminate the extra blank line after base class
	changes and remove unneeded new line logic.
	* tests/data/test*report*.txt: Remove blank lines after base
	class diffs in 9 files.

Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-03-30 16:26:45 +02:00
Giuliano Procida
a5a67c2ffd abidiff: Clean up new lines between sections.
Some blank lines were unintentionally removed with a recent patch. The
new line to remove should have been the one after all changed
functions not the one after each one. Changed functions (and
variables) tend to be reported as paragraphs, rather than single
lines, so the extra spacing is useful.

This patch removes the blank line emitted after all changed
functions, variables and unreachable types, and adds back the
missing blank lines between changed functions.

	* src/abg-default-reporter.cc (report): In the corpus_diff
	override, add back the extra blank line per changed function
	but remove the extra one after all changed changed functions
	and variables; comment these.
	* src/abg-leaf-reporter.cc (report): In the corpus_diff
	override, add back the extra blank line per changed function
	but remove the extra one after all changed changed functions
	and variables; comment these.
	* src/abg-reporter-priv.cc
	(maybe_report_unreachable_type_changes): Remove extra blank
	line emitted after all unreachable type changes; comment
	this.
	* tests/data/test*report*.txt: Remove/add blank lines.

Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-03-30 16:26:31 +02:00
Giuliano Procida
95868e0c42 abg-ir.cc: Add types_have_similar_structure tests.
This is a follow-up to a recent commit. This patch adds more tests, as
suggested by a reviewer.

	* src/abg-ir.cc (types_have_similar_structure): Update TODO
	regarding structure of arrays - multidimensional arrays are
	the issue.
	* tests/data/test-abidiff-exit/test-leaf-peeling-report.txt:
	Updated following changes.
	* tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc: Add
	more cases (see below).
	* tests/data/test-abidiff-exit/test-leaf-peeling-v0.o:
	Updated.
	* tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc: Add
	comment about a potential change to local-change semantics;
	add test cases to demonstrate that * and & and * and *** are
	structurally different; add a TODO regarding multidimensional
	arrays where changes are sometimes missed in leaf mode.
	* tests/data/test-abidiff-exit/test-leaf-peeling-v1.o

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-03-30 12:35:05 +02:00
Dodji Seketeli
4d49e12833 Update tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
Following commit 474ad38f, we need to update the test reference output
tests/data/test-abidiff-exit/test-leaf-peeling-report.txt.

	* tests/data/test-abidiff-exit/test-leaf-peeling-report.txt:
          Update output.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-03-26 15:22:53 +01:00
Giuliano Procida
474ad38f37 abidiff: Remove some more unnecessary blank lines.
This is another round of moving responsibility for generation of new
lines into functions  and getting rid of redundant new lines.

	* src/abg-default-reporter.cc (report) In the
	class_or_union_diff overload, don't emit a new line after
	calls to represent. In the union_diff overload, emit a new
	line after a from/to change; fix indentation. In the
	corpus_diff overload, don't emit an extra new line after
	reporting a diff.
	* src/abg-leaf-reporter.cc (report_diffs) Don't emit a new
	line after reporting a canonical diff. In the
	class_or_union_diff overload, don't emit a new line after
	calls to represent. In the corpus_diff overload, don't emit an
	extra new line after reporting a diff.
	* src/abg-reporter-priv.cc (represent): Emit a final new line,
	but only if needed.
	(maybe_report_interfaces_impacted_by_diff): Emit a new line
	after the last impacted interface.
	* tests/data/test-*/*report*.txt: Remove blank lines (and add
	a missing one) to 77 test cases.
2020-03-26 14:39:35 +01:00
Giuliano Procida
9cf76b1175 abg-ir.cc: Improve types_have_similar_structure.
This function is used to determine whether or not type differences are
"local" and is primarily used in --leaf-changes-only mode. The logic
has some issues which are addressed by this patch:

    - Any number of points-to (*) and refers-to (& and &&) components
      are peeled off the types being compared, rather than checking
      these match in number and kind.
    - This peeling is done with peel_typedef_pointer_or_reference_type
      which also peels any number of CV qualifiers (OK) and
      array-of ([N]) type components (not OK).
    - The function sets a state variable (was_indirect_type) to modify
      the behaviour of downstream comparisons, but this cannot be
      passed through recursive calls.

The effect of the indirect_type flag is to switch to comparisons by
name: identically named structs don't get introspected. Arguably, a
more useful behaviour for --leaf-changes-only mode would be to treat
any change to a named type as non-local, except in the context of the
definition of that type itself. This would be a more significant
change in behaviour.

	* include/abg-fwd.h (types_have_similar_structure): In both
	overloads, add an indirect_type argument, defaulting to
	false.
	* src/abg-ir.cc (reference_type_def constructor): Tabify.
	(types_have_similar_structure): In both overloads, add an
	indirect_type argument and update documentation text. In the
	type_base_sptr overload, pass indirect_type in the tail
	call. In the type_base* overload, replace was_indirect_type
	with indirect_type; peel CV qualifiers and typedefs without
	testing as the peel function does this; replace the
	indiscriminate peeling of qualifier/pointer/reference/array
	type components with code that checks the same
	pointer/reference/array type component is used on each side
	and makes recursive calls with indirect_type set to true; pass
	the indirect_type argument recursively when comparing other
	subtypes; move the typeid check earlier, document its purpose
	and remove unneccessary checks after later dynamic casts;
	remove an always-true conditional; add a TODO for comparing
	array types more accurately.
	* tests/data/Makefile.am: Add new test case files.
	* tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc: New
	test case.
	* tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc: Ditto.
	* tests/data/test-abidiff-exit/test-leaf-peeling-report.txt:
	Ditto.
	* tests/test-abidiff-exit.cc: Run new test case.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-03-26 11:45:03 +01:00
Dodji Seketeli
b1b0586dc2 dwarf-reader: Fix bloom filter access in GNU_HASH section
The bloom filter of the GNU_HASH section of binaries is made of words
(bitmasks) that are either 32-bits for ELFCLASS32 binaries or 64-bits
for ELFCLASS64 binaries.

By using the GElf_Word type to hold the content of each bitmask we
assume the bitmask to be of a size of at most 'sizeof(Elf64_Word)'.
Unfortunately, the name Elf64_Word is a bit misleading because it's a
32-bits wide type (uint32_t).  That type is thus too short to hold an
entire bitmask for 64-bits binaries.

I won't give too many details here about how the bloom filter works as
it's described in details in the documentation for the GNU_HASH
section at http://www.linker-aliens.org/blogs/ali/entry/gnu_hash_elf_sections/.

Suffice it to say that in practise, we were comparing the least
significant bytes of a 64-bits bloom bitmask to a 32-bits value.
Depending on if we read those least significant bytes on a big or
little endian, we obviously don't get the same result.  Hence the
recent buid breakage at
https://builder.wildebeest.org/buildbot/#builders/14/builds/352 where
the runtestlookupsyms test fails.

This patch thus changes the code of lookup_symbol_from_gnu_hash_tab to
use a 64-bits type to hold the value of the bloom bitmask.  That way,
we never have any information loss.  The function bloom_word_at is
also changed to read the bloom bitmask as a 64-bits value when looking
at an ELFCLASS64 binary and to always return a 64-bits value.

It also adds a test to access the bloom filter of an ELFCLASS32
binary.

	* src/abg-dwarf-reader.cc (bloom_word_at): Properly read an
	element from the bloom bitmasks array of 32-bits values as a
	64-bits value in a portable way.  Always return a 64 bits value.
	(lookup_symbol_from_gnu_hash_tab): Use a 64-bits value to store
	the bloom bitmask.
	* tests/data/test-lookup-syms/test1-32bits.so: New test input,
	compiled as a 32bits binary to test for ELFCLASS32 binaries.
	* tests/data/test-lookup-syms/test1.c.compiling.txt: Documentation
	about how to compile the test1[-21bits].so files.
	* tests/data/Makefile.am: Add the new test material above to
	source distribution.
        * tests/test-lookup-syms.cc (in_out_specs): Add the
	test1-32bits.so test case to this test harness.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-03-20 13:35:00 +01:00
Matthias Maennich
34e867e74f dwarf-reader: remove superfluous ABG_ASSERT
maybe_adjust_et_rel_sym_addr_to_abs_addr contained an ABG_ASSERT to
ensure symbol_section is not used on an invalid value. Since
maybe_adjust_et_rel_sym_addr_to_abs_addr handles this case, this assert
can be removed.

	* src/abg-dwarf-reader.cc
	(maybe_adjust_et_rel_sym_addr_to_abs_addr): improve NULL check,
	remove superfluous ABG_ASSERT
	* tests/data/Makefile.am: Add new test case to the distribution.
	* tests/test-read-dwarf.cc: Likewise.
	* tests/data/test-read-dwarf/test27-bogus-binary.elf: New test case.

Signed-off-by: Matthias Maennich <maennich@google.com>
2020-03-18 23:10:14 +01:00
Matthias Maennich
247b4a1815 test-read-dwarf: ensure in_elf_path exists and add missing test files
test-read-dwarf silently succeeded even if the input elf file was not
existing. Hence, make distcheck succeeded even though the testfiles were
not distributed. Assert on the existence of the input file and add the
missing test case files.

	* tests/data/Makefile.am: add missing test case files
	* tests/test-read-dwarf.cc (test_task::perform): assert the
	input elf file exists.

Signed-off-by: Matthias Maennich <maennich@google.com>
2020-03-18 22:46:03 +01:00
Giuliano Procida
dc5e2dd893 Tag add/remove/change lines unconditionally with [A], [D], [C].
These tags were previously only emitted by the default reporter if the
there were more than 100 (hard-coded constant) items in a a list. The
leaf reporter emitted them unconditionally. This change simplifies the
code, makes output more consistent and makes it easier to interpret
diffs of diff output.

Additionally, in the reporting of changed unreachable types, the
indentation and quoting for the deleted and added cases was missing.
This patch corrects these issues.

Finally, when doing package differences, there were no tags for
deleted/added binaries. This patch adds them.

	* src/abg-default-reporter.cc (report): In the corpus_diff
	override, remove calculations of number of changes (total) and
	comparisons against arbitrary threshold (large_num); emit [A],
	[D], [C] tags unconditionally.
	* src/abg-reporter-priv.cc
	(maybe_report_unreachable_type_changes): Remove comparisons of
	number of changes against arbitrary threshold (large_num);
	emit [A], [D], [C] tags unconditionally; fix quoting of
        deleted unreachable types; fix indentation of changed
	unreachable types.
	* tools/abipkgdiff.cc (compare_prepared_userspace_packages):
	Emit [D] and [A] tags for removed and added binaries.
	* tests/data/test-*/*report*.txt: In 109 report files, add
        tags [A], [D], [C] tags and correct some indentation and
        quoting.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-03-18 14:45:26 +01:00
Giuliano Procida
09946c4aee Treat function type changes as local.
In leaf-changes-only mode, if the type of a struct's function pointer
member changes it currently gets categorised as a non-local change and
so is not reported. The change to any function passing such a struct
is considered non-local and also not reported.

This patch broadens the definition of local changes to include these
cases and so have them be reported in leaf-changes-only mode. It may
be the first of a sequence of such patches,

	* src/abg-ir.cc (types_have_similar_structure): Always compare
	function types (instead of just returning true) regardless of
        whether they are components of pointer-to-function or
        reference-to-function types.
	* tests/data/Makefile.am: Add new test case files.
	* tests/data/test-abidiff-exit/test-leaf2-report.txt: New test
	case.
	* tests/data/test-abidiff-exit/test-leaf2-v0.cc: Ditto.
	* tests/data/test-abidiff-exit/test-leaf2-v0.o: Ditto.
	* tests/data/test-abidiff-exit/test-leaf2-v1.cc: Ditto.
	* tests/data/test-abidiff-exit/test-leaf2-v1.o: Ditto.
	* tests/test-abidiff-exit.cc: Run new test case.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-03-17 13:33:26 +01:00
Giuliano Procida
2eb592169b Output 2-space indentation consistently.
abidiff emits hierarchical difference information using 2-space
indentation, almost everywhere.

In a few places, long lines are split up and 1-space is used for
clarity. Otherwise 1-space indentation appears to be only used when
reporting:

    - data member changes (not additions or removals)
    - the change of the type of a variable

This patch resolves these inconsistencies in favour of 2-space
indentation.

	* src/abg-default-reporter.cc (report): In the
	class_or_union_diff override, use 2-space indentation when
        listing changed members. In the var_diff override, do the same
        for variable type changes.
	* src/abg-leaf-reporter.cc: Ditto.
        * tests/data/test-*/*report*.txt: Update many test cases.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-03-16 22:39:35 +01:00
Giuliano Procida
0fd7565d06 Eliminate some unnecessary blank lines in diff output.
v2: More code simplification. Tests unchanged.

There is distributed responsibility for horizontal and vertical
whitespace in the reporting code with indent and new line control
information being manipulated by and passed in and out of functions.
Occasionally, this information is ignored or incorrect and the code
tends to err on the side of more rather than fewer new lines.

The outcome is that abidiff output sometimes contains extra blank
lines which can be confusing.

This patch eliminates some of the more obvious cases:

   - after data member deletions
   - in enumerator change lists
   - after "type size hasn't changed"
   - before listing impacted interfaces

A lot of passing of "new line needed" booleans between functions has
been eliminated in the process.

The patch cleans up the reporting of data members. The code will
either emit indentation, a short description and a new line or do
nothing at all.

The patch also removes some stray location reporting code for array
diffs which would have produced some oddly placed output. I could not
get this code to trigger as loc = decl->get_location() was never
present on the array decls in question.

	* src/abg-default-reporter.cc (report): In the type_decl_diff,
	enum_diff, array_diff, class_diff, union_diff and var_diff
	overrides, simplify new line logic which no longer needs to be
	threaded through report_name_size_and_alignment_changes. In
	the distinct_diff override, simplify new line logic which no
	longer needs to be threaded through
	report_size_and_alignment_changes. In the enum_diff override,
	emit just one blank line after each enum. In the array_diff
	override, remove stray location reporting which doesn't appear
	to ever trigger; fix new line logic. In the
	class_or_union_diff override, simplify new line logic for
	deleted members; pass indentation to represent_data_member.
	* src/abg-leaf-reporter.cc (report): In the array_diff,
	class_diff, union_diff and var_diff overrides, simplify new
	line logic which no longer needs to be threaded through
	report_name_size_and_alignment_changes. In the distinct_diff
	override, simplify new line logic which no longer needs to be
	threaded through report_size_and_alignment_changes. In the
	array_diff override, remove stray location reporting which
	doesn't appear to ever trigger; fix new line handling. In the
	class_or_union_diff override, simplify new line logic for
	deleted members; pass indentation to represent_data_member.
	In the corpus_diff override, tabify source indentation.
	* src/abg-reporter-priv.cc (represent_data_member): Handle
	indentation; fix new line logic.
	(report_size_and_alignment_changes): Fix new line logic
	for "type size hasn't changed" message; simplify new line
	logic and replace local bool n with argument bool nl for
	clarity.
	(report_size_and_alignment_changes): Remove bool nl argument
	and associated code as it had become always false; take
	responsibility for emitting terminating new lines and change
	return type to void.
	(report_name_size_and_alignment_changes): Fix new line logic;
	remove bool nl argument and associated code as it had become
	always false; take responsibility for emitting terminating new
	lines and change return type to void.
	(maybe_report_interfaces_impacted_by_diff) In both overrides,
	remove new line prefix code and new_line_prefix argument.
	* src/abg-reporter-priv.h (represent_data_member): Add indent
	argument.
	(report_size_and_alignment_changes) Remove bool nl argument;
	change return type to void.
	(report_name_size_and_alignment_changes) Remove bool nl
	argument; change return type to void.
	(maybe_report_interfaces_impacted_by_diff) In both overrides,
	remove new_line_prefix argument.
	* tests/data/test-*/*report*.txt: Remove some blank lines.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-03-16 16:15:09 +01:00
Giuliano Procida
28af7232c2 abg-leaf-reporter.cc: Fix indentation of function parameter diffs.
When reporting the details of changes to function parameter
differences in leaf-changes-only mode, the details are output at the
same level of indentation as the introductory text. In default mode
the usual 2-space indentation is used.

This patch fixes this discrepancy, making the output more readable.

	* src/abg-leaf-reporter.cc (report): In the fn_parm_diff
	override, indent the lines of detail by 2 spaces.
	* tests/data/test-abidiff-exit/test-leaf3-report.txt: Update
	report with correct indentation.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-03-13 18:30:31 +01:00
Giuliano Procida
dcba808257 Fix interaction of --redundant and --leaf-changes-only options.
The --redundant (meaning show-redundant-changes) option is supposed to
be implied by --leaf-changes-only and this is currently implemented by
making diff_context's --leaf-changes-only setter also duplicate the
behaviour of its --redundant setter.

In both abidiff and abipkgdiff, the diff_context setters are called
unconditionally, but the relative order of the calls for these two
options is different in each case, resulting in two different issues.

In abidiff, the --redundant setter is called second, undoing the
intended side-effect of any --leaf-changes-only flag. So --redundant
is not actually turned on in --leaf-changes-only mode unless requested
explicitly.

In abipkgdiff, the leaf-changes-only setter is called second, undoing
(in non-leaf mode) the effect of any --redundant flag. So --redundant
has no effect in default reporting mode.

The fix is move to move the "--leaf-changes-only implies --redundant"
logic from the setter to the set_diff_context_from_opts functions.
This patch also documents the implied behaviour in the usage strings.

	* src/abg-comparison.cc (diff_context::show_leaf_changes_only):
	Remove "--leaf-changes-only implies --redundant" logic.
	* tools/abidiff.cc (display_usage): Mention that
	--leaf-changes-only implies --redundant.
	(set_diff_context_from_opts): Make --leaf-changes-only imply
	--redundant; document this behaviour in a comment.
	* tools/abipkgdiff.cc: Ditto.
	* tests/data/Makefile.am: Add new test case files.
	* tests/data/test-abidiff-exit/test-leaf3-report.txt: Add new
	test case, to show --leaf-changes-only implies --redundant.
	* tests/data/test-abidiff-exit/test-leaf3-v0.c: Ditto.
	* tests/data/test-abidiff-exit/test-leaf3-v0.o: Ditto.
	* tests/data/test-abidiff-exit/test-leaf3-v1.c: Ditto.
	* tests/data/test-abidiff-exit/test-leaf3-v1.o: Ditto.
	* tests/test-abidiff-exit.cc: Run new test case.
	* tests/data/test-diff-pkg/libcdio-0.94-1.fc26.x86_64--libcdio-0.94-2.fc26.x86_64-report.1.txt:
	Update abipkgdiff report with --redundant output.
	* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-1.txt:
	Ditto.
	* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
	Ditto.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-03-13 18:06:14 +01:00
Giuliano Procida
3665c8ff44 Add more leaf change reporting.
The leaf-changes-only reporting path does not report on all the same
kinds of differences as the default reporting path does, such as
reporting about changes to variables, even though they can be
considered leaf changs.

    - The addition or removal of any symbol affects the ABI and is
      clearly a leaf change.
    - A change to a variable's declaration may be local rather than
      caused by a type change elsewhere.

This patch adds these missing pieces and reorders some of the existing
leaf reporting, bringing the default and leaf corpus_diff functions
closer to the point where they can be trivially merged or refactored.

This patch also corrects an error in reporting the total number of
leaf changes.

	* doc/manuals/abidiff.rst: Update the documentation for
	--leaf-changes-only.
	* doc/manuals/abipkgdiff.rst: Likewise.
	* src/abg-comparison.cc (emit_diff_stats): Exclude non-leaf
	changes to variables from the reported total of leaf changes.
	* src/abg-default-reporter.cc (report): In the corpus_diff
	override, move some code and comments for clarity.
	* src/abg-leaf-reporter.cc (report): In the corpus_diff
	override, additionally report removed/added/changed variables
	and removed/added symbols absent from debug info.
	* tests/data/Makefile.am: Add new test case files.
	* tests/data/test-abidiff-exit/test-leaf0-report.txt: Update
	to include reporting of variable diff (change of type).
	* tests/data/test-abidiff-exit/test-leaf1-report.txt: New test
	case with added/removed variables/functions and changed
	variables (both local and non-local type changes).
	* tests/data/test-abidiff-exit/test-leaf1-v0.cc: Ditto.
	* tests/data/test-abidiff-exit/test-leaf1-v0.o: Ditto.
	* tests/data/test-abidiff-exit/test-leaf1-v1.cc: Ditto.
	* tests/data/test-abidiff-exit/test-leaf1-v1.o: Ditto.
	* tests/test-abidiff-exit.cc: Run new test case. Supply
	--redundant otherwise the test isn't meaningful.

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-03-13 17:58:30 +01:00
Dodji Seketeli
42fcccb0d8 Update fedabipkgdiff tests according to commit b602f46c
This patch is a follow-up to this commit:

    b602f46c Fix spurious new lines after diff sections.

The intent is to update the tests that relate to the fedpkgdiff tool.
I didn't notice that those tests reference output were not updated by
commit b602f46c because I didn't have 'koji' installed on the system
I ran the regression test suite on; and without koji installed, the
fedpkgdiff test is automatically disabled.

The patch just mechanically adjusts the reference output that needed
it to comply with the newer better output of fedpkgdiff.

	* tests/data/test-fedabipkgdiff/test0-from-fc20-to-fc23-dbus-glib-report-0.txt:
	Adjust for useless whitespace removal.
	* tests/data/test-fedabipkgdiff/test1-from-fc20-to-dbus-glib-0.106-1.fc23.x86_64-report-0.txt: Likewise.
	* tests/data/test-fedabipkgdiff/test2-dbus-glib-0.100.2-2.fc20--dbus-glib-0.106-1.fc23-report-0.txt: Likewise.
	* tests/data/test-fedabipkgdiff/test3-dbus-glib-0.100.2-2.fc20.i686--dbus-glib-0.106-1.fc23.i686-report-0.txt: Likewise.
	* tests/data/test-fedabipkgdiff/test4-glib-0.100.2-2.fc20.x86_64.rpm-glib-0.106-1.fc23.x86_64.rpm-report-0.txt: Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-03-12 14:57:17 +01:00
Giuliano Procida
b602f46c00 Fix spurious new lines after diff sections.
The top-level corpus diff routines in abidiff have varied ways of
tracking whether or not to emit a new line after each section. Reuse
of state variables (which aren't always cleared) between sections
means that spurious new lines are sometimes output.

This patch replaces this new line logic in the functions with the same
simple pattern of using a local boolean state variable.

	* src/abg-default-reporter.cc (report): In the corpus_diff
	overload, just use a local boolean emitted state variable
	within each section to determine whether or not to follow the
	section with an extra new line.
	* src/abg-leaf-reporter.cc: Ditto.
	* tests/data/test-*/*report*.txt: Remove unwanted new lines
	from 27 files.

Signed-off-by: Giuliano Procida <gprocida@google.com>
2020-03-12 10:19:45 +01:00
Giuliano Procida
aadbd8cdbf abisym: Remove leading space in output.
When abisym reports a symbol as found, it currently emits a leading
space. It does not do this when reporting a symbol as not found.

This patch removes the leading space.

	* tools/abisym.cc (main): Remove leading space from output.
	* tests/data/test-lookup-syms/test0-report.txt: Remove leading
	space from expected output.
	* tests/data/test-lookup-syms/test01-report.txt: Ditto.
	* tests/data/test-lookup-syms/test02-report.txt: Ditto.
	* tests/data/test-lookup-syms/test1-1-report.txt: Ditto.

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-03-10 18:03:03 +01:00
Giuliano Procida
b7755d092a Fix the reporting of leaf change statistics.
Leaf changes (as reported with --leaf-changes-only) to variables were
miscounted as changes to functions.

	* src/abg-comparison.cc
	(apply_filters_and_compute_diff_stats):	Increment the correct
	counter for leaf variable changes.
	* tests/data/Makefile.am: Add new test case files.
	* tests/data/test-abidiff-exit/test-leaf0-report.txt: New test
	case.
	* tests/data/test-abidiff-exit/test-leaf0-v0.cc: Ditto.
	* tests/data/test-abidiff-exit/test-leaf0-v0.o: Ditto.
	* tests/data/test-abidiff-exit/test-leaf0-v1.cc: Ditto.
	* tests/data/test-abidiff-exit/test-leaf0-v1.o: Ditto.
	* tests/test-abidiff-exit.cc: Run new test case.

Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-03-10 17:41:22 +01:00
Giuliano Procida
6dfccee786 Add space missing between "[C]" tag and description of changed item.
All such tags are now followed by a space and are more readable.

	* src/abg-default-reporter.cc (report): In the overload for
	corpus_diff, output space after "[C]".
	* src/abg-leaf-reporter.cc (report): Likewise.
	* tests/data/test-*/*report*.txt: Update all the test
	reports.

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
2020-03-06 15:23:22 +01:00