]> sourceware.org Git - libabigail.git/log
libabigail.git
4 years agotest-read-dwarf: ensure in_elf_path exists and add missing test files
Matthias Maennich [Sun, 15 Mar 2020 12:55:11 +0000 (13:55 +0100)]
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>
4 years agoconfigure: add support for memory sanitizer (--enable-msan)
Matthias Maennich [Tue, 17 Mar 2020 23:18:37 +0000 (00:18 +0100)]
configure: add support for memory sanitizer (--enable-msan)

Similarly to asan, tsan and ubsan, add support for msan conditionally at
configure time. This allows us to track down issues caused by using
uninitialized values.

        * configure.ac: Add configure options for -fsanitize=memory

Signed-off-by: Matthias Maennich <maennich@google.com>
4 years agoabipkgdiff: fix documentation of --impacted-interface
Dodji Seketeli [Wed, 18 Mar 2020 16:17:29 +0000 (17:17 +0100)]
abipkgdiff: fix documentation of --impacted-interface

Update the help string of the --impacted-interface option to make it
match the one from the abidiff tool.

* tools/abipkgdiff.cc (display_usage): Use the same help string
for the --impacted-interface option as abidiff does.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
4 years agoabidiff: fix documentation of --impacted-interfaces
Matthias Maennich [Sun, 15 Mar 2020 22:31:43 +0000 (23:31 +0100)]
abidiff: fix documentation of --impacted-interfaces

abidiff --impacted-interfaces is supposed to show the impacted
interfaces. Hence fix the documentation to reflect that.

* tools/abidiff.cc(display_usage): Fix doc string for
--impacted-interfaces.

Reported-by: Saravana Kannan <saravanak@google.com>
Signed-off-by: Matthias Maennich <maennich@google.com>
4 years agoTag add/remove/change lines unconditionally with [A], [D], [C].
Giuliano Procida [Tue, 17 Mar 2020 13:19:59 +0000 (13:19 +0000)]
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>
4 years agoabg-workers: guard bring_workers_down to avoid dead lock
Matthias Maennich [Thu, 12 Mar 2020 11:31:58 +0000 (12:31 +0100)]
abg-workers: guard bring_workers_down to avoid dead lock

Since bring_workers_down is not atomic, a worker thread can make a racy
read on it while do_bring_workers_down() writes it. That can lead to a
deadlock between the worker thread waiting for more work and
do_bring_workers_down() waiting for the worker to finish.
Address this by guarding all access to bring_workers_down with locking
tasks_todo_mutex. This is likely to be dropped after migrating to newer
C++ standards supporting std::atomic.

* src/abg-workers.cc(do_bring_workers_down): keep
        task_todo_mutex locked while writing bring_workers_down,
(wait_to_execute_a_task): rewrite the loop condition to ensure
safe access to bring_workers_down.

Signed-off-by: Matthias Maennich <maennich@google.com>
4 years agoconfigure: add support for thread sanitizer (--enable-tsan)
Matthias Maennich [Thu, 12 Mar 2020 11:31:57 +0000 (12:31 +0100)]
configure: add support for thread sanitizer (--enable-tsan)

Similarly to asan and ubsan, add support for tsan conditionally at
configure time. This allows us to track down race conditions etc.

* configure.ac: Add configure options for -fsanitize=thread

Signed-off-by: Matthias Maennich <maennich@google.com>
4 years agodwarf-reader: gnu_hash_tab lookup: fix overflow in bloom hash calculation
Matthias Maennich [Fri, 13 Mar 2020 21:28:26 +0000 (22:28 +0100)]
dwarf-reader: gnu_hash_tab lookup: fix overflow in bloom hash calculation

For valid values of h1/h2 and c, the signed integer left shift
expression (1 << (h1 % c)) might overflow, exposing undefined behaviour.
Fix that by using a data type that can hold the value.

That issue had been reported by ASAN when running test-lookup-syms:
src/abg-dwarf-reader.cc:2028:50: runtime error:
shift exponent 53 is too large for 32-bit type 'int'

* src/abg-dwarf-reader.cc(lookup_symbol_from_gnu_hash_tab): Fix
signed integer overflow.

Signed-off-by: Matthias Maennich <maennich@google.com>
4 years agoTreat function type changes as local.
Giuliano Procida [Thu, 12 Mar 2020 06:30:36 +0000 (06:30 +0000)]
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>
4 years agoOutput 2-space indentation consistently.
Giuliano Procida [Fri, 13 Mar 2020 15:38:35 +0000 (15:38 +0000)]
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>
4 years agoEliminate some unnecessary blank lines in diff output.
Giuliano Procida [Mon, 16 Mar 2020 13:10:25 +0000 (13:10 +0000)]
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>
4 years agoabg-leaf-reporter.cc: Fix indentation of function parameter diffs.
Giuliano Procida [Thu, 12 Mar 2020 06:30:33 +0000 (06:30 +0000)]
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>
4 years agoFix interaction of --redundant and --leaf-changes-only options.
Giuliano Procida [Thu, 12 Mar 2020 06:30:32 +0000 (06:30 +0000)]
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>
4 years agoAdd more leaf change reporting.
Giuliano Procida [Fri, 13 Mar 2020 15:34:29 +0000 (15:34 +0000)]
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>
4 years agoUpdate fedabipkgdiff tests according to commit b602f46c
Dodji Seketeli [Thu, 12 Mar 2020 13:57:17 +0000 (14:57 +0100)]
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>
4 years agoFix spurious new lines after diff sections.
Giuliano Procida [Wed, 11 Mar 2020 10:35:23 +0000 (10:35 +0000)]
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>
4 years agoabg-comparison.cc: Remove stray function declaration.
Giuliano Procida [Tue, 10 Mar 2020 14:45:06 +0000 (14:45 +0000)]
abg-comparison.cc: Remove stray function declaration.

There was a stray file-scoped declaration of show_relative_offset_changes. This
function is now a member of diff_context.

* src/abg-comparison.cc (show_relative_offset_changes): Remove
this stray function declaration.

Signed-off-by: Giuliano Procida <gprocida@google.com>
4 years agoabisym: Remove leading space in output.
Giuliano Procida [Tue, 10 Mar 2020 10:31:13 +0000 (10:31 +0000)]
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>
4 years agoFix the reporting of leaf change statistics.
Giuliano Procida [Mon, 9 Mar 2020 15:10:32 +0000 (15:10 +0000)]
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>
4 years agoAdd space missing between "[C]" tag and description of changed item.
Giuliano Procida [Thu, 5 Mar 2020 16:03:16 +0000 (16:03 +0000)]
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>
4 years agoCorrect various inconsequential typos.
Giuliano Procida [Thu, 5 Mar 2020 15:34:31 +0000 (15:34 +0000)]
Correct various inconsequential typos.

In the leaf reporter member subtype changes are labelled as plain
changes and vice versa. This was probably due to the different
ordering of the code sections in the default and leaf reporters.
Output is unchanged as these tags currently map to the same strings.

When generating diff reports there were some rare cases where a
pretty representation might have been emitted twice or the trailing
whitespace might have been missing.

* src/abg-leaf-reporter.cc (report): In the class_or_union_diff
overoad, swap calls to report_mem_header to match the rest of the
code.
* src/abg-reporter-priv.cc (represent): In the var_diff_sptr
overload, add some missing whitespace; remember we've emitted the
pretty representation in 2 cases where this was omitted (though 1
of these is the last case where it makes no difference).
maybe_report_diff_for_symbol Add some missing whitespace; remember
we've reported a diff (and need a trailing newline) in 1 case
where this was omitted, also affecting the return value of the
function (but no caller cares).

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
4 years agoCorrect spelling of "alignment".
Giuliano Procida [Thu, 5 Mar 2020 14:40:39 +0000 (14:40 +0000)]
Correct spelling of "alignment".

* src/abg-reporter-priv.cc: s/alignement/alignment/
* tests/data/test-abidiff/test-struct0-report.txt: Ditto..
* tests/data/test-abidiff/test-struct1-report.txt: Ditto.
* tests/data/test-abidiff/test-var0-report.txt: Ditto.

Signed-off-by: Giuliano Procida <gprocida@google.com>
4 years agoBump version number to 1.8
Dodji Seketeli [Thu, 27 Feb 2020 17:18:10 +0000 (18:18 +0100)]
Bump version number to 1.8

     * configure.ac: Update version number to 1.8

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
4 years agoUpdate website for 1.7 libabigail-1.7
Dodji Seketeli [Wed, 26 Feb 2020 13:05:26 +0000 (14:05 +0100)]
Update website for 1.7

* doc/website/mainpage.txt: Update for 1.7

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
4 years agoUpdate ChangeLog
Dodji Seketeli [Tue, 25 Feb 2020 14:22:15 +0000 (15:22 +0100)]
Update ChangeLog

       * ChangeLog: Automatically update by running:
         'make update-changelog'.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
4 years agoUpdate NEWS file for 1.7
Dodji Seketeli [Tue, 25 Feb 2020 09:07:41 +0000 (10:07 +0100)]
Update NEWS file for 1.7

* NEWS: update for 1.7

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
4 years agogen-changelog.py: Update the script for python3
Dodji Seketeli [Tue, 25 Feb 2020 13:54:59 +0000 (14:54 +0100)]
gen-changelog.py: Update the script for python3

* gen-changelog.py (process_commit): Use the functional notation
for the print function invocation required by python3.
(output_commits, get_rel_tags, ): Specify that the output stream
of the subprocess running the git command is in the text format.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
4 years agotools-utils: Drop redefinition of fopen when BAD_FTS is defined
Dodji Seketeli [Wed, 26 Feb 2020 10:48:44 +0000 (11:48 +0100)]
tools-utils: Drop redefinition of fopen when BAD_FTS is defined

When the BAD_FTS macro was defined at compile time (to handle the use
of fts.h on glibc < 2.23), we needed to re-map fopen to fopen64 if
_FILE_OFFSET_BITS=64 was defined.  We do not need this anymore because
we don't use fopen in that module anymore.  Furthermore, as we now use
fstream, doing the fopen to fopen64 remapping is now preventing the
fstream c++ headers to compile on el6 systems.

This patch just simply does away with the fopen to fopen64 remapping and
thus fixes the compilation on el6 systems.

* src/abg-tools-utils.cc: Do not remap fopen to fopen64 as we
don't use fopen explicitly anymore.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
4 years agoUpdate copyright year to 2020
Dodji Seketeli [Fri, 21 Feb 2020 15:42:35 +0000 (16:42 +0100)]
Update copyright year to 2020

We are in February 2020 so this is long overdue.

* include/abg-comp-filter.h: Update copyright year to 2020.
* include/abg-comparison.h: Likewise.
* include/abg-config.h: Likewise.
* include/abg-corpus.h: Likewise.
* include/abg-cxx-compat.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-interned-str.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-reporter.h: Likewise.
* include/abg-sptr-utils.h: Likewise.
* include/abg-suppression.h: Likewise.
* include/abg-tools-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-workers.h: Likewise.
* include/abg-writer.h: Likewise.
* src/abg-comp-filter.cc: Likewise.
* src/abg-comparison-priv.h: Likewise.
* src/abg-comparison.cc: Likewise.
* src/abg-config.cc: Likewise.
* src/abg-corpus-priv.h: Likewise.
* src/abg-corpus.cc: Likewise.
* src/abg-default-reporter.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-internal.h: Likewise.
* src/abg-ir-priv.h: Likewise.
* src/abg-ir.cc: Likewise.
* src/abg-leaf-reporter.cc: Likewise.
* src/abg-libxml-utils.cc: Likewise.
* src/abg-libzip-utils.cc: Likewise.
* src/abg-reader.cc: Likewise.
* src/abg-reporter-priv.cc: Likewise.
* src/abg-reporter-priv.h: Likewise.
* src/abg-sptr-utils.cc: Likewise.
* src/abg-suppression-priv.h: Likewise.
* src/abg-suppression.cc: Likewise.
* src/abg-tools-utils.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-workers.cc: Likewise.
* src/abg-writer.cc: Likewise.
* tests/print-diff-tree.cc: Likewise.
* tests/test-abicompat.cc: Likewise.
* tests/test-abidiff-exit.cc: Likewise.
* tests/test-abidiff.cc: Likewise.
* tests/test-alt-dwarf-file.cc: Likewise.
* tests/test-core-diff.cc: Likewise.
* tests/test-diff-dwarf-abixml.cc: Likewise.
* tests/test-diff-dwarf.cc: Likewise.
* tests/test-diff-filter.cc: Likewise.
* tests/test-diff-pkg.cc: Likewise.
* tests/test-diff-suppr.cc: Likewise.
* tests/test-diff2.cc: Likewise.
* tests/test-dot.cc: Likewise.
* tests/test-ini.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-svg.cc: Likewise.
* tests/test-tools-utils.cc: Likewise.
* tests/test-types-stability.cc: Likewise.
* tests/test-utils.cc: Likewise.
* tests/test-utils.h: Likewise.
* tests/test-write-read-archive.cc: Likewise.
* tools/abiar.cc: Likewise.
* tools/abicompat.cc: Likewise.
* tools/abidiff.cc: Likewise.
* tools/abidw.cc: Likewise.
* tools/abilint.cc: Likewise.
* tools/abipkgdiff.cc: Likewise.
* tools/abisym.cc: Likewise.
* tools/binilint.cc: Likewise.
* tools/kmidiff.cc: Likewise.
* update-copyright.sh: Fix the updating script to handle not just
"Red Hat, Inc."

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
4 years agoabixml-reader: Support SONAME related properties on file suppression
Dodji Seketeli [Fri, 21 Feb 2020 11:32:42 +0000 (12:32 +0100)]
abixml-reader: Support SONAME related properties on file suppression

When comparing binary files (using abidiff for instance) libabigail
can interpret the [suppress_file] section of a suppression
specification.  If the suppression specification matches either of the
compared files, no comparison is performed.

At the moment, that doesn't work when comparing abixml files.

Thus, this patch implements that feature for abixml files.

With this patch, one can now write a suppression specification like
this:

  [suppress_file]
    soname_regexp = <some-regexp>

or

  [suppress_file]
    file_name_regexp = <some-regexp>

If either abixml file has a soname matched by such a regexp, then no
comparison is performed.

* doc/manuals/libabigail-concepts.rst: Update the documentation to
mention soname_regexp and soname_not_regexp is supported in the
[suppress_file] section.
* include/abg-suppression.h (suppression_matches_soname)
(suppression_matches_soname_or_filename): Declare new functions.
Make them be friends of class suppression_base.
* src/abg-reader.cc
(read_context::corpus_is_suppressed_by_soname_or_filename): Define
new member function.
(read_corpus_from_input): Apply file suppression.
* src/abg-suppression.cc (read_file_suppression): Support
"soname_regexp" and "soname_not_regexp" in the [suppress_file]
section.
(suppression_matches_soname)
(suppression_matches_soname_or_filename): Define new functions.
* tests/data/test-diff-suppr/libtest48-soname-abixml-report-{1,2}.txt:
New test reference output files.
Likewise.
* tests/data/test-diff-suppr/libtest48-soname-abixml-suppr.txt:
New test suppression file.
* tests/data/test-diff-suppr/libtest48-soname-abixml-suppr-{2,3,4}.txt::
Likewise.
* tests/data/test-diff-suppr/libtest48-soname-abixml-v{0,1}.so: New
test binary input files.
* tests/data/test-diff-suppr/libtest48-soname-abixml-v{0,1}.so.abi:
New abixml for the binary input files above.
* tests/data/test-diff-suppr/test48-soname-abixml-v{0,1}.c: Source
code of the binary input files above.
* tests/data/Makefile.am: Add the above test material to source
distribution.
* tests/test-diff-suppr.cc (in_out_specs): Add the test input
above to this test harness.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
4 years agosuppression: Better handle soname/filename properties evaluation
Dodji Seketeli [Fri, 21 Feb 2020 10:30:20 +0000 (11:30 +0100)]
suppression: Better handle soname/filename properties evaluation

At the moment, we don't make any difference between these two cases:

  1/ A suppression specification has no soname or file name related
  property.

  2/ A suppression specification has a soname or file name related
  property that doesn't match the soname or file name of a given
  corpus.

In both cases 1/ and 2/ libabigail currently assumes that the
suppression specification does not match the given corpus we are
looking at.  This can be wrong, especially if we are in the case 1/
and if the suppression does have other properties that should make it
match the corpus.

This patch fixes this issue.

* include/abg-suppression.h
(suppression_base::has_{soname,file_name}_related_property): Add
new member functions.
* src/abg-dwarf-reader.cc (read_context::suppression_can_match):
Fix the logic to make a difference between the case where the
suppression doesn't have any soname/filename property and the case
where the suppression does have a soname/filename property that
does not match the current binary.
* src/abg-reader.cc (read_context::suppression_can_match):
Likewise.
* src/abg-suppression-priv.h
(suppression_base::priv::matches_soname): If the suppression does
not have any soname related property then it doesn't match the
soname we are looking at.
(suppression_base::priv::matches_binary_name): If the suppression
does not have any filename related property then it doesn't match
the filename we are looking at.
* src/abg-suppression.cc
(suppression_base::has_{soname,file_name}_related_property):
Define new member functions.
(sonames_of_binaries_match): If the suppression does not have any
soname related property then it doesn't match the corpora of the
diff we are looking at.
(names_of_binaries_match): If the suppression does not have any
filename related property then it doesn't match the corpora of the
diff we are looking at.
(type_suppression::suppresses_type): Fix the logic to make a
difference between the case where the suppression doesn't have any
soname/filename property and the case where the suppression does
have a soname/filename property that does not match the current
binary.
(function_suppression::suppresses_{function, function_symbol}):
Likewise.
(variable_suppression::suppresses_{variable, variable_symbol}):
Likewise.
(file_suppression::suppresses_file): Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
4 years agoFix stray comma in leaf-changes-only mode.
Giuliano Procida [Wed, 5 Feb 2020 11:17:13 +0000 (11:17 +0000)]
Fix stray comma in leaf-changes-only mode.

This change replaces the "\n," sometimes seen in output (such as for
the PR25128 test cases) with a correctly indented "and".

* src/abg-reporter-priv.cc (represent): Don't try to follow
        output of indented pretty representation with a comma, just
        emit "and" unconditionally; remove unnecessary intermediate
        ostringstream.
* tests/data/Makefile.am: Add new test case files.
* tests/data/test-abidiff-exit/test-no-stray-comma-*: New test
        cases.
        * tests/data/test-diff-suppr/test46-PR25128-report-?.txt:
        Replace unindented comma with indented "and".
* tests/test-abidiff-exit.cc: Add no-stray-comma test case.

Signed-off-by: Giuliano Procida <gprocida@google.com>
Reviewed-by: Matthias Maennich <maennich@google.com>
4 years agoSort kernel module object files before processing them.
Giuliano Procida [Tue, 4 Feb 2020 17:16:27 +0000 (17:16 +0000)]
Sort kernel module object files before processing them.

This imposes a deterministic ordering, making diffs more predictable
and allowing reproducible testing.

        * src/abg-tools-utils.cc (get_binary_paths_from_kernel_dist):
        Sort module_paths.

Signed-off-by: Giuliano Procida <gprocida@google.com>
4 years agoabg-workers: Rework the worker queue to improve concurrent behaviour
Matthias Maennich [Tue, 4 Feb 2020 13:05:49 +0000 (13:05 +0000)]
abg-workers: Rework the worker queue to improve concurrent behaviour

This patch refactors the abigail::workers::queue and
abigail::workers::worker implementations to avoid holding locking
primitives longer than necessary.

In particular, the queue_cond_mutex was held during the entiry worker
runtime, effectively serializing the workers. Hence, use a mutex+cond
pair for each, the input and output queue and only synchronize around
the interaction with their corresponding queues. The
tasks_todo_(mutex|cond) are meant to synchronize scheduling and
distribution of work among workers, while tasks_done_(mutex|cond) are
used for synchronizing threads when putting back the tasks to the output
queue and to hold back threads waiting for the queue and workers to
drain.

Along that way, I did some cleanup that was now possible.
 - Move entire implementation of abigail::workers::task into header.
 - Make default_notify a static member.
 - Replace the multiple constructors with one with default arguments.

* include/abg-workers.h (workers::task): move entire
implementation to header and drop superfluous forward declaration.
* src/abg-workers.cc (workers::task):: Likewise.
(workers::queue::priv): Drop queue_cond_mutex, rename queue_cond
to tasks_todo_cond, add task_done_cond, make default_notify
static.
(workers::queue::priv::priv): Add default arguments to fully
qualified constructor, drop the remaining ones.
(workers::queue:prive::more_tasks_to_execute): Drop method.
(workers::queue:prive::schedule_task): Do not synchronize access
to the queue condition variable, but only on the mutex.
(do_bring_workers_down): Likewise. Also await tasks_done to be
empty.
(workers::queue:prive::worker::wait_to_execute_a_task): Await
tasks on the tasks_todo with tasks_todo_(cond|mutex) and signal
task completion to tasks_done_cond.

Signed-off-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
4 years agoabg-dwarf-reader: zero initialize local Dwarf_Addr values
Matthias Maennich [Mon, 3 Feb 2020 16:25:16 +0000 (16:25 +0000)]
abg-dwarf-reader: zero initialize local Dwarf_Addr values

Not initializing those might lead to undefined behaviour. E.g. if the
call to 'dwarf_ranges' does not initialize 'addr', we pass that
uninitialized value to 'maybe_adjust_fn_sym_address' and test it for
zero as first action, depending on the random value. Hence, fix that by
initializing the values.

* src/abg-dwarf-reader.cc
(read_context::get_first_exported_fn_address_from_DW_AT_ranges):
initialize local Dwarf_Addr variables.

Reviewed-by: Dodji Seketeli <dodji@seketeli.org>
Signed-off-by: Matthias Maennich <maennich@google.com>
4 years agoabg-comparison: prefer .empty() over implicit bool conversion of .size()
Matthias Maennich [Wed, 29 Jan 2020 15:31:26 +0000 (15:31 +0000)]
abg-comparison: prefer .empty() over implicit bool conversion of .size()

size() is not guaranteed to be a constant-time function. Also, using
.empty() shows clearer intent. Hence switch to using .empty().
That issue was flagged by clang-tidy[1].

* src/abg-comparison.cc (corpus_diff::has_changes): prefer
!container.empty() over bool(container.size())

[1] https://clang.llvm.org/extra/clang-tidy/checks/readability-container-size-empty.html

Reviewed-by: Dodji Seketeli <dodji@seketeli.org>
Signed-off-by: Matthias Maennich <maennich@google.com>
4 years agoFix some parameter name inconsistencies
Matthias Maennich [Wed, 29 Jan 2020 14:54:10 +0000 (14:54 +0000)]
Fix some parameter name inconsistencies

Clang-tidy flags those name inconsistencies and they are trivial to fix,
hence just do it. No functional change.

* src/abg-comparison-priv.h
(corpus_diff::priv::count_unreachable_types): use consistent
parameter naming.
* tools/abidiff.cc(main): Likewise.

Reviewed-by: Dodji Seketeli <dodji@seketeli.org>
Signed-off-by: Matthias Maennich <maennich@google.com>
4 years agoDon't ignore options when diffing translation units (.bi files).
Giuliano Procida [Mon, 3 Feb 2020 12:04:39 +0000 (12:04 +0000)]
Don't ignore options when diffing translation units (.bi files).

There was an inconsistency in the way the diff context was used for
different file types. This change eliminates this and so .bi files now
have all the command line options applied to their diffs.

* tests/data/Makefile.am: Add test case files.
* tests/data/test-abidiff-exit/test-loc-*: New test cases.
* tests/test-abidiff-exit.cc (in_out_specs): Add new test cases.
* tools/abidiff.cc (main): Use populated ctxt for translation unit
diff.

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
4 years agoFix typo in COMPILING.
Giuliano Procida [Wed, 29 Jan 2020 17:45:38 +0000 (17:45 +0000)]
Fix typo in COMPILING.

COMPILING: Fix typo: s/wan/want.

Signed-off-by: Giuliano Procida <gprocida@google.com>
4 years agoRemove redundant mention of libtool in COMPILING documentation.
Giuliano Procida [Wed, 29 Jan 2020 17:01:21 +0000 (17:01 +0000)]
Remove redundant mention of libtool in COMPILING documentation.

* COMPILING: Remove redundant mention of libtool.

Signed-off-by: Giuliano Procida <gprocida@google.com>
4 years agoTesting: add Catch Unit test framework
Matthias Maennich [Sun, 26 Jan 2020 20:29:56 +0000 (20:29 +0000)]
Testing: add Catch Unit test framework

This patch adds the Catch [1] unit test framework in version v1.12.2 [2]
along with its integration into the existing build and test definition.

While there is version v2 available, v1 still supports C++98, hence we
can make use of it. The framework is distributed as a single header
file. And since it is less then 500k and it comes with a permissive
license, I decided to directly add the file rather than requiring
users/developers/distributors to satisfy the new dependency.

The integration is fairly simple: A new library libcatch.a provides the
`main` for the tests that run with Catch. The tests themselves require
to include the header as well and to link against said library.

As an example I migrated the test-kmi-whitelist test to Catch. The test
becomes a bit more structured and error reporting significantly
improved. E.g. see this intentional breakage:

 |  --- a/tests/test-kmi-whitelist.cc
 |  +++ b/tests/test-kmi-whitelist.cc
 |  @@ -140,5 +140,5 @@ TEST_CASE("WhitelistWithTwoSections", "[whitelists]")
 |     suppressions_type suppr
 |         = gen_suppr_spec_from_kernel_abi_whitelists(abi_whitelist_paths);
 |     REQUIRE(!suppr.empty());
 |  -  test_suppressions_are_consistent(suppr, "^test_symbol1$|^test_symbol2$");
 |  +  test_suppressions_are_consistent(suppr, "^test_symbol$|^test_symbol2$");

It leads to this test output:

 |  ---------------------------------------------------------------------------
 |  WhitelistWithTwoSections
 |  ---------------------------------------------------------------------------
 |  ../../tests/test-kmi-whitelist.cc:136
 |  ...........................................................................
 |
 |  ../../tests/test-kmi-whitelist.cc:81: FAILED:
 |    REQUIRE( left->get_symbol_name_not_regex_str() == expr )
 |  with expansion:
 |    "^test_symbol1$|^test_symbol2$"
 |    ==
 |    "^test_symbol$|^test_symbol2$"
 |
 |  ===========================================================================
 |  test cases:  6 |  5 passed | 1 failed
 |  assertions: 41 | 40 passed | 1 failed

[1] https://github.com/catchorg/Catch2
[2] https://github.com/catchorg/Catch2/releases/tag/v1.12.2

* tests/.gitignore: Add entry for .dirstamp
* tests/Makefile.am: Add libcatch test library and use it for
runtestkmiwhitelist.
* tests/lib/catch.cc: New test driver implementation.
* tests/lib/catch.hpp: Add Catch v1.12.2 header only test library.
* tests/test-kmi-whitelist.cc: Migrate to use Catch test framework.

Reviewed-by: Dodji Seketeli <dodji@seketeli.org>
Signed-off-by: Matthias Maennich <maennich@google.com>
4 years agoabg-fwd: drop duplicate forward declaration for corpus_sptr
Matthias Maennich [Sat, 25 Jan 2020 23:02:00 +0000 (23:02 +0000)]
abg-fwd: drop duplicate forward declaration for corpus_sptr

This declaration has already been done some lines above. Drop the
duplicate.

* include/abg-fwd.h: drop superfluous forward declaration.

Signed-off-by: Matthias Maennich <maennich@google.com>
4 years agoFix / add include guards
Matthias Maennich [Sat, 25 Jan 2020 22:57:38 +0000 (22:57 +0000)]
Fix / add include guards

Fix the include guards of abg-dwarf-reader.h and abg-reporter-priv.h by
moving them before any other includes where they actually belongs.
Add missing include guards for abg-libxml-utils.h and abg-libzip-utils.h.

* include/abg-dwarf-reader.h: Move include guard to the beginning.
* include/abg-reporter-priv.h: Likewise.
* include/abg-libxml-utils.h: Add include guard.
* include/abg-libzip-utils.h: Likewise.

Signed-off-by: Matthias Maennich <maennich@google.com>
4 years agodwarf-reader: handle binaries with missing symtab
Matthias Maennich [Sat, 25 Jan 2020 21:40:14 +0000 (21:40 +0000)]
dwarf-reader: handle binaries with missing symtab

A broken elf file might not have a valid symtab. As of now we would hit
an ABG_ASSERT and crash. Let's catch that case and bail out instead.

        * src/abg-dwarf-reader.cc (load_symbol_maps_from_symtab_section):
        Handle elf file with missing symtab.
        * tests/test-read-dwarf.cc (InOutSpec): add test case.
        * tests/data/test-read-dwarf/test26-bogus-binary.elf: new test data.

Signed-off-by: Matthias Maennich <maennich@google.com>
5 years agodwarf-reader: handle symtab.section_header.sh_entsize == 0
Matthias Maennich [Fri, 24 Jan 2020 22:45:56 +0000 (22:45 +0000)]
dwarf-reader: handle symtab.section_header.sh_entsize == 0

A broken elf file with a sh_entsize of 0 makes the dwarf reader crash
due to a division by zero. Fix this by validating the input and exiting
early in that case.

* src/abg-dwarf-reader.cc (load_symbol_maps_from_symtab_section):
Handle elf file with invalid sh_entsize.
* tests/test-read-dwarf.cc (test_task::perform): handle empty
in_abi_path and out_abi_path as 'read only' test.
(InOutSpec): add test case.
* tests/data/test-read-dwarf/test25-bogus-binary.elf: new test data.

Signed-off-by: Matthias Maennich <maennich@google.com>
5 years agoclang-format: Better approximation for binary operators and assignments
Matthias Maennich [Tue, 21 Jan 2020 18:26:42 +0000 (18:26 +0000)]
clang-format: Better approximation for binary operators and assignments

The project style requires assignment operators to be on the first line
of two if the line needs to break. Reflect that in the .clang-format
configuration to approximate the style better when using clang-format.

* .clang-format: Add BreakBeforeBinaryOperators option.

Signed-off-by: Matthias Maennich <maennich@google.com>
5 years agoKMI Whitelists: Drop old whitelist extraction methods
Matthias Maennich [Tue, 14 Jan 2020 17:34:50 +0000 (17:34 +0000)]
KMI Whitelists: Drop old whitelist extraction methods

The previous commit introduces a new (tested) way of creating function
and variable suppressions from multiple whitelist definitions. Migrate
to this new way of processing KMI whitelists.

* include/abg-tools-utils.h
(gen_suppr_spec_from_kernel_abi_whitelist): Delete declaration.
* src/abg-tools-utils.cc
(gen_suppr_spec_from_kernel_abi_whitelist): Delete definition
and migrate users to gen_suppr_spec_from_kernel_abi_whitelists.
* tools/abidiff.cc (set_suppressions): Migrate from using
gen_suppr_spec_from_kernel_abi_whitelist to
gen_suppr_spec_from_kernel_abi_whitelists.
* tools/abidw.cc (set_suppressions): Likewise.
* tools/abipkgdiff.cc: Drop unused using definition.
* tools/kmidiff.cc: Likewise.

Reviewed-by: Dodji Seketeli <dodji@seketeli.org>
Signed-off-by: Matthias Maennich <maennich@google.com>
5 years agoKMI Whitelists: Add functionality to make whitelists additive
Matthias Maennich [Tue, 14 Jan 2020 17:34:49 +0000 (17:34 +0000)]
KMI Whitelists: Add functionality to make whitelists additive

If multiple KMI whitelists are specified, either by passing
--kmi-whitelist several times or by having multiple whitelist sections
in the whitelist files, the generated suppressions are created as an
intersection of symbols. That is rather unusual, as whitelisting should
rather work additive. That means that the symbols (or expressions
thereof) defined across several sections or files shall be considered a
union of symbols. This patch combines the whitelist parsing to create
exactly one function_suppression and one variable suppression. A test
case has been added to ensure the functionality is working.

Please note, migrating the existing code to this new functionality is
done in a separate commit.

* include/abg-tools-utils.h
(gen_suppr_spec_from_kernel_abi_whitelists): New function.
* src/abg-tools-utils.cc
(gen_suppr_spec_from_kernel_abi_whitelists): Likewise.
* tests/.gitignore: Ignore new test executable.
* tests/Makefile.am: Add new test executable.
* tests/data/test-kmi-whitelist/whitelist-with-another-single-entry:
New test input file.
* tests/data/test-kmi-whitelist/whitelist-with-duplicate-entry:
Likewise.
* tests/data/test-kmi-whitelist/whitelist-with-single-entry:
Likewise.
* tests/data/test-kmi-whitelist/whitelist-with-two-sections:
Likewise.
* tests/data/Makefile.am: Add above test material.
* tests/test-kmi-whitelist.cc: Add new test executable.

Reviewed-by: Dodji Seketeli <dodji@seketeli.org>
Signed-off-by: Matthias Maennich <maennich@google.com>
5 years agowriter: completely skip over empty corpora
Matthias Maennich [Mon, 13 Jan 2020 14:44:51 +0000 (14:44 +0000)]
writer: completely skip over empty corpora

A corpus that has no symbols contributing to the ABI surface (e.g.
because of an exhaustive suppression), will not contribute in a later
comparison via abidiff and friends. Hence, there is no need for such
entries to appear in the ABI xml representation. This patch completely
suppresses empty corpora.

* src/abg-writer.cc (write_corpus): completely skip empty
corpora rather than creating an empty entry for them.

Reviewed-by: Dodji Seketeli <dodji@seketeli.org>
Signed-off-by: Matthias Maennich <maennich@google.com>
5 years agocorpus: is_empty: consider actual translation unit contents
Matthias Maennich [Mon, 13 Jan 2020 14:44:50 +0000 (14:44 +0000)]
corpus: is_empty: consider actual translation unit contents

A corpus with completely filtered out symbols (exhaustive whitelist),
still contains compilation units, but they are empty. A list of empty
translation units shall be considered empty as no entries need to be
considered. That is useful to skip empty corpora when writing out the
xml for them.

Hence, teach is_empty() to have a look at the actual translation units.

* src/abg-corpus.cc (corpus::is_empty): consider a list of
  empty members to be empty.

Reviewed-by: Dodji Seketeli <dodji@seketeli.org>
Signed-off-by: Matthias Maennich <maennich@google.com>
5 years agoabg-reader: handle empty corpus nodes in xml representation
Matthias Maennich [Mon, 13 Jan 2020 10:42:14 +0000 (10:42 +0000)]
abg-reader: handle empty corpus nodes in xml representation

An abi-corpus might be part of the representation, but might (due to
filters like whitelisting) not contain actual symbols to be considered.
In that case, `abidw` produces an empty abi-corpus node.

Valid ways of representing this in XML are

 -  <abi-corpus path='vmlinux' architecture='elf-arm-aarch64'/>

 -  <abi-corpus path='vmlinux' architecture='elf-arm-aarch64'></abi-corpus>

 -  <abi-corpus path='vmlinux' architecture='elf-arm-aarch64'>
    </abi-corpus>

abg-reader could currently only handle the last format and crashed upon
processing the first two ones. The crash happened due to the XMLNode
having no children, but that was assumed. The last case succeeded so
far as this form actually contains a text node (with the newline
character) as a child.

Fix this by handling the case of a node not having children by exiting
early with an empty node.

* src/abg-reader.cc (read_corpus_from_input): when assigning a
corpus node, assure the node actually has children.
* tests/test-abidiff.cc (main): Add test for variants of empty
xml nodes to the test harness.
* tests/data/test-abidiff/test-empty-corpus-0.xml: Test input
containing an empty xml node that closes immediately.
* tests/data/test-abidiff/test-empty-corpus-0.xml: Test input
containing an empty xml node that closes immediately with a tag.
* tests/data/test-abidiff/test-empty-corpus-0.xml: Test input
containing an empty xml node that closes with a tag on a new line.
* tests/data/test-abidiff/test-empty-corpus-report.txt:
Expected test output (empty abidiff) for diffing xml with itself.
* tests/data/Makefile.am: Add the new test input material above
to source distribution.

Reviewed-by: Dodji Seketeli <dodji@seketeli.org>
Signed-off-by: Matthias Maennich <maennich@google.com>
5 years agoBug 25409 - Fix reading layout-offset-in-bits attribute of data-member
Dodji Seketeli [Fri, 17 Jan 2020 15:13:50 +0000 (16:13 +0100)]
Bug 25409 - Fix reading layout-offset-in-bits attribute of data-member

In the abixml format, when reading the value of the
'layout-offset-in-bits' attribute of the data-member child element of
a class-decl element, we wrongly use atoi.  The reason why atoi is
wrong is that it can only read an 'int' but layout-offset-in-bits can
be a 64 bits unsigned value which comes fromt the DWARF
DW_AT_bit_offset attribute.

We are thus using stroull instead, in this patch.

* src/abg-reader.cc (read_offset_in_bits): Fix comment.  Use
stroull rather than atoi.
* tests/data/test-diff-dwarf-abixml/PR25409-librte_bus_dpaa.so.20.0:
Add new binary test input.
* tests/data/test-diff-dwarf-abixml/PR25409-librte_bus_dpaa.so.20.0-report-0.txt:
Add new reference output.
* tests/data/test-diff-dwarf-abixml/PR25409-librte_bus_dpaa.so.20.0.abi:
Add new abixml representation for the binary test input above.
* tests/data/Makefile.am: Add the new test material above to
source distribution.
* tests/test-diff-dwarf-abixml.cc (in_out_specs): Add the test
input above to the test harness.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoabidiff/kmidiff: do not default-suppress added symbols
Matthias Maennich [Mon, 13 Jan 2020 10:16:37 +0000 (10:16 +0000)]
abidiff/kmidiff: do not default-suppress added symbols

kmidiff and abidiff do filter out added symbols (vars, functions and
symbols without debug info) by default when dealing with kernel
binaries. The reason for this is that the ABI could be considered
compatible and not broken when adding symbols. In practice, this is
confusing as there is no possibility for a symmetric comparison (i.e. a
deleted function when comparing left to right is an added function when
comparing right to left). Furthermore, there is no option available to
actually report these added symbols. I thought of adding an option to
report added symbols, but in the end came to the conclusion that we
should behave consistent across the various ways you can diff an ABI
with abidiff and kmidiff and should not change default behaviour for a
particular type of binary. Hence, remove the default behaviour of
filtering out added symbols when comparing kernel binaries. To restore
the current behaviour, the user needs to parametrize with the tools with
--no-added-syms --no-unreferenced-symbols.

Adjusted test cases accordingly and add a new test that covers the old
behaviour new available with additional flags to abidiff.

* tools/abidiff.cc (adjust_diff_context_for_kmidiff): Drop
default suppression of added symbols.
* tools/kmidiff.cc (set_diff_context): Likewise.
* tests/data/test-diff-suppr/test46-PR25128-report-1.txt: Adjust
test expectation.
* tests/data/test-diff-suppr/test46-PR25128-report-2.txt: Add
test case for abidiff with flag --no-added-syms.
* tests/data/Makefile.am: add new testcase.

Reviewed-by: Dodji Seketeli <dodji@seketeli.org>
Signed-off-by: Matthias Maennich <maennich@google.com>
5 years agoAdd (undocumented) support for version suffixes
Matthias Maennich [Sun, 15 Dec 2019 23:51:51 +0000 (23:51 +0000)]
Add (undocumented) support for version suffixes

Allow appending arbitrary text to the libabigail version string
representation. That is useful to identify custom versions of the
library (e.g. development versions or versions of a particular origin).

The feature can be enabled by passing VERSION_SUFFIX to `configure`,
e.g.

  $ configure VERSION_SUFFIX="-dev"

That will extend the version string to (currently) 1.7.0-dev.
The behaviour before this patch remains the default behaviour of not
appending any additional text.

The feature stays intentionally undocumented as the main release of
libabigail will usually not carry a version suffix.

* configure.ac: add substitution for VERSION_SUFFIX
* include/abg-version.h.in: add define for ABIGAIL_VERSION_SUFFIX
* include/abg-config.h(abigail_get_library_version): add support
for a version suffix
* src/abg-config.cc(abigail_get_library_version): Likewise.
* src/abg-tools-utils.cc(get_library_version_string): Likewise.

Reviewed-by: Dodji Seketeli <dodji@seketeli.org>
Signed-off-by: Matthias Maennich <maennich@google.com>
5 years agodwarf-reader: relax restriction about relocation sections in try_reading_first_ksymta...
Matthias Maennich [Wed, 13 Nov 2019 15:09:17 +0000 (15:09 +0000)]
dwarf-reader: relax restriction about relocation sections in try_reading_first_ksymtab_entry

Commit 43679a610316 ("dwarf-reader: refactor
try_reading_first_ksymtab_entry_using{pre,}_v4_19_format") introduced an
assertion to ensure the absence of ksymtab relocation sections as they
are an unhandled case for try_reading_first_ksymtab_entry. This
assertion turns out to be too strict as relocation sections might be
present (e.g. on x86_64), but not affecting the functionality of this
function (i.e. helping to detect the ksymtab format). Hence, remove the
assertion and document that case.

* src/abg-dwarf-reader.cc (try_reading_first_ksymtab_entry):
remove assertion and update documentation

Fixes: 43679a610316 ("dwarf-reader: refactor try_reading_first_ksymtab_entry_using{pre,}_v4_19_format")
Reviewed-by: Dodji Seketeli <dodji@seketeli.org>
Signed-off-by: Matthias Maennich <maennich@google.com>
5 years agoabg-dwarf-reader: resolve relocation sections by index
Matthias Maennich [Mon, 11 Nov 2019 21:48:10 +0000 (21:48 +0000)]
abg-dwarf-reader: resolve relocation sections by index

Looking up relocation sections by name introduces a dependency to the
linker in use. Relocation sections might be named differently. For
instance, linking kernel modules with the bfd linker leads to a
.rela__ksymtab section corresponding to the __ksymtab section. Using lld
as a linker leads to .rela___ksymtab as section name. Both are valid.
When the kernel loads these, it simply applies all relocations from all
sections it finds. Tools should not depend on the concrete name (even
though I would prefer consistency among them). Libabigail hit an
assertion when trying to extract the ABI from a kernel module linked
with lld.

Hence, resolve the relocation sections for __ksymtab and __ksymtab_gpl
by iterating over the ELF sections, searching for relocation sections
and identifying the one that points to the respective ksymtab.

* src/abg-dwarf-reader.cc (find_relocation_section): New function.
(find_ksymtab_reloc_section): Use find_relocation_section to
resolve the ksymtab's relocation section.
(find_ksymtab_gpl_reloc_section): Likewise.

Fixes: e6870326e01a ("Support pre and post v4.19 ksymtabs for Linux kernel modules")
Cc: Jessica Yu <jeyu@kernel.org>
Cc: Android Kernel Team <kernel-team@android.com>
Reviewed-by: Dodji Seketeli <dodji@seketeli.org>
Signed-off-by: Matthias Maennich <maennich@google.com>
5 years agoMisc typo fixes
Dodji Seketeli [Tue, 3 Dec 2019 17:35:10 +0000 (18:35 +0100)]
Misc typo fixes

* src/abg-comparison.cc
(leaf_diff_node_marker_visitor::visit_begin): Fix typo in
comments.
* src/abg-corpus.cc
(corpus::get_types_not_reachable_from_public_interfaces):
Likewise.
* src/abg-ir.cc (decl_base::set_has_anonymous_parent): Likewise.
* src/abg-reader.cc (add_read_context_suppressions): Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoBug 24690 - Support comparing non-reachable types of a binary
Dodji Seketeli [Wed, 13 Nov 2019 10:04:18 +0000 (11:04 +0100)]
Bug 24690 - Support comparing non-reachable types of a binary

This patch adds the ability to compare all types of a binary,
including those types that are not reachable from global functions and
variables.

This implies that for types that are not reachable from public
interfaces, we want compare them against each others directly, without
first comparing global functions/variables and walking the graph of
reachable types from there.

The patch adds the --non-reachable-types option to abidiff and
abipkgdiff, instructing them to also compare types that are
non-reachable from global variables and functions.

Using that option, for instance, here is what the summary of
abipkgdiff now looks like, in the test case attached added by this
patch:

================ changes of 'libflatpak.so.0.10204.0'===============
  Functions changes summary: 0 Removed, 0 Changed (16 filtered out), 16 Added functions
  Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
  Unreachable types summary: 3 removed (2 filtered out), 1 changed (15 filtered out), 3 added (1 filtered out) types

You can see that there is a new summary line which starts with the
string: "Unreachable types summary:"

Then in the body of the report, those unreachable types are reported
separately.

In practise, we want to limit the unreachable types to compare
somehow, otherwise we'll end up comparing all the types of the types
of the binary and that can be huge.  So we want to limit the
unreachable type analysis to types that are defined in public headers.

So, for abipkgdiff, one can limit the analysis of non-reachable types
to those defined in public headers by supplying the --devel{1,2}
options that specifies the development packages that contain said
public headers.  For abidiff however, you'll want to use the
--headers-dir{1,2} options for that.

The patch comes with appropriate regression tests.

* include/abg-comparison.h (string_type_base_sptr_map): Define new
typedef.
(diff_context::show_unreachable_types): Declare new member
functions.
(corpus_diff::{deleted_unreachable_types,
deleted_unreachable_types_sorted, added_unreachable_types,
added_unreachable_types_sorted, changed_unreachable_types,
changed_unreachable_types_sorted}): Likewise.
(maybe_report_unreachable_type_changes): Declare this function a
friend of class corpus_diff.
(corpus_diff::diff_stats::{num_added_unreachable_types,
num_added_unreachable_types_filtered_out,
net_num_added_unreachable_types, num_removed_unreachable_types,
num_removed_unreachable_types_filtered_out,
net_num_removed_unreachable_types, num_changed_unreachable_types,
num_changed_unreachable_types_filtered_out,
net_num_changed_unreachable_types}): Likewise.
* src/abg-comparison-priv.h
(diff_context::priv::show_unreachable_types_): Define new data
member.
(diff_context::priv::priv): Initialize the new data member.
(diff_comp::operator()): Use pretty representation of diff
subjects to sort them, rather than just their name.  Also, add
comment to the other member functions of diff_comp.
(corpus_diff::{unreachable_types_edit_script_,
deleted_unreachable_types_, deleted_unreachable_types_sorted_,
suppressed_deleted_unreachable_types_, added_unreachable_types_,
added_unreachable_types_sorted_,
suppressed_added_unreachable_types_, changed_unreachable_types_,
changed_unreachable_types_sorted_}): Define new data members.
(corpus_diff::priv::apply_supprs_to_added_removed_fns_vars_unreachable_types):
Changed the name of
corpus_diff::priv::apply_suppressions_to_added_removed_fns_vars into
this.
(corpus_diff::priv::{added_unreachable_type_is_suppressed,
deleted_unreachable_type_is_suppressed,
changed_unreachable_types_sorted, count_unreachable_types}):
Declare new member functions.
(corpus_diff::diff_stats::priv::{num_added_unreachable_types,
num_added_unreachable_types_filtered_out,
num_removed_unreachable_types,
num_removed_unreachable_types_filtered_out,
num_changed_unreachable_types,
num_changed_unreachable_types_filtered_out}): Define new data
members.
(sort_string_type_base_sptr_map): Declare new function.
* src/abg-comparison.cc (sort_string_type_base_sptr_map)
(diff_context::show_unreachable_types): Define new functions.
(corpus_diff::diff_stats::{num_added_unreachable_types,
num_added_unreachable_types_filtered_out,
net_num_added_unreachable_types,
net_num_removed_unreachable_types,
num_removed_unreachable_types_filtered_out,
num_removed_unreachable_types}): Define new member functions.
(diff_maps::insert_diff_node): Do not update the map "diff ->
impacted interfaces" if the current impacted interface is nil.
This happens if we are looking at a diff node for a change on a
type that is not reachable from any interfaces.
(corpus_diff::priv::ensure_lookup_tables_populated): Handle the
edit script for unreachable types.
(corpus_diff::priv::apply_supprs_to_added_removed_fns_vars_unreachable_types):
Rename
corpus_diff::priv::apply_suppressions_to_added_removed_fns_vars
into this.  Apply suppression specifications to added and removed
unreachable types as well.
(corpus_diff::priv::{added,deleted}_unreachable_type_is_suppressed):
Define new member functions.
(corpus_diff::priv::{count_unreachable_types,
changed_unreachable_types_sorted}): Likewise.
(corpus_diff::priv::apply_filters_and_compute_diff_stats): Update
statistics (including walking changed unreachable types to apply
categorization and redundancy filters to them) related to
unreachable types.
(corpus_diff::priv::emit_diff_stats): Emit diff stats related to
unreachable types.
(corpus_diff::priv::maybe_dump_diff_tree): Dump diff tree nodes
related to unreachable types.
(corpus_diff::{deleted_unreachable_types,
deleted_unreachable_types_sorted, added_unreachable_types,
added_unreachable_types_sorted, changed_unreachable_types,
changed_unreachable_types_sorted): Define new member functions.
(corpus_diff::has_changes): Take deleted/added/changed unreachable
types into account.
(corpus_diff::has_incompatible_changes): Take net removed/changed
unreachable types into account.
(corpus_diff::has_net_subtype_changes): Take net removed and
changed unreachable types into account.
(corpus_diff::has_net_changes): Take net removed/added/changed
unreachable types into account.
(corpus_diff::traverse): When traversing the components of a
corpus_diff node, make sure to traverse the changed unreachable
types of the corpus.
(leaf_diff_node_marker_visitor::visit_begin): Arrange for the fact
that the current topmost interface can be nil if we are looking at
types not reachable from global functions/variables.  Also, make
sure that only leaf nodes that are reachable from a global
function/variable are recorded as leaf nodes.
(compute_diff): In the overload for corpus_sptr, compute the
changes between types not reachable from global functions and
variables, if the user wishes that we do so.  Also, add more
comments.
(apply_suppressions): Update for the name change of the function
apply_suppressions_to_added_removed_fns_vars to
apply_supprs_to_added_removed_fns_vars_unreachable_types.
* include/abg-corpus.h
(corpus::{record_type_as_reachable_from_public_interfaces,
type_is_reachable_from_public_interfaces,
get_types_not_reachable_from_public_interfaces}): Declare new
member functions.
(corpus::recording_types_reachable_from_public_interface_supported):
Declare new virtual member function.
(corpus_group::get_public_types_pretty_representations): Declare
new member functons.
(corpus_group::recording_types_reachable_from_public_interface_supported):
Declare new virtual member function.
* src/abg-corpus-priv.h
(corpus::priv::{types_not_reachable_from_pub_ifaces_,
pub_type_pretty_reprs_}): Define new data members.
(corpus::priv::priv): Initialize the pub_type_pretty_reprs_ data
member because it's a pointer.
(corpus::priv::get_public_types_pretty_representations): Declare
new member function.
(corpus::priv::~priv): Declare a destructor.
* src/abg-corpus.cc
(corpus::priv::get_public_types_pretty_representations): Define
new member function.
(corpus::priv::~priv): Define new destructor to delete the new
pub_type_pretty_reprs_ member pointer.
(corpus::{record_type_as_reachable_from_public_interfaces,
type_is_reachable_from_public_interfaces,
get_types_not_reachable_from_public_interfaces,
recording_types_reachable_from_public_interface_supported}):
Define new member functions
(corpus_group::get_public_types_pretty_representations): Likewise.
* include/abg-diff-utils.h (struct deep_ptr_eq_functor): Document
the equality operator.  Also, add an overload to the equality
operator, for weak_ptr<T>.  The existing equality operator
overload was just for shared_ptr<T>.
* include/abg-fwd.h (is_user_defined_type): Declare function.
* include/abg-ir.h (operator!=(const decl_base_sptr&, const
decl_base_sptr&)): Declare new operator.
(type_maps::get_types_sorted_by_name): Declare
new member function.
(decl_base::{g,s}et_is_artificial): Declare new member function.
(function_decl::parameter::{g,s}et_artificial): Remove these
member functions.
* src/abg-ir.cc (operator!=(const decl_base_sptr&, const
decl_base_sptr&)): Define new operator.
(decl_base::priv::is_artificial_): Define new data
member.
(type_maps::priv::sorted_types_): Define new data member.
(struct type_name_comp): Define new comparison functor to sort
types based on their pretty representations.
(decl_base::priv::priv): Initialize it.
(decl_base::{g,s}et_is_artificial): Define new member functions.
(type_maps::get_types_sorted_by_name): Define new member function.
(is_user_defined_type): Define new function overloads.
(strip_typedef, function_type::{function_type, set_parameters}):
Adjust using decl_base::get_is_artificial rather than
function_decl::parameter::get_artificial.
(function_decl::parameter::priv::artificial_): Remove this data
member.
(function_decl::parameter::priv::priv): Adjust to the removal of
function_decl::parameter::priv::artificial_.  This constructor
does not take an "is_artificial" flag anymore.
(function_decl::parameter::parameter): Adjust to the removal of
the is_artificial flag from the arguments of the constructor of
function_decl::parameter::parameter::priv.
(function_decl::parameter::get_artificial): Remove this member
function.
* src/abg-reporter-priv.h (maybe_report_unreachable_type_changes):
Declare new function.
* src/abg-reporter-priv.cc
(maybe_report_unreachable_type_changes): Define new function.
* src/abg-default-reporter.cc (default_reporter::report): In the
overload for corpus_diff&, report added/removed/changed types that
are not reachable from global functions and variables using the
new function maybe_report_unreachable_type_changes.
* src/abg-leaf-reporter.cc (leaf_reporter::report): In the
overload for corpus_diff, report changes to types unreachable from
global functions or variables, using the new function
maybe_report_unreachable_type_changes.
* src/abg-dwarf-reader.cc (build_ir_node_from_die): When the user
requests that all types be loaded, record relevant types as
reachable from global functions and variables.
(build_enum_type, add_or_update_class_type)
(add_or_update_union_type): Read the 'is-artificial' DWARF
attribute and set the corresponding decl_base property
accordingly.
(finish_member_function_reading, strip_typedef)
(function_type::function_type): Adjust using
decl_base::get_is_artificial, rather than
function_decl::parameter::get_artificial.
* include/abg-reader.h
(consider_types_not_reachable_from_public_interfaces): Declare new
function.
* src/abg-reader.cc
(read_context::m_tracking_non_reachable_types): Add new data
member.
(read_context::read_context): Initialize it.
(read_context::tracking_non_reachable_types): Define accessors for
the new data member above.
(read_is_declaration_only): Re-indent.
(read_is_artificial): Define new helper function.
(build_function_parameter): Use the new read_is_artificial
function here, rather than open-coding it.
(build_enum_type_decl, build_class_decl, build_union_decl):
Support reading the 'is-artificial' property by using the new
read_is_artificial function.
(read_corpus_from_input): If the user wants us to take
non-reachable types into account, then make sure we do so.
(read_tracking_non_reachable_types, read_is_non_reachable_type):
Define new static functions.
(handle_element_node, build_type): Read the "is-non-reachable"
attribute on type element nodes if the user wants us to track
non-reachable types.
(consider_types_not_reachable_from_public_interfaces): Define new
function.
* src/abg-writer.cc (write_is_artificial): Define new static
helper function.
(annotate): Adjust using decl_base::get_is_artificial rather than
function_decl::parameter::get_artificial.
(write_enum_type_decl, write_class_decl_opening_tag)
(write_union_decl_opening_tag): Support writing the
"is-artificial" property, using the new write_is_artificial
function.
(write_function_type): Adjust this to use the new
write_is_artificial rather than open-coding writing the
'is-artificial' attribute.
(write_is_non_reachable)
(write_tracking_non_reachable_types): Define new static functions.
(write_enum_type_decl, write_class_decl_opening_tag)
(write_union_decl_opening_tag): Write the 'is-no-reachable'
attribute when applicable.
(write_corpus, write_corpus_group): Write the
'tracking-non-reachable-types' attribute when applicable.
* tools/abidiff.cc (options::options): Initialize ...
(options::show_all_types): ... new data member.
(display_usage): Add help string from the new
--non-reachable-types option.
(parse_command_line): Parse the new --non-reachable-types option.
(set_diff_context_from_opts): Set the
dwarf_reader::read_context::show_unreachable_types property.
(set_native_xml_reader_options): Define new
static function.
(main): Load all types when analyzing the DWARF or the ABIXML
files, if the user wants us to do so.
* tools/abipkgdiff.cc (options::show_all_types): Define new data
member.
(options::options): Initialize it.
(parse_command_line): Parse the --non-reachable-types option to
set the options::show_all_types data member.
(display_usage): Add a help string for the new
--non-reachable-types option.
(set_diff_context_from_opts): Set the
dwarf_reader::read_context::show_unreachable_types property based
on the options::show_all_type data member.
(compare): Configure the read context to load all types while
analyzing the DWARF info, depending on the options::show_all_type
data member.
* doc/manuals/abidiff.rst: Document the new --non-reachable-types
option added to abidiff above.
* doc/manuals/abipkgdiff.rst: Add documentation for the
--non-reachable-types option.
* tests/data/test-diff-suppr/test47-non-reachable-types-v{0,1}.c:
Source code files of test binary input.
* tests/data/test-diff-suppr/test47-non-reachable-types-suppr-{1,2,3,4,5}.txt:
New test input files.
* tests/data/test-diff-suppr/test47-non-reachable-types-report-{1,2,3,4,5,6,7,8,9,10}.txt:
New test reference output files.
* tests/data/test-diff-suppr/test47-non-reachable-types-v{0,1}.o.alltypes.abixml:
New test input abixml.
* tests/data/Makefile.am: Add the new test material to source
distribution.
* tests/test-diff-suppr.cc (in_out_specs): Add the new tests above
to this test harness.
* tests/data/test-abidiff/test-struct1-report.txt: Adjust.
* tests/data/test-diff-pkg/PR24690/flatpak-debuginfo-1.2.4-3.fc30.x86_64.rpm:
New input binary RPM.
* tests/data/test-diff-pkg/PR24690/flatpak-debuginfo-1.4.0-1.fc30.x86_64.rpm:
Likewise.
* tests/data/test-diff-pkg/PR24690/flatpak-devel-1.2.4-3.fc30.x86_64.rpm:
Likewise.
* tests/data/test-diff-pkg/PR24690/flatpak-devel-1.4.0-1.fc30.x86_64.rpm:
Likewise.
* tests/data/test-diff-pkg/PR24690/flatpak-libs-1.2.4-3.fc30.x86_64.rpm:
Likewise.
* tests/data/test-diff-pkg/PR24690/flatpak-libs-1.4.0-1.fc30.x86_64.rpm:
Likewise.
* tests/data/test-diff-pkg/PR24690/flatpak-libs-debuginfo-1.2.4-3.fc30.x86_64.rpm:
Likewise.
* tests/data/test-diff-pkg/PR24690/flatpak-libs-debuginfo-1.4.0-1.fc30.x86_64.rpm:
Likewise.
* tests/data/test-diff-pkg/PR24690/PR24690-report-0.txt: New test
reference output.
* tests/data/Makefile.am: Add the new test material above to
source distribution.
* tests/test-diff-pkg.cc (in_out_specs): Add the new test material
above to this test harness.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoSmall style fix in abg-default-reporter.cc
Dodji Seketeli [Tue, 26 Nov 2019 17:16:51 +0000 (18:16 +0100)]
Small style fix in abg-default-reporter.cc

Break a line longer than 80 characters.

* src/abg-default-reporter.cc (default_reporter::report): In the
overload for corpus_diff, break a line longer than 80 characters.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agodwarf-reader: add support for symbol namespaces in ksymtab entries
Matthias Maennich [Mon, 21 Oct 2019 16:09:50 +0000 (17:09 +0100)]
dwarf-reader: add support for symbol namespaces in ksymtab entries

Kernel v5.4 introduces Symbol Namespaces [1]. That changes the layout of
ksymtab entries in Kernel binaries. In particular, the kernel_symbol
entry gains a new member to represent the namespace. That change affects
binaries that have position relative relocations (we name that format
here V4_19_KSYMTAB_FORMAT) as well as those that don't
(PRE_V4_19_KSYMTAB_FORMAT). In any case there is an additional entry
that has the same size as the previous entries.

Since we iterate over the ksymtab entries to collect them, we need to
determine the correct size of these entries even though we do not
grab the namespace for ABI analysis purposes at this time.

In order to determine the size, we attempt to find the beginning of the
next entry by trying to read symbols with an increasing offset. Once we
succeed, we have the offset and therefore the size of one entry.

Since try_reading_first_ksymtab_entry() does already everything we need
to attempt to read a symbol from a beginning of a ksymtab, we only
needed to teach it to operate on an offset to read the potential second
entry.

'load_kernel_symbol_table' was determining the number of entries
unconditionally, even when we do have the unsupported case of a ksymtab
with relocations. Hence only load when needed.

* src/abg-dwarf-reader.cc
(read_context::try_reading_first_ksymtab_entry): Add
symbol_offset parameter.
(read_context::get_ksymtab_entry_size): Add support for variable
size ksymtab entries due to symbol namespaces.
(load_kernel_symbol_table): only load nb_entries when needed

[1] https://lore.kernel.org/lkml/20190906103235.197072-1-maennich@google.com/

Signed-off-by: Matthias Maennich <maennich@google.com>
5 years agodwarf-reader: refactor try_reading_first_ksymtab_entry_using{pre,}_v4_19_format
Matthias Maennich [Mon, 21 Oct 2019 16:09:49 +0000 (17:09 +0100)]
dwarf-reader: refactor try_reading_first_ksymtab_entry_using{pre,}_v4_19_format

Avoid code duplication and increase maintainebility of these helper
functions. As their only difference was the application of position
relative relocations, consolidate them and add a flag for exactly this
feature.

This is purely stylistic and not changing functionality.

* src/abg-dwarf-reader.cc(try_reading_first_ksymtab_entry):
New function to consolidate functionality for
try_reading_first_ksymtab_entry_using_{pre,}v4_19_format functions.
(try_reading_first_ksymtab_entry_using_v4_19_format,
try_reading_first_ksymtab_entry_using_pre_v4_19_format):
refactor to use try_reading_first_ksymtab_entry

Signed-off-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoBug 25128 - Handle decl-only classes that differ only in size
Dodji Seketeli [Mon, 28 Oct 2019 12:23:39 +0000 (13:23 +0100)]
Bug 25128 - Handle decl-only classes that differ only in size

Because DWARF sometimes emit decl-only classes (real one, with no
members) with a size property, and the rest of the time, would emit
the same decl-only class without a size property, comparing the two
might yield some false positives.

This patch handles those beasts when comparing classes.

* include/abg-comp-filter.h (is_decl_only_class_with_size_change):
Declare an overload.
* include/abg-fwd.h (look_through_decl_only_class): Declare an
overload.
* src/abg-comp-filter.cc (is_decl_only_class_with_size_change):
Define an overload that takes class_or_union& type. Re-write the
previous overload in terms of this new one.
* src/abg-ir.cc (look_through_decl_only_class): Define a new
overload that takes a class_or_union&.  Rewrite the previous
overload in terms of this one.
(equals): In the overload for class_or_union&, use
is_decl_only_class_with_size_change to detect cases of decl-only
classes that differ only by their size attribute and avoid
comparing them.
* tests/data/test-annotate/test21-pr19092.so.abi: Adjust.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise.
* tests/data/test-diff-filter/test41-report-0.txt: Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoBug 25128 - Leaf diff reporter shouldn't compare decl-only classes
Dodji Seketeli [Thu, 24 Oct 2019 08:42:19 +0000 (10:42 +0200)]
Bug 25128 - Leaf diff reporter shouldn't compare decl-only classes

The leaf_diff_node_marker_visitor pass which collects leaf diff nodes
for the leaf diff reporter considers bogus decl-only classes (that have the
is-declaration-only flag set, are empty, and yet have a non-nil size
property) originated from bogus DWARF.

The leaf reporter thus potentially reports size changes among
decl-only classes, which does not make sense.  Two decl-only classes
of the same name should always be considered equal, in this context.

This patch thus teaches the leaf_diff_node_marker_visitor to avoid
collecting a leaf diff node that is about a size change on a true
decl-only class.

* include/abg-comp-filter.h (is_decl_only_class_with_size_change):
Declare new function.
* src/abg-comp-filter.cc (is_decl_only_class_with_size_change):
Define new function.
* src/abg-comparison.cc
(leaf_diff_node_marker_visitor::visit_begin): Use the newly
defined is_decl_only_class_with_size_change above to ignore bogus
decl-only classes with a size change.
* tests/data/test-diff-suppr/test45-abi-report-1.txt: New test input.
* tests/data/test-diff-suppr/test45-abi-wl.xml: Likewise.
* tests/data/test-diff-suppr/test45-abi.xml: Likewise.
* tests/data/test-diff-suppr/test45-abi.suppr.txt: New reference
output for the test input above.
* tests/data/test-diff-suppr/test46-PR25128-base.xml: New test input.
* tests/data/test-diff-suppr/test46-PR25128-new.xml: Likewise.
* tests/data/test-diff-suppr/test46-PR25128-report-1.txt: New
reference input for the test input above.
* tests/data/Makefile.am: Add the new test material to source distribution.
* tests/test-diff-suppr.cc (in_out_spec): Add the new test input
above to this test harness.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoBug 25095 - Apply symbol white lists to ELF symbols
Dodji Seketeli [Fri, 18 Oct 2019 09:16:38 +0000 (11:16 +0200)]
Bug 25095 - Apply symbol white lists to ELF symbols

Right now, the symbol names that are part of a Linux Kernel ABI
white list are matched against function and variable names that appear
in the DWARF information.

In other words, when Libabigail processes the Linux Kernel ABI
whitelist, it generates a suppression specifications which keeps
functions and variables (as described by DWARF) whose names match the
symbol names specified in the white list.  All other functions and
variables are dropped.  But that doesn't apply to ELF symbols.
Libabigail generates no suppression at all for ELF symbols.  It only
considers variables and functions described in the debug information.

With this patch, Libabiagil now generates a suppression specification
which keeps functions and variables whose ELF symbol name match the
symbol names specified in the whitelist.  The suppression
specification also drops ELF symbols whose name don't match the names
specified in the white list.

Note that this patch uses the previous commit which description is:
    "Support symbol_name_not_regexp in [suppress_{function, variable}]"

* src/abg-tools-utils.cc (gen_suppr_spec_from_kernel_abi_whitelist):
Generate a suppression specification which considers the name of
the symbol associated to a function/variable, rather than just the
name of said function/variable.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoSupport symbol_name_not_regexp in [suppress_{function, variable}]
Dodji Seketeli [Fri, 18 Oct 2019 08:13:41 +0000 (10:13 +0200)]
Support symbol_name_not_regexp in [suppress_{function, variable}]

In the suppress_function and suppress_variable directives of the
suppression specification language, we lack the
'symbol_name_not_regexp' properties, that would allow users to specify
which (function/variable) symbols to *keep* as opposed to specifying
which symbols to suppress.

This patch adds that feature.  That will later allow us to make the
linux kernel symbol white lists[1] functionality use this feature;
that is, upon analysing the content of a kernel symbol whitelist which
lists a symbol named "foo", Libabigail would automatically generate a
suppression specification which contains, e.g a 'suppress_function"
directive that has this new 'symbol_name_not_regexp' property which
value is set to "foo".

Note that the patch makes sure that feature is supported when
analyzing both abixml and DWARF formats.

[1]: You can learn about what a Linux Kernel symbols white list is by
reading about it at
https://sourceware.org/libabigail/manual/kmidiff.html#environment.

* doc/manuals/libabigail-concepts.rst: Document the new
symbol_name_not_regexp properties for the
suppress_{function,variable} directives.
* include/abg-suppression.h
({function,variable}_suppression::{g,s}et_symbol_name_not_regex_str):
Declare new member functions.
* src/abg-dwarf-reader.cc
(read_context::is_elf_symbol_suppressed): Define new member functions.
(read_context::{load_symbol_maps_from_symtab_section,
populate_symbol_map_from_ksymtab,
populate_symbol_map_from_ksymtab_reloc}): Drop suppressed symbols
when reading symbol tables.
({function,variable}_is_suppressed): Consider that in C, the
linkage name is _by default_ the same as the function/variable
name. Remove local variable.
* include/abg-ir.h (elf_symbol_is_{function,variable}): Add ...
* src/abg-ir.cc (elf_symbol_is_{function,variable}): ... new
functions.
* src/abg-reader.cc (build_elf_symbol): Take an additional boolean
to detect and drop suppressed symbols.
(build_elf_symbol_db): Adjust the call to build_elf_symbol to make
it detect and drop suppressed symbols.
(read_corpus_from_input): Be mindful that the set of symbols for a
given corpus can be empty because of suppression specifications.
* src/abg-suppression-priv.h
({function,variable}_suppression::priv::symbol_name_not_regex[_str_]):
Add new data members.
(function,variable}_suppression::priv::get_symbol_name_not_regex):
Add new member functions.
({function,variable}_is_suppressed): Guard against empty name.
(is_elf_symbol_suppressed): Define new function template.
* src/abg-suppression.cc
({function,variable}_suppression::{g,s}et_symbol_name_not_regex_str):
Define new member functions.
({function,variable}_suppression::suppresses_function)
(suppression_matches_{function,variable}_sym_name)
(read_{function,variable}_suppression): Support the new
"symbol_name_not_regex" property.
* tests/data/test-diff-suppr/test44-suppr-sym-name-not-regexp-report-1.txt:
New test reference report.
* tests/data/test-diff-suppr/test44-suppr-sym-name-not-regexp-report-2.txt:
Likewise.
* tests/data/test-diff-suppr/test44-suppr-sym-name-not-regexp-v{0,1}.c:
Sources of the new test input.
* tests/data/test-diff-suppr/test44-suppr-sym-name-not-regexp-v{0,1}.o:
New test input binaries.
* tests/data/test-diff-suppr/test44-suppr-sym-name-not-regexp-v{0,1}.o.abi:
New test input abixml files.
* tests/data/test-diff-suppr/test44-suppr-sym-name-not-regexp.suppr.txt:
Next test suppression specification.
* tests/data/Makefile.am: Add the new test material above to
source distribution.
* tests/test-diff-suppr.cc (in_out_specs): Add the input tests
above to the test harness.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years ago[abg-comparison.cc] Fix comments typo
Dodji Seketeli [Thu, 24 Oct 2019 09:56:34 +0000 (11:56 +0200)]
[abg-comparison.cc] Fix comments typo

* src/abg-comparison.cc
(leaf_diff_node_marker_visitor::visit_begin): Fix typo in comment.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agokmidiff: fix help message
Matthias Maennich [Fri, 4 Oct 2019 15:26:31 +0000 (16:26 +0100)]
kmidiff: fix help message

Add the missing line breaks.

* tools/kmidiff.c (display_usage): add missing line breaks to
help text

Signed-off-by: Matthias Maennich <maennich@google.com>
5 years agodwarf-reader: read_corpus_from_elf: unconditionally load elf properties
Matthias Maennich [Fri, 4 Oct 2019 15:27:07 +0000 (16:27 +0100)]
dwarf-reader: read_corpus_from_elf: unconditionally load elf properties

When loading the corpus from elf while specifying a whitelist, we might
be able to ignore the symbol table. In any case we have to load the elf
properties into the context, such as the binary's architecture.
Otherwise they are missing from the internal / xml representation.
Previously, elf properties were not loaded when a whitelist was
specified. Fix that.

* src/abg-dwarf-reader.cc (read_corpus_from_elf):
unconditionally load elf properties into context

Signed-off-by: Matthias Maennich <maennich@google.com>
5 years agoPR25058 - Better support fn DIEs referring to symbols using DW_AT_ranges
Dodji Seketeli [Fri, 4 Oct 2019 08:05:42 +0000 (10:05 +0200)]
PR25058 - Better support fn DIEs referring to symbols using DW_AT_ranges

In the previous commit 2f7248f, we were just taking the first address
referred to by the DW_AT_ranges attribute as being the address of the
symbol of the function we are looking at.  But there can be cases
where this is not true, as explained at
https://sourceware.org/bugzilla/show_bug.cgi?id=25058#c7.

We really need to get the first address that represents an exported
and defined function symbol, which is pointed to by the DW_AT_ranges
attribute.

And this is what this patch does.

* src/abg-dwarf-reader.cc
(read_context::get_first_exported_fn_address_from_DW_AT_ranges):
Rename read_context::get_first_address_from_DW_AT_ranges into
this.  Walk through the addresses referred to by the DW_AT_ranges
attribute until we find one that is for an exported function
symbol, rather than just picking the first address of the set.
(read_context::get_function_address): Adjust.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoPR25058 - Support decl DIEs referring to symbols using DW_AT_ranges
Dodji Seketeli [Thu, 3 Oct 2019 15:15:28 +0000 (17:15 +0200)]
PR25058 - Support decl DIEs referring to symbols using DW_AT_ranges

Usually, function DIEs (DW_TAG_subprogram) refer to the address of the
underlying ELF symbol by using the DW_AT_low_pc attribute.  However,
there are cases where it does so by using the DW_AT_ranges attribute.
In those cases, the first address of the sequence defined in the value
of that attribute is the address of the ELF symbol.

The problem is that the DWARF reader of Libabigail fails to get the
address of the underlying ELF symbol when the DW_AT_low_pc attribute
is missing.  Rather, it should then look at the value of the
DW_AT_ranges attribute instead.

This is what this patch does.

* src/abg-dwarf-reader.cc
(read_context::get_first_address_from_DW_AT_ranges): Define new
member function.
(read_context::get_function_address): Use the new
read_context::get_first_address_from_DW_AT_ranges here.
* tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt:
New reference test output.
* tests/data/test-diff-dwarf/PR25058-liblttng-ctl.so: New test
input binary.
* tests/data/test-diff-dwarf/PR25058-liblttng-ctl2.10.so: New test
input binary.
* tests/data/Makefile.am: Add the new test materials above to
source distribution.
* tests/test-diff-dwarf.cc (in_out_specs): Add the new input test
input binary files to this test harness.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoFix thinkos in DW_FORM_strx detection in configure.ac
Dodji Seketeli [Tue, 1 Oct 2019 14:50:37 +0000 (16:50 +0200)]
Fix thinkos in DW_FORM_strx detection in configure.ac

My patch "568dee1 PR25042 - Support string form DW_FORM_strx{1,4} from
DWARF 5" introduced a thinko in configure.ac.  The thinko triggers a
regression test issue on old systems where we don't support
DW_FORM_strx from DWARF 5.  Fixed thus.

* configure.ac: Fix thinko when setting the HAVE_DW_FORM_strx
macro.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoFix a typo in a comment of abg-dwar-reader.cc
Dodji Seketeli [Tue, 1 Oct 2019 12:14:57 +0000 (14:14 +0200)]
Fix a typo in a comment of abg-dwar-reader.cc

* src/abg-dwarf-reader.cc (compare_dies_string_attribute_value):
Fix a typo in the comment of this function.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoPR25042 - Support string form DW_FORM_strx{1,4} from DWARF 5
Dodji Seketeli [Mon, 30 Sep 2019 15:36:58 +0000 (17:36 +0200)]
PR25042 - Support string form DW_FORM_strx{1,4} from DWARF 5

* configure.ac: Detect the presence of the DW_FORM_strx{1,4}
enumerators.
* src/abg-dwarf-reader.cc (form_is_DW_FORM_strx): Define new
function.
(compare_dies_string_attribute_value): Use the new
form_is_DW_FORM_strx here.
* tests/data/Makefile.am: Add the new test input files below to
source distribution.
* tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0:
New binary test input file.
* tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.6.0.0.abi:
Reference output of the new binary test input file.
* tests/test-read-dwarf.cc (in_out_specs): Add the input test
files above to the test harness, for platforms that support the
DW_FORM_strx form.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoSupport the "name_not_regexp" property in the [suppress_type] section
Dodji Seketeli [Wed, 4 Sep 2019 08:36:31 +0000 (10:36 +0200)]
Support the "name_not_regexp" property in the [suppress_type] section

When writting a suppression specification in which the user wants to
keep a family of types (whose names set is specified by a regular
expression) and suppress/drop all other types, one needs to write
something like:

[suppress_type]
  name_regexp = (?!the-regexp-of-the-types-to-keep)

It would be nicer (like what is done for other properties that take
regular expressions as value in suppression specifications) to be able
to write:

[suppress_type]
  name_not_regexp = the-regexp-of-types-to-keep

This patch does just that.

It augments the abigail::suppr::type_suppression type to make it carry
the new 'name_not_regex' property.  It updates the suppression engine
to take the 'name_not_regex' property into account when interpreting
instances of abigail::suppr::type_suppression.  The parser for type
suppression directives is updated to recognize the new name_not_regexp
property.  The manual has been updated accordingly to describe the new
property.  New regression tests have been added.

* doc/manuals/libabigail-concepts.rst: Update this to document the
new name_not_regexp property of the suppress_type directive.
* include/abg-suppression.h
(type_suppression::{g,s}et_type_name_not_regex_str): Declare new accessors.
* src/abg-suppression-priv.h
(type_suppression::priv::{type_name_not_regex_str_,
type_name_not_regex_}): Define new data members.
(type_suppression::priv::{get_type_name_not_regex,
set_type_name_not_regex, get_type_name_not_regex_str,
set_type_name_not_regex_str}): Define new member functions.
* src/abg-suppression.cc
(type_suppression::get_type_name_regex_str): Fix comments.
(type_suppression::{set_type_name_not_regex_str,
get_type_name_not_regex_str}): Define new data members.
(suppression_matches_type_name): Adapt to support the new
type_name_not_regex property.
(read_type_suppression): Support parsing the type_name_not_regexp
property.
* tests/data/test-diff-suppr/test42-negative-suppr-type-report-0.txt:
New test reference output.
* tests/data/test-diff-suppr/test42-negative-suppr-type-report-1.txt: Likewise.
* tests/data/test-diff-suppr/test42-negative-suppr-type-suppr-1.txt:
New test input.
* tests/data/test-diff-suppr/test42-negative-suppr-type-suppr-2.txt: Likewise.
* tests/data/test-diff-suppr/test42-negative-suppr-type-v0.{cc, o}: Likewise.
* tests/data/test-diff-suppr/test42-negative-suppr-type-v1.{cc,
o}: Likewise.
* tests/data/Makefile.am: Add the test files above to source
distribution.
* tests/test-diff-suppr.cc (int_out_specs): Add the new tests to
the harness.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoBetter propagation of suppressed-ness to function types
Dodji Seketeli [Wed, 4 Sep 2019 09:10:31 +0000 (11:10 +0200)]
Better propagation of suppressed-ness to function types

In the comparison engine, when a sub-type of a function type (say, a
parameter type size change) has been suppressed, this suppression is
not necessarily well propagated to the function carrying the function
type, because the parameter type size, for instance, is considered as
a type local change to that function; and we generally don't propagate
suppression to a non-suppressed parent diff node that already carries
a local change.

This leads to an empty change report for the function we are looking
at because the only sub-type change has been suppressed.

This patch properly propagates the suppressed-ness in that case, so
that the parent function diff node is suppressed as well.

* src/abg-comparison.cc
(suppression_categorization_visitor::visit_end): Propagate
suppression-ness from suppressed function type diff node to its
parent function node if the latter doesn't have any local non-type
change.
* tests/data/test-diff-suppr/test43-suppr-direct-fn-subtype-report-1.txt:
New test reference output.
* tests/data/test-diff-suppr/test43-suppr-direct-fn-subtype-suppr-1.txt:
New test input suppression file.
* tests/data/test-diff-suppr/test43-suppr-direct-fn-subtype-v{0,1}.cc:
Source code of input binary file.
* tests/data/test-diff-suppr/test43-suppr-direct-fn-subtype-v{0,1}.o:
Input binary files.
* tests/data/Makefile.am: Add the new test input files above to
source distribution.
* tests/test-diff-suppr.cc (in_out_specs): Add the test input to
test harness.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years ago[has_type_change] Better detect type size changes
Dodji Seketeli [Wed, 4 Sep 2019 09:07:59 +0000 (11:07 +0200)]
[has_type_change] Better detect type size changes

While looking at something else, I noticed that we could be missing
type size changes on function parameters when using has_type_change.

Fixed thus.

* src/abg-comp-filter.cc (has_type_change): Support function
parameters.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoFix reading of relocation sections when endianness mismatches
Dodji Seketeli [Mon, 23 Sep 2019 13:55:08 +0000 (15:55 +0200)]
Fix reading of relocation sections when endianness mismatches

When the endianness of the ELF binary differs from the endianness of
the host, some byte swapping needs to happen when we read the reloc
section to either determine the format of the kernel symbol table or
to get the set of symbols referenced by the kernel symbol table.

So we need to use elf_getdata rather than elf_rawdata to read the data
from the reloc section, because the former handles the proper byte
swapping for us.

This patch does just that and thus fixes the build breakage that is
occuring when running the testreaddwarf test on s390x (big endian),
especially when trying to read the AARCH64 little endian binary
data/test-read-dwarf/PR25007-sdhci.ko.

* src/abg-dwarf-reader.cc
(read_context::{get_ksymtab_format_module,
populate_symbol_map_from_ksymtab_reloc}): Use elf_getdata rather
than elf_rawdata.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoGuard testing v4.19+ AARCH64 kernel module loading for EL6 support
Dodji Seketeli [Fri, 20 Sep 2019 13:59:56 +0000 (15:59 +0200)]
Guard testing v4.19+ AARCH64 kernel module loading for EL6 support

When analyzing an AARCH64 linux kernel module built with support for
either R_AARCH64_ABS64 or R_AARCH64_PREL32 relocations, we need these
macros to be defined in elf.h (i.e a recent enough version of libelf),
otherwise we cannot properly support those kernel modules using the
scheme that uses the relocation table of the __ksymtab and
__ksymtab_gpl sections to read those sections.

In the future, I think we should automatically fallback to another way
of trying to read those sections if those macros are not defined, and
emit a message hinting at what is happening, when in verbose mode.  I
am keeping it as is for the moment, so that we can get a better case
of the when these macros are not defined and whatnot.

In the mean time, this patch conditionalizes the test that reads a
kernel module build with support for these relocations to avoid
running it on platform that support these relocations.

* tests/test-read-dwarf.cc: Do not run the test on
          PR25007-sdhci.ko if the macros R_AARCH64_PREL32 and
          R_AARCH64_ABS64 are not defined.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoRemove the elf_symbol::get_value property
Dodji Seketeli [Thu, 19 Sep 2019 15:36:45 +0000 (17:36 +0200)]
Remove the elf_symbol::get_value property

Now that there are proper facilities to lookup ELF symbols inside the
ELF/DWARF reader and get a native GElf_Sym type instance (from
libelf), we don't need to carry the value of the symbol (that is
relevant only that low level anyway) in the abigail::ir::elf_symbol
type.

This patch removes that property.

* include/abg-ir.h (elf_symbol::{elf_symbol, create}): Remove the
'val' parameter.
* src/abg-dwarf-reader.cc (elf_symbol::get_value): Remove this
member function declaration.
(lookup_symbol_from_sysv_hash_tab)
(lookup_symbol_from_gnu_hash_tab, lookup_symbol_from_symtab)
(create_default_var_sym, create_default_fn_sym)
(read_context::lookup_elf_symbol_from_index): Adjust calls to
creating elf_symbol instances.
* src/abg-ir.cc (elf_symbol::priv::value_): Remove this data
member.
(elf_symbol::{priv::priv, elf_symbol, create): Adjust.
* src/abg-reader.cc (build_elf_symbol): Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoBug 25007 - Don't use section-relative symbol values on ET_REL binaries
Dodji Seketeli [Thu, 19 Sep 2019 13:10:36 +0000 (15:10 +0200)]
Bug 25007 - Don't use section-relative symbol values on ET_REL binaries

In relocatable files, two symbols listed in the .symtab section can
have the same value and yet be different.  That is because those
symbols can be *defined* in different sections.  And the value of
those symbols represent addresses (offsets) within their own
respective sections (a.k.a section-relative addresses).

In the same time, symbol address as referred-to in the DWARF
information are *not* section-relative, rather, they are relative to
the beginning of the whole binary.

Until now, the DWARF-referred-to symbol addresses were translated into
section-relative addresses, so that they could be compared to the
other section-relative addresses we were getting from listing the
symbols and their values from the .symtab section.  The problem with
that approach is that, during the translation from binary-relative to
section-relative addresses we were wrongly assuming that all symbols
referenced from the DWARF were defined in the .text section.  This is
wrong especially for ET_REL files because they could be defined in
sections named .foo.text or .bar.text, for instance.

This leads to issues where we wrongly consider that two symbols having the
same value are the same.  Because we wrongly assume that they are all
defined in the same .text section.

This patch fixes this problem by translating the section-relative
addresses we see in .symtab into binary-relative addresses by adding
the address of the section to the section-relative address.  Those
binary-addresses can thus safely be compared to the binary-relative
addresses we see in the DWARF.  And also, when two symbols have the
same binary-relative address, we can now safely assume that they are
the same -- they are aliases, basically.

* src/abg-dwarf-reader.cc
(read_context::{lookup_native_elf_symbol_from_index,
maybe_adjust_et_rel_sym_addr_to_abs_addr}): Define new member
functions.
(read_context::lookup_elf_symbol_from_index): Add a new overload.
Write the old overloads in terms of the new one.
(read_context::{load_symbol_maps_from_symtab_section,
populate_symbol_map_from_ksymtab_reloc}): Use the new
maybe_adjust_et_rel_sym_addr_to_abs_addr function to translate the
symbol value/address into a binary-relative address before adding
it to the addr->sym maps.
(read_context::maybe_adjust_{fn, var}_sym_address): Do not adjust
DWARF-referred-to addresses of ET_REL symbols anymore.
* tests/data/test-read-dwarf/PR25007-sdhci.ko: New binary test input.
* tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: ABI
representation of the above.
* tests/test-read-dwarf.cc: Add the new test input to the harness.
* tests/data/test-diff-dwarf/test28-vtable-changes-report-0.txt: Adjust.
* tests/data/test-diff-filter/test20-inline-report-0.txt: Likewise.
* tests/data/test-diff-filter/test20-inline-report-1.txt: Likewise.
* tests/data/test-diff-filter/test41-report-0.txt: Likewise.
* tests/data/test-diff-filter/test9-report.txt: Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoDetect the presence of R_AARCH64_{ABS64, PREL32} macros
Dodji Seketeli [Wed, 18 Sep 2019 07:16:27 +0000 (09:16 +0200)]
Detect the presence of R_AARCH64_{ABS64, PREL32} macros

The patch:

 "e687032 Support pre and post v4.19 ksymtabs for Linux kernel modules"

introduces the use of the R_AARCH64_{ABS64, PREL32} macros.  However,
some older "elf.h" don't define these.  When compiling on these older
platforms, we thus need to avoid using these new macros.

With this patch, the configure system detects the presence of these
macros and defines the HAVE_R_AARCH64_{ABS64, PREL32}_MACRO macros
accordingly.

Note that just to comply with what's in there in the code already, we
don't directly do "#ifdef R_AARCH64_ABS64", but rather "#ifdef
HAVE_R_AARCH64_ABS64_MACRO", to allow cases where we want to
artificially disable the "feature" at configure time, in the future.

* configure.ac: Define macros HAVE_R_AARCH64_{ABS64, PREL32}_MACRO
if the macros R_AARCH64_{ABS64, PREL32} are present.
* src/abg-dwarf-reader.cc
(read_context::get_ksymtab_format_module): Conditionalize the use
of R_AARCH64_{ABS64, PREL32} using HAVE_R_AARCH64_{ABS64, PREL32}_MACRO.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoSupport pre and post v4.19 ksymtabs for Linux kernel modules
Jessica Yu [Tue, 3 Sep 2019 15:35:14 +0000 (17:35 +0200)]
Support pre and post v4.19 ksymtabs for Linux kernel modules

As described in commit ad8c2531fb9, the format of the Linux kernel
ksymtab changed in v4.19 to use relative references instead of absolute
references. This changes the type of relocations emitted for ksymtab
sections to be place-relative 32-bit relocations instead of absolute
relocations. One side-effect of this is that libdwfl will not relocate
the ksymtab sections due to the PC-relative relocations. This breaks
load_kernel_symbol_table() for kernel modules because it only reads in
zeros from the unrelocated ksymtab section and is subsequently unable to
determine what exported symbols it refers to. Since a vmlinux binary is
already fully linked and relocated (and therefore we can read its
ksymtab section just fine), this problem is only relevant to Linux
kernel modules.

To work around this, we utilize the ksymtab relocation sections to
determine which symbols the ksymtab entries refer to. We do this by
inspecting each relocation's r_info field for the symbol table index and
from there we are able to read each symbol's value and subsequently add
that to the set of exported symbols.

In addition, for Linux kernel modules, we can utilize relocation types
to implement a new heuristic to determine the ksymtab format we have.
The presence of PC-relative relocations suggest the new v4.19 format,
and absolute relocation types suggest the old pre v4.19 format.

        * include/abg-ir.h (elf_symbol::{elf_symbol, create}): Take new
symbol value and shndx parameters.
        (elf_symbol::{get_value, get_shndx}): Declare new accessors.
        * src/abg-ir.cc (elf_symbol::priv::{value_, shndx_}): New data
members.
        (elf_symbol::priv::priv): Adjust.
        (elf_symbol::elf_symbol): Take new value and is_linux_string_cst
parameters.
        (elf_symbol::create): Likewise.
        (elf_symbol::{get_value, get_is_linux_string_cst}): Define new
accessors.
        * src/abg-reader.cc (build_elf_symbol): Adjust.
        * src/abg-dwarf-reader.cc (binary_is_linux_kernel)
(binary_is_linux_kernel): New static functions.
(lookup_symbol_from_sysv_hash_tab)
        (lookup_symbol_from_gnu_hash_tab)
        (lookup_symbol_from_symtab): Adjust.
        (read_context::{ksymtab_reloc_section_,
ksymtab_gpl_reloc_section_, ksymtab_strings_section_}): New data
members.
        (read_context::read_context): Initialize ksymtab_reloc_section_,
ksymtab_gpl_reloc_section_, ksymtab_strings_section_.
        (read_context::{find_ksymtab_reloc_section,
find_ksymtab_gpl_reloc_section, find_ksymtab_strings_section,
find_any_ksymtab_reloc_section, get_ksymtab_format_module,
populate_symbol_map_from_ksymtab,
populate_symbol_map_from_ksymtab_reloc, is_linux_kernel_module}):
New member functions.
        (read_context::load_kernel_symbol_table): Adjust to call either
populate_symbol_map_from_ksymtab{_reloc,} depending on ksymtab
format.
        (read_context::get_ksymtab_format): Adjust to call
get_ksymtab_format_module for linux kernel modules.
        (read_context::lookup_elf_symbol_from_index): Adjust.
        (create_default_var_sym, create_default_fn_sym): Adjust.

Signed-off-by: Jessica Yu <jeyu@kernel.org>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoSerialize canonical types to avoid testing if types have been emitted
Dodji Seketeli [Fri, 2 Aug 2019 15:06:37 +0000 (17:06 +0200)]
Serialize canonical types to avoid testing if types have been emitted

When emitting abixml, profiling shows that we spend a great deal of
time testing if a given type has been emitted already, to avoid
emitting a given type more than once.  This makes the serialization
phase take more time than the binary analysis phase!

This patch leverages the fact that we already have the set of
canonical types in the system.  While emitting that set entirely, we
don't need to test if a type has been emitted already because we know
by definition that every type is present just once in that set, more
or less.  OK, because there are also types that don't have canonical
types (for instance, declaration-only class/structs), we'll still have
to check of those types have already been emitted, but this is a very
small set to handle.

The patch thus organizes the canonical types per scope, so that when
emitting a scope and the canonical types within it, the type is
emitted in its correct namespace.

Then, when emitting a translation unit and each namespaces in it, the
patch emits the canonical types of those namespaces.

The patch arranges for some ancillary things that are needed to make
the whole picture be coherent enough for things to keep working.

Testing shows that we gained ~ 30% of performance by doing this, while
analysing the whole linux kernel 5.1 version.  We went from ~ 3m30s
minutes to less than 2m30s.

With this patch, the serialization phase now takes less time than the
analysis time.

* include/abg-fwd.h (is_decl_slow)
(peel_pointer_or_reference_type): Declare new functions.
* include/abg-ir.h (struct canonical_type_hash): Define new type.
(type_base_ptr_set_type, type_base_ptrs_type)
(type_base_sptrs_type, canonical_type_sptr_set_type): Define new
typedefs.
(environment::get_canonical_types_map): Declare new member
function.
(scope_decl::{get_canonical_types, get_sorted_canonical_types}):
Declare new member functions.
* src/abg-ir.cc (is_ptr_ref_or_qual_type)
(peel_pointer_or_reference_type, is_decl_slow): Define new
functions.
(environment::{get_canonical_types_map}): Define new member
functions.
(canonical_type_hash::operator()): Likewise.
(scope_decl::{get_canonical_types, get_sorted_canonical_types}):
Likewise.
(struct type_topo_comp): Define new comparison functor type.
(environment::{sorted_canonical_types_}): Define new data member.
(scope_decl::priv::{canonical_types_, sorted_canonical_types_}):
Likewise.
(scope_decl::is_empty): Take the presence of canonical types into
account when determining if a scope is empty or not.
(is_decl): Make this work for cases where the artifact at hand is
a type which has a declaration, as opposed to being a pure
declaration like a variable or a function.
(canonicalize): Add the canonical type the list of canonical types
of its scope.
* src/abg-dwarf-reader.cc (read_context::die_is_in_cplus_plus):
Define new member function.
* src/abg-writer.cc (write_type, write_canonical_types_of_scope):
Define new static functions.
(fn_type_ptr_set_type): Define new typedef.
(write_context::{m_referenced_fn_types_set,
m_referenced_non_canonical_types_set}): Add new data members.
(write_context::m_referenced_types_set): Renamed
m_referenced_types_map into this.
(write_context::get_referenced_types): Adjust.
(write_context::get_referenced_{function_types,
non_canonical_types}):
(write_context::record_type_as_referenced): Adjust to add the
referenced type in the proper set which would be one of the three
following: write_context::{get_referenced_types,
get_referenced_function_types,
get_referenced_non_canonical_types}.
(write_context::{type_is_referenced, clear_referenced}): Adjust.
(write_translation_unit): Use the new
write_canonical_types_of_scope.  Also emit declaration-only
classes that have member types.  Do not test if a given type of a
given scope has been emitted, in general, as this was super slow
given the number of types.  Emit referenced function types (as
these don't belong to any scope).  Rather than using the expensive
"is_function_type" on *all* the referenced types, just walk the
set write_context::get_referenced_function_types.  Likewise,
rather than using type_base::get_naked_canonical_type on
*all* the referenced types, just walk the set
write_context::get_referenced_non_canonical_types
(write_class): Use write_canonical_types_of_scope here.
* tools/abilint.cc (main): Support linting corpus group abixml
files.
* tests/data/test-annotate/libtest23.so.abi: Adjust.
* tests/data/test-annotate/libtest24-drop-fns-2.so.abi: Likewise.
* tests/data/test-annotate/libtest24-drop-fns.so.abi: Likewise.
* tests/data/test-annotate/test-anonymous-members-0.o.abi: Likewise.
* tests/data/test-annotate/test0.abi: Likewise.
* tests/data/test-annotate/test1.abi: Likewise.
* tests/data/test-annotate/test13-pr18894.so.abi: Likewise.
* 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/test2.so.abi: Likewise.
* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise.
* tests/data/test-annotate/test21-pr19092.so.abi: Likewise.
* tests/data/test-annotate/test4.so.abi: Likewise.
* tests/data/test-annotate/test6.so.abi: Likewise.
* tests/data/test-annotate/test7.so.abi: Likewise.
* tests/data/test-annotate/test8-qualified-this-pointer.so.abi: Likewise.
* tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi: Likewise.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Likewise.
* tests/data/test-read-dwarf/PR24378-fn-is-not-scope.abi: Likewise.
* tests/data/test-read-dwarf/libtest23.so.abi: Likewise.
* tests/data/test-read-dwarf/libtest24-drop-fns-2.so.abi: Likewise.
* tests/data/test-read-dwarf/libtest24-drop-fns.so.abi: Likewise.
* tests/data/test-read-dwarf/test0.abi: Likewise.
* tests/data/test-read-dwarf/test1.abi: Likewise.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Likewise.
* tests/data/test-read-dwarf/test11-pr18828.so.abi: Likewise.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Likewise.
* tests/data/test-read-dwarf/test13-pr18894.so.abi: Likewise.
* tests/data/test-read-dwarf/test14-pr18893.so.abi: Likewise.
* tests/data/test-read-dwarf/test15-pr18892.so.abi: Likewise.
* tests/data/test-read-dwarf/test16-pr18904.so.abi: Likewise.
* tests/data/test-read-dwarf/test17-pr19027.so.abi: Likewise.
* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi: Likewise.
* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi: Likewise.
* tests/data/test-read-dwarf/test2.so.abi: Likewise.
* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi: Likewise.
* tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise.
* tests/data/test-read-dwarf/test4.so.abi: Likewise.
* tests/data/test-read-dwarf/test6.so.abi: Likewise.
* tests/data/test-read-dwarf/test7.so.abi: Likewise.
* tests/data/test-read-dwarf/test8-qualified-this-pointer.so.abi: Likewise.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Likewise.
* tests/data/test-read-write/test10.xml: Likewise.
* tests/data/test-read-write/test14.xml: Likewise.
* tests/data/test-read-write/test15.xml: Likewise.
* tests/data/test-read-write/test17.xml: Likewise.
* tests/data/test-read-write/test18.xml: Likewise.
* tests/data/test-read-write/test19.xml: Likewise.
* tests/data/test-read-write/test2.xml: Likewise.
* tests/data/test-read-write/test20.xml: Likewise.
* tests/data/test-read-write/test21.xml: Likewise.
* tests/data/test-read-write/test22.xml: Likewise.
* tests/data/test-read-write/test23.xml: Likewise.
* tests/data/test-read-write/test24.xml: Likewise.
* tests/data/test-read-write/test25.xml: Likewise.
* tests/data/test-read-write/test26.xml: Likewise.
* tests/data/test-read-write/test27.xml: Likewise.
* tests/data/test-read-write/test28-without-std-fns-ref.xml: Likewise.
* tests/data/test-read-write/test28-without-std-vars-ref.xml: Likewise.
* tests/data/test-read-write/test3.xml: Likewise.
* tests/data/test-read-write/test6.xml: Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoabg-dwarf-reader: detect kernel modules without exports as such
Matthias Maennich [Wed, 24 Jul 2019 21:32:55 +0000 (22:32 +0100)]
abg-dwarf-reader: detect kernel modules without exports as such

Kernel modules without exported symbols (no use of EXPORT_SYMBOL*()),
will not have a __ksymtab_strings section. Libabigail will therefore
assume they are usual ELF binaries. That leads to wrong results as
now all ELF symbols are considered part of the ABI. That is obviously
wrong. Instead consider binaries having a .modinfo section to be kernel
binaries. We keep the __ksymtab_strings condition as vmlinux has no
.modinfo section but a __ksymtab_strings if symbols are exported.

One case is still open (and requires maybe some documentation): if a
kernel does not export symbols (no module support), none of the
conditions apply. But, who would be interested in the ABI of a kernel
that does not expose any?

* src/abg-dwarf-reader.cc(is_linux_kernel_binary): consider
  binaries only having a .modinfo section to be kernel binaries

Co-developed-by: Alessio Balsini <balsini@android.com>
Signed-off-by: Alessio Balsini <balsini@android.com>
Signed-off-by: Matthias Maennich <maennich@google.com>
5 years agoEnsure a consistent C++ standard use
Matthias Maennich [Mon, 22 Jul 2019 08:39:40 +0000 (09:39 +0100)]
Ensure a consistent C++ standard use

On older compilers (such as g++ 4.8), the default C++ standard is set to
gnu++98. When compiling libabigail with --enable-cxx11=yes, src/ and
tests/ where compiled with the correct flag, while tools/ was compiled
without specifying a standard. With a compiler falling back to gnu++98
that leads to unresolved references when linking the tools against the
libabigail library. Fix that by consistently using the std= flag across
the code base.

* configure.ac: add -std=c++11 flag to CXXFLAGS when compiling
for C++11
* src/Makefile.am: drop now obsolete setting of the -std flag
* tests/Makefile.am: likewise

Reported-by: Chun-Hung Wu <Chun-hung.Wu@mediatek.com>
Signed-off-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoBug 24787 - Filter out enum changes into compatible integer types
Dodji Seketeli [Fri, 19 Jul 2019 15:45:27 +0000 (17:45 +0200)]
Bug 24787 - Filter out enum changes into compatible integer types

Libabigail's filtering engine fails to recognize an enum changing into
a compatible integer (or vice versa) as a harmless change.

This patch fixes that.

* include/abg-comparison.h (peel_typedef_or_qualified_type_diff):
Declare new function.
(peel_pointer_or_qualified_type_diff): Rename
peel_pointer_or_qualified_type into this.
* include/abg-fwd.h (is_enum_type): Declare a new overload for
type_or_decl_base*.
* src/abg-comp-filter.cc (has_harmless_enum_to_int_change): Define
new static function.
* src/abg-comparison.cc (categorize_harmless_diff_node): Use the
new has_harmless_enum_to_int_change here.
(peel_pointer_or_qualified_type_diff): Renamed
peel_pointer_or_qualified_type into this.
(is_diff_of_basic_type): Adjust.
(peel_typedef_or_qualified_type_diff): Define new function.
* test-diff-filter/PR24787-lib{one, two}.so: New test input
binaries.
* test-diff-filter/PR24787-{one, two}.c: Source files of the test
input binaries above.
* test-diff-filter/PR24787-report-0.txt: Test output reference.
* tests/data/Makefile.am: Add the new testing material to source
distribution.
* tests/test-diff-filter.cc (in_out_specs): Add the new test to
the test harness.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoAdd timing to the verbose logs of abidw
Dodji Seketeli [Fri, 19 Jul 2019 08:14:40 +0000 (10:14 +0200)]
Add timing to the verbose logs of abidw

While doing my recent optimization work, it became useful to have an
idea of the time different parts of the processing pipeline are
taking.

This patch introduces an abigail::tools_utils::timer type that is easy
to use to time a given part of the code and emit the elapsed time to
an output stream.

This abigail::tools_utils::timer type is thus used to time various
parts of the processing pipeline involved in abidw.  Just using the
existing --verbose option now yields timing information.

* include/abg-tools-utils.h (class timer): Declare new type.
(operator<<(ostream&, const timer&)): Declare new streaming
operator for the new timer type.
* src/abg-tools-utils.cc (struct timer::priv): Define new type.
(timer::{timer, start, stop, value_in_seconds, value,
value_as_string, ~timer}): Define member functions.
(operator<<(ostream& o, const timer& t)): Define streaming
operator.
(build_corpus_group_from_kernel_dist_under): Add timing logs to
the linux kernel reading process.
* src/abg-dwarf-reader.cc
(read_context::canonicalize_types_scheduled): Add timing logs to
type canonicalization.
(read_debug_info_into_corpus): Add timing logs for the whole debug
info loading and internal representation building process.
* tools/abidw.cc (load_corpus_and_write_abixml): Add timing logs
for the binary loading and serizalization process.
(load_kernel_corpus_group_and_write_abixml): Add timing logs the
Linux Kernel binary loading and writing process.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years ago[ir] Fix indentation and add comments
Dodji Seketeli [Thu, 18 Jul 2019 08:23:41 +0000 (10:23 +0200)]
[ir] Fix indentation and add comments

GCC 9+ rightfully complains about some indentation issues in the
types_defined_same_linux_kernel_corpus_public function.

This patch fixes it and adds more comments.

* src/abg-ir.cc (types_defined_same_linux_kernel_corpus_public):
Fix indentation and add comments.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoImplement fast comparison of Linux Kernel types when applicable
Dodji Seketeli [Mon, 15 Jul 2019 08:41:16 +0000 (10:41 +0200)]
Implement fast comparison of Linux Kernel types when applicable

During type canonicalization there are observations that can speed-up
type comparison significantly without impacting correctness too much.

Typically, when two types are of the same name and kind, are found in
the same corpus and are defined in the same translation unit, they
ought to be the same type, even in C.  So there is no need in this
case to actually perform the structural comparison of the two types
which does have a quadratic performance at best.

Using this optimization made the loading of the
drivers/gpu/drm/i915/i915.ko module go from a quasi inifite time (many
hours on my system) to less than two minutes.  I am confining this
optimization to the Linux kernel case only for now, but I believe it
could benefit all C programs.  I am waiting for more testing before
applying it more broadly.

Also, while looking at this, I noticed that when loading several
corpora into a given corpus group (i.e, loading several linux kernel
binaries to represent a single conceptual kernel), we sometimes fail
to recognize that a type defined in a header file that is included in
several corpora is actually the same type, and should be re-used,
rather than being re-defined in each corpus.  This later adds stress
(time and space) on the system as we need to canonicalize and
de-duplicate these type later on.

This is because the "per-corpus" type maps that we use to lookup a
type by name and location when we see it (so that we know it's defined
in a different corpus of our current group) should really be
per-corpus-group type maps!  That is a type can be defined in the
corpus representing a .ko binary, and that type would be seen again in
another .ko binary later.  Until now, we were wrongly considering that
types were to be first defined in the corpus of the vmlinux binary,
and then could be re-used later.

I have thus fixed the code so that whenever we add a type to its
scope, the relevant per-corpus type maps are updated, as well as the
per-corpus-group ones, so that we can later lookup types in those
per-corpus-group type maps to know if a type is already defined in any
corpus of the group.

* include/abg-corpus.h (corpus::origin): Add a new
LINUX_KERNEL_BINARY_ORIGIN enumerator.
(corpus::{s,g}et_group): Declare new member
functions.
(class corpus): Make the corpus_group class friend of this one.
(corpus_group::get_main_corpus): Declare new member function.
* src/abg-corpus-priv.h (corpus::priv::group): Define new data
member.
(corpus::priv::priv): Initialize the new corpus::priv::group data
member.
* src/abg-corpus.cc (corpus::{g,s}et_group): Define new member
functions.
(corpus_group::get_main_corpus): Likewise.
(corpus_group::add_corpus): Use the new corpus::set_group() here
to to make the corpus be aware of the group it belongs to.
* src/abg-dwarf-reader.cc (read_debug_info_into_corpus): Set the
current corpus origin to the corpus::LINUX_KERNEL_BINARY_ORIGIN if
we are looking at a Linux Kernel binary.
(read_context::main_corpus_from_current_group): Use the
corpus_group::get_main_corpus method.
(should_reuse_type_from_corpus_group): Return the corpus group,
rather than the main corpus.
(read_debug_info_into_corpus): Add the current corpus to the
current corpus group before the debug info reading is done.  That
way, the corpus group will be accessible from the current corpus
during the construction of the internal representation.
(read_and_add_corpus_to_group_from_elf): Add the corpus to the
group only if it wasn't added to it before.
* include/abg-ir.h (operator{==,!=}): Declare new deep equality
and inequality operators for class_or_union_sptr and
union_decl_sptr.
* src/abg-ir.cc (types_defined_same_linux_kernel_corpus_public):
Define a new static function.
(type_base::get_canonical_type_for): Use the new
types_defined_same_linux_kernel_corpus_public here to speed up
type comparison.
(equals): In the overload of class_or_union, use the new
types_defined_same_linux_kernel_corpus_public as well, to speed up
type comparison.
(operator{==,!=}): Define new deep equality and inequality
operators for class_or_union_sptr and union_decl_sptr.
(maybe_update_types_lookup_map): In the overload function for
type_decl_sptr, class_decl_sptr, union_decl_sptr,
enum_type_decl_sptr, typedef_decl_sptr, qualified_type_def_sptr,
reference_type_def_sptr, array_type_def_sptr,
array_type_def::subrange_sptr, and function_type_sptr, update the
type lookup maps of the containing corpus group as well, not just
the ones of the current corpus.
* src/abg-reader.cc (build_enum_type_decl): Forgot to set the
"is-anonymous" flag.  Oops, fix this.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Adjust.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Adjust.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Adjust.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Adjust.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Adjust.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoabg-tools-utils: add missing header include guards
Matthias Maennich [Tue, 9 Jul 2019 08:22:22 +0000 (09:22 +0100)]
abg-tools-utils: add missing header include guards

* include/abg-tools-utils.h: add header include guards

Signed-off-by: Matthias Maennich <maennich@google.com>
5 years agoAdd compatibility layer for C++11 mode
Matthias Maennich [Fri, 5 Jul 2019 11:00:23 +0000 (12:00 +0100)]
Add compatibility layer for C++11 mode

Introduce a compatibility layer for C++11 code by adding
include/abg-cxx-compat.h. abg-cxx-compat defines a new namespace
abg_compat and defines
abg_compat::hash
abg_compat::shared_ptr
abg_compat::weak_ptr
abg_compat::dynamic_pointer_cast
abg_compat::static_pointer_cast
abg_compat::unordered_map
abg_compat::unordered_set
based on definitions from std::tr1 (std=gnu++98) or std:: (std=gnu++11).

I decided for introducing abg_compat:: rather than polluting abigail::
to allow an easier transition to C++11 at a later time and to not subtly
break existing code.

As the shared_ptr in C++11 defines shared_ptr::operator bool() explicit,
some locations where a shared_ptr is assigned to boolean, needed to be
adjusted to explicitly cast to bool.

* include/abg-cxx-compat.h: new file introducing the abg_compat
  namespace to provide C++11 functionality from either std::tr1
  or std::
* include/Makefile.am: Add the new abg-cxx-compat.h to source
  distribution.
* include/abg-comparison.h: replace std::tr1 usage by abg_compat
  and adjust includes accordingly: likewise
* include/abg-diff-utils.h: likewise
* include/abg-fwd.h: likewise
* include/abg-ini.h: likewise
* include/abg-interned-str.h: likewise
* include/abg-ir.h: likewise
* include/abg-libxml-utils.h: likewise
* include/abg-libzip-utils.h: likewise
* include/abg-reporter.h: likewise
* include/abg-sptr-utils.h: likewise
* include/abg-suppression.h: likewise
* include/abg-tools-utils.h: likewise
* include/abg-workers.h: likewise
* src/abg-comp-filter.cc: likewise
* src/abg-comparison-priv.h: likewise
* src/abg-corpus.cc: likewise
* src/abg-dwarf-reader.cc: likewise
* src/abg-hash.cc: likewise
* src/abg-ir.cc: likewise
* src/abg-reader.cc: likewise
* src/abg-suppression.cc: likewise
* src/abg-tools-utils.cc: likewise
* src/abg-writer.cc: likewise
* tests/test-diff-filter.cc: likewise
* tests/test-diff-pkg.cc: likewise
* tests/test-read-dwarf.cc: likewise
* tests/test-read-write.cc: likewise
* tests/test-types-stability.cc: likewise
* tests/test-write-read-archive.cc: likewise
* tools/abicompat.cc: likewise
* tools/abidiff.cc: likewise
* tools/abidw.cc: likewise
* tools/abilint.cc: likewise
* tools/abipkgdiff.cc: likewise

Signed-off-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoUpdate tests/.gitignore to ignore runtesttoolsutils
Matthias Maennich [Fri, 5 Jul 2019 10:59:18 +0000 (11:59 +0100)]
Update tests/.gitignore to ignore runtesttoolsutils

* tests/.gitignore: ignore runtesttoolsutils

Signed-off-by: Matthias Maennich <maennich@google.com>
5 years agoDrop requirement to compile with GNU extensions
Matthias Maennich [Fri, 5 Jul 2019 09:10:37 +0000 (10:10 +0100)]
Drop requirement to compile with GNU extensions

__gnu_cxx::stdio_filebuf is a GNU extension only available in certain
std libraries. It is not e.g. in libc++. In order to be able to compile
with using libc++, replace the usage of __gnu_cxx::stdio_filebuf with
standard C++ methods. In this case, reopen the temporary file with a
std::fstream and expose that stream rather than the previously exposed
std::iostream.

* include/abg-tools-utils.h (get_stream): Change return type to
  std::fstream
* src/abg-corpus.cc: remove unused #include of ext/stdio_filebuf.h
* src/abg-tools-utils (temp_file::priv): remove filebuf_ member,
  and replace iostream_ by fstream_ with changing the shared_ptr
  type accordingly
  (temp_file::priv::priv): initialize fstream_ based on
  temporary file name
  (temp_file::priv::~priv): adjust destruction accordingly
  (temp_file::is_good): test the fstream rather than the fd
  (temp_file::get_stream): adjust return type to std::fstream
  and adjust implementation based on the changes in temp_file::priv
* src/Makefile.am: remove gnu extension from c++ standard flag
* tests/Makefile.am: likewise

Signed-off-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoMisc indent cleanup
Dodji Seketeli [Tue, 9 Jul 2019 07:12:31 +0000 (09:12 +0200)]
Misc indent cleanup

* src/abg-dwarf-reader.cc (addr_elf_symbol_sptr_map_sptr): Fix a
typo in the comment of this typedef.
* src/abg-ir.cc (hash_type_or_decl): Fix typo in a comment.
* src/abg-writer.cc (write_translation_unit): Remove useless
vertical space.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years ago[xml-writer] Remove a useless kludge
Dodji Seketeli [Mon, 8 Jul 2019 15:13:00 +0000 (17:13 +0200)]
[xml-writer] Remove a useless kludge

* src/abg-writer.cc (write_context::type_is_emitted): Remove
useless kludge from here.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years ago[xml-writter] Speedup function_type::get_cached_name
Dodji Seketeli [Mon, 8 Jul 2019 15:00:17 +0000 (17:00 +0200)]
[xml-writter] Speedup function_type::get_cached_name

It looks like due to a typo, we are never caching the name of the
function_type, so we are computing it all the time, *OOOPS*.  So this
is having an impact when comparing instance of function_type during
de-duplication at abixml writting time.

Things are faster now, thanks to this patch.

* src/abg-ir.cc (function_type::get_cached_name): Really cache the
computed name of function_type instances.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years ago[xml-writter] Avoid using RTTI when dynamically hashing types
Dodji Seketeli [Mon, 8 Jul 2019 14:53:52 +0000 (16:53 +0200)]
[xml-writter] Avoid using RTTI when dynamically hashing types

When we dynamically hash types in the abixml writter, we use
hash_type_or_decl.  This function uses runtime type identification to
determine if the (type) artifact is a decl or a type, and based on
that, choose how to compute its hash value.  Profiling shows that
using the RTTI in hash_type_or_decl at this point is a hotspot.

Because we know that the type ABI is a *type*, we obviously can avoid
using RTTI there.

The patch thus implements a hash_type function, and uses that in the
xml writter.  Emitting the abixml output is faster with this patch.

* include/abg-fwd.h (hash_type): Declare new function.
* src/abg-ir.cc (hash_type): Define new function.
* src/abg-writer.cc (type_hasher::operator()): Use the new
  hash_type rather than the old hash_type_or_decl.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years agoImplement a poor-man's RTTI for performance
Dodji Seketeli [Mon, 8 Jul 2019 13:37:18 +0000 (15:37 +0200)]
Implement a poor-man's RTTI for performance

Profiling showed that a number of use of dynamic_cast are a speed
bottleneck.

This patch implements a poor-man's RTTI that allows us to implement a
form of dynamic_cast that is specific to the types of the internal
reprenstation that are in the namespace abigail::ir.  It speeds up
things greatly.

Basically, the base type of all ABI artifacts
(abigail::ir::type_or_decl_base) now contains three new data members.
The first one contains a bitmap that identifies the type of artifact.
The second one contains a pointer to the dynamic type sub-object of
the current instance of the artifact.  The last one contains either a
pointer to the type_base sub-object of the current instance of ABI
artifact if it's a type, or a pointer to the type_decl sub-object of
the current instance.

Together these three data members allow the patch to implement the
abigail::ir::{is_type(), is_decl(), is_<type_kind>_type} functions
that we need to make the code base noticeably faster when using abidw
on a big vmlinux binary.

* include/abg-fwd.h (is_type_decl): Replace the overloads
that takes a type_base* and/or a decl_base* by one that takes a
type_or_decl_base*.
* include/abg-ir.h (type_or_decl_base::type_or_decl_kind): Define
new enum.
(type_or_decl_base::{kind, runtime_type_instance,
type_or_decl_base_pointer}): Declare new accessors.
(operator{|,|=,&,&=): Declare new operators for the new
type_or_decl_base::type_or_decl_kind enum.
(global_scope::global_scope): Move the definition of this
constructor to ...
* src/abg-ir.cc (global_scope::global_scope): ... here.
(type_or_decl_base::priv::{kind_, rtti_, type_or_decl_ptr_}):
Add new data members.
(type_or_decl_base::priv::priv): Take a
type_or_decl_base::type_or_decl_kind enum.
(type_or_decl_base::priv::kind): Define new accessors.
(operator{|,|=,&,&=): Define new operators for the new
type_or_decl_base::type_or_decl_kind enum.
(type_or_decl_base::type_or_decl_base): Take a
type_or_decl_base::type_or_decl_kind enum.
(type_or_decl_base::{kind, runtime_type_instance,
type_or_decl_base_pointer}): Define new accessors.
(decl_base::decl_base, scope_decl::scope_decl)
(type_base::type_base, scope_type_decl::scope_type_decl)
(class_or_union::class_or_union) : Adjust to set the runtime type
identifier of the instances of these types.
(global_scope::global_scope, type_decl::type_decl)
(qualified_type_def::qualified_type_def)
(pointer_type_def::pointer_type_def)
(reference_type_def::reference_type_def
array_type_def::subrange_type::subrange_type)
(array_type_def::array_type_def, enum_type_decl::enum_type_decl)
(typedef_decl::typedef_decl, var_decl::var_decl)
(function_type::function_type, method_type::method_type)
(function_decl::function_decl)
(function_decl::parameter::parameter, method_decl::method_decl)
(class_decl::class_decl, class_decl::base_spec::base_spec)
(union_decl::union_decl, template_decl::template_decl)
(type_tparameter::type_tparameter)
(non_type_tparameter::non_type_tparameter)
(template_tparameter::template_tparameter)
(type_composition::type_composition)
(function_tdecl::function_tdecl, function_tdecl::function_tdecl)
(class_tdecl::class_tdecl):
Likewise and call runtime_type_instance() here to set the runtime
type instance pointers of the current instance.
(is_decl, is_type, is_class_type, is_pointer_type): Adjust to use
the new poor-man's rtti machinery.
(is_type_decl): Replace the overloads that takes a type_base*
and/or a decl_base* by one that takes a type_or_decl_base*.
(pointer_type_def::operator==, class_decl::operator==): Use the
poor-man's rtti machinery to replace dynamic_cast.
hash_type_or_decl: Replace dynamic_cast<const type_base> by
is_type() and dynamic_cast<const decl_base*> by is_decl().

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years ago[dwarf-reader] Make sure to canonicalize anonymous types
Dodji Seketeli [Fri, 5 Jul 2019 15:47:28 +0000 (17:47 +0200)]
[dwarf-reader] Make sure to canonicalize anonymous types

For a reason, anonymous types are not canonicalized.  I think this is
due to the fact that because they have no name,
read_context::lookup_type_from_die(die) used by maybe_canonicalize_type()
falls short in trying to canonicalize the *DIE*.

So later, at comparison time, things can be really slow because we
can't do canonical comparison; we ressort to structural comparison.

This patch ensures that even anonymous types are canonicalized.

* src/abg-dwarf-reader.cc (maybe_canonicalize_type): Add two new
overloads.  One that takes type_base_sptr, one that takes a
Dwarf_Die* and type_base_sptr.  These force canonicalization for
anonymous types.
(build_function_type): Schedule function types for
canonicalization.
(build_ir_node_from_die): For struct/classes and unions, use the
new overload of maybe_canonicalize_type to schedule
canonicalization.
* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Adjust.
* tests/data/test-read-dwarf/test10-pr18818-gcc.so.abi: Adjust.
* tests/data/test-read-dwarf/test12-pr18844.so.abi: Adjust.
* tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Adjust.
* tests/data/test-read-dwarf/test9-pr18818-clang.so.abi: Adjust.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
5 years ago[dwarf-reader] Constify the first parameter of maybe_canonicalize_type
Dodji Seketeli [Thu, 4 Jul 2019 16:44:18 +0000 (18:44 +0200)]
[dwarf-reader] Constify the first parameter of maybe_canonicalize_type

In preparation for some coming patches, I figured it'd be more type
safe to make the Dwarf_Die parameter of maybe_canonicalize_type be a
const pointer.  The patch subsequently adjusts code that needs adjusting.

* src/abg-dwarf-reader.cc (maybe_canonicalize_type): Make the
first parameter const.
(read_context::{get_canonical_die, lookup_artifact_from_die,
lookup_type_from_die, schedule_type_for_late_canonicalization}):
Adjust.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This page took 0.131873 seconds and 5 git commands to generate.