diff mbox series

Bug 26309 - Wrong leaf reporting of changes to typedef underlying type

Message ID 87y2lcr605.fsf@redhat.com
State New
Headers show
Series Bug 26309 - Wrong leaf reporting of changes to typedef underlying type | expand

Commit Message

Dodji Seketeli Sept. 14, 2020, 10:45 a.m. UTC
Hello,

In leaf mode, libabigail fails to report changes to the underlying
type of a typedef.

At its core, this is due to the fact that changes to the underlying
type of a typedef are not considered local.  As the leaf reporter only
reports local changes (as opposed to non-local changes which are
changes to sub-types) it doesn't detect those non-local typedef
changes.

To handle this, this patch makes changes to the underlying type of a
typedef be considered as local changes.  This is like what we already
do for pointer and qualified types.

Now that we have another set of changes to report in the leaf
reporter, we need to handle how to propagate the category and
redundancy status of those changes.  The patch does this too.

Also, just like we do pointer and qualified type changes, the patch
avoids marking the diff node carrying the typedef change as being a
leaf change.  That way, only existing leaf changes carrying that
typedef diff node will be reported.  For instance, a function whose
parameter has a typedef change will be reported because that change to
the function is considered a leaf change.  Otherwise, reporting the
typedef (or the pointer or qualified) type change on its own is not
useful unless it impacts those leaf changes that we deem useful.

The patch adds the example given in problem report to the testsuite.

 	* src/abg-ir.cc (equals): In the overload for typedef_decls,
	report changes to the underlying type as being local of kind
	LOCAL_TYPE_CHANGE_KIND.
	* src/abg-comparison.cc
	(leaf_diff_node_marker_visitor::visit_begin): Do not mark typedef
	diff node as leaf node.
	(suppression_categorization_visitor::visit_end): Propagate the
	'suppressed' category of the underlying type to the parent typedef
	unless the later has a local non-type change.
	(redundancy_marking_visitor::visit_end): Likewise for the
	'redundant' category.
	* include/abg-reporter.h (report_non_type_typedef_changes): Rename ...
	* src/abg-default-reporter.cc (report_non_type_typedef_changes):
	... report_local_typedef_changes into this.
	* src/abg-leaf-reporter.cc (leaf_reporter::report): Make the leaf
	reporter invoke the reporting method of the default reporter for
	typedefs as all typedef changes are now local.
	* tests/data/test-diff-filter/test-PR26309-report-0.txt: Add new
	test reference output.
	* tests/data/test-diff-filter/test-PR26309-v{0,1}.o: Add new test
	binary input.
	* tests/data/test-diff-filter/test-PR26309-v{0,1}.c: Add source
	code for new test binary input.
	* tests/data/Makefile.am: Add the new text material above to
	source distribution.
	* tests/test-diff-filter.cc (in_out_specs): Add the new test input
	above to this test harness.
	* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt: Adjust.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
---
 include/abg-reporter.h                             |   6 +-
 src/abg-comparison.cc                              |  67 ++++++++++------
 src/abg-default-reporter.cc                        |  14 ++--
 src/abg-ir.cc                                      |   4 +-
 src/abg-leaf-reporter.cc                           |   4 +-
 tests/data/Makefile.am                             |   5 ++
 .../test-diff-filter/test-PR26309-report-0.txt     |  13 ++++
 tests/data/test-diff-filter/test-PR26309-v0.c      |   5 ++
 tests/data/test-diff-filter/test-PR26309-v0.o      | Bin 0 -> 2720 bytes
 tests/data/test-diff-filter/test-PR26309-v1.c      |   5 ++
 tests/data/test-diff-filter/test-PR26309-v1.o      | Bin 0 -> 2752 bytes
 ...-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt |  86 +--------------------
 tests/test-diff-filter.cc                          |   7 ++
 13 files changed, 100 insertions(+), 116 deletions(-)
 create mode 100644 tests/data/test-diff-filter/test-PR26309-report-0.txt
 create mode 100644 tests/data/test-diff-filter/test-PR26309-v0.c
 create mode 100644 tests/data/test-diff-filter/test-PR26309-v0.o
 create mode 100644 tests/data/test-diff-filter/test-PR26309-v1.c
 create mode 100644 tests/data/test-diff-filter/test-PR26309-v1.o

Comments

Giuliano Procida Sept. 14, 2020, 5:43 p.m. UTC | #1
Hi there.

On Mon, 14 Sep 2020 at 11:45, Dodji Seketeli <dodji@redhat.com> wrote:

> Hello,
>
> In leaf mode, libabigail fails to report changes to the underlying
> type of a typedef.
>
> At its core, this is due to the fact that changes to the underlying
> type of a typedef are not considered local.  As the leaf reporter only
> reports local changes (as opposed to non-local changes which are
> changes to sub-types) it doesn't detect those non-local typedef
> changes.
>
> To handle this, this patch makes changes to the underlying type of a
> typedef be considered as local changes.  This is like what we already
> do for pointer and qualified types.
>
> Now that we have another set of changes to report in the leaf
> reporter, we need to handle how to propagate the category and
> redundancy status of those changes.  The patch does this too.
>
> Also, just like we do pointer and qualified type changes, the patch
> avoids marking the diff node carrying the typedef change as being a
> leaf change.  That way, only existing leaf changes carrying that
> typedef diff node will be reported.  For instance, a function whose
> parameter has a typedef change will be reported because that change to
> the function is considered a leaf change.  Otherwise, reporting the
> typedef (or the pointer or qualified) type change on its own is not
> useful unless it impacts those leaf changes that we deem useful.
>
> The patch adds the example given in problem report to the testsuite.
>
>         * src/abg-ir.cc (equals): In the overload for typedef_decls,
>         report changes to the underlying type as being local of kind
>         LOCAL_TYPE_CHANGE_KIND.
>         * src/abg-comparison.cc
>         (leaf_diff_node_marker_visitor::visit_begin): Do not mark typedef
>         diff node as leaf node.
>         (suppression_categorization_visitor::visit_end): Propagate the
>         'suppressed' category of the underlying type to the parent typedef
>         unless the later has a local non-type change.
>         (redundancy_marking_visitor::visit_end): Likewise for the
>         'redundant' category.
>         * include/abg-reporter.h (report_non_type_typedef_changes): Rename
> ...
>         * src/abg-default-reporter.cc (report_non_type_typedef_changes):
>         ... report_local_typedef_changes into this.
>         * src/abg-leaf-reporter.cc (leaf_reporter::report): Make the leaf
>         reporter invoke the reporting method of the default reporter for
>         typedefs as all typedef changes are now local.
>         * tests/data/test-diff-filter/test-PR26309-report-0.txt: Add new
>         test reference output.
>         * tests/data/test-diff-filter/test-PR26309-v{0,1}.o: Add new test
>         binary input.
>         * tests/data/test-diff-filter/test-PR26309-v{0,1}.c: Add source
>         code for new test binary input.
>         * tests/data/Makefile.am: Add the new text material above to
>         source distribution.
>         * tests/test-diff-filter.cc (in_out_specs): Add the new test input
>         above to this test harness.
>         *
> tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt:
> Adjust.
>
> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
> ---
>  include/abg-reporter.h                             |   6 +-
>  src/abg-comparison.cc                              |  67 ++++++++++------
>  src/abg-default-reporter.cc                        |  14 ++--
>  src/abg-ir.cc                                      |   4 +-
>  src/abg-leaf-reporter.cc                           |   4 +-
>  tests/data/Makefile.am                             |   5 ++
>  .../test-diff-filter/test-PR26309-report-0.txt     |  13 ++++
>  tests/data/test-diff-filter/test-PR26309-v0.c      |   5 ++
>  tests/data/test-diff-filter/test-PR26309-v0.o      | Bin 0 -> 2720 bytes
>  tests/data/test-diff-filter/test-PR26309-v1.c      |   5 ++
>  tests/data/test-diff-filter/test-PR26309-v1.o      | Bin 0 -> 2752 bytes
>  ...-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt |  86
> +--------------------
>  tests/test-diff-filter.cc                          |   7 ++
>  13 files changed, 100 insertions(+), 116 deletions(-)
>  create mode 100644 tests/data/test-diff-filter/test-PR26309-report-0.txt
>  create mode 100644 tests/data/test-diff-filter/test-PR26309-v0.c
>  create mode 100644 tests/data/test-diff-filter/test-PR26309-v0.o
>  create mode 100644 tests/data/test-diff-filter/test-PR26309-v1.c
>  create mode 100644 tests/data/test-diff-filter/test-PR26309-v1.o
>
> diff --git a/include/abg-reporter.h b/include/abg-reporter.h
> index bf113f0..6aec6a6 100644
> --- a/include/abg-reporter.h
> +++ b/include/abg-reporter.h
> @@ -175,9 +175,9 @@ public:
>          const std::string& indent = "") const;
>
>    void
> -  report_local_typedef_changes(const typedef_diff &d,
> -                              std::ostream& out,
> -                              const std::string& indent) const;
> +  report_non_type_typedef_changes(const typedef_diff &d,
> +                                 std::ostream& out,
> +                                 const std::string& indent) const;
>
>    virtual void
>    report(const typedef_diff& d, std::ostream& out,
> diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
> index 7f91753..11f506f 100644
> --- a/src/abg-comparison.cc
> +++ b/src/abg-comparison.cc
> @@ -10901,14 +10901,15 @@ struct leaf_diff_node_marker_visitor : public
> diff_node_visitor
>         // Similarly, a *local* change describing a type that changed
>         // its nature doesn't make sense.
>         && !is_distinct_diff(d)
> -       // Similarly, a pointer (or reference or array) or qualified
> -       // type change in itself doesn't make sense.  It's would
> -       // rather make sense to show that pointer change as part of
> -       // the variable change whose pointer type changed, for
> +       // Similarly, a pointer (or reference or array), a typedef or
> +       // qualified type change in itself doesn't make sense.  It
> +       // would rather make sense to show that pointer change as part
> +       // of the variable change whose pointer type changed, for
>         // instance.
>         && !is_pointer_diff(d)
>         && !is_reference_diff(d)
>         && !is_qualified_type_diff(d)
> +       && !is_typedef_diff(d)
>         && !is_array_diff(d)
>         // Similarly a parameter change in itself doesn't make sense.
>         // It should have already been reported as part of the change
> @@ -11670,16 +11671,16 @@ struct suppression_categorization_visitor :
> public diff_node_visitor
>         //  2/ and has no local change (unless it's a pointer,
>         //  reference or qualified diff node).
>         //
> -       //  Note that qualified type diff nodes are a bit special.
> -       //  The local changes of the underlying type are considered
> -       //  local for the qualified type, just like for
> -       //  pointer/reference types.  But then the qualified type
> -       //  itself can have local changes of its own, and those
> -       //  changes are of the kind LOCAL_NON_TYPE_CHANGE_KIND.  So a
> -       //  qualified type which have local changes that are *NOT* of
> -       //  LOCAL_NON_TYPE_CHANGE_KIND (or that has no local changes
> -       //  at all) and which is in the PRIVATE_TYPE_CATEGORY or
> -       //  SUPPRESSED_CATEGORY can see these categories be
> +       //  Note that qualified type and typedef diff nodes are a bit
> +       //  special.  The local changes of the underlying type are
> +       //  considered local for the qualified/typedef type, just like
> +       //  for pointer/reference types.  But then the qualified or
> +       //  typedef type itself can have local changes of its own, and
> +       //  those changes are of the kind LOCAL_NON_TYPE_CHANGE_KIND.
> +       //  So a qualified type which have local changes that are
> +       //  *NOT* of LOCAL_NON_TYPE_CHANGE_KIND (or that has no local
> +       //  changes at all) and which is in the PRIVATE_TYPE_CATEGORY
> +       //  or SUPPRESSED_CATEGORY can see these categories be
>         //  propagated.
>         //
>         // Note that all pointer/reference diff node changes are
> @@ -11687,7 +11688,7 @@ struct suppression_categorization_visitor : public
> diff_node_visitor
>         // pointed-to-type are considered local to the pointer itself.
>         //
>         // Similarly, changes local to the type of function parameters,
> -       // variables (and data members) and classe (that are not of
> +       // variables (and data members) and classes (that are not of
>         // LOCAL_NON_TYPE_CHANGE_KIND kind) and that have been
>         // suppressed can propagate their SUPPRESSED_CATEGORY-ness to
>         // those kinds of diff node.
> @@ -11697,6 +11698,8 @@ struct suppression_categorization_visitor : public
> diff_node_visitor
>             || is_reference_diff(d)
>             || (is_qualified_type_diff(d)
>                 && (!(d->has_local_changes() &
> LOCAL_NON_TYPE_CHANGE_KIND)))
> +           || (is_typedef_diff(d)
> +               && (!(d->has_local_changes() &
> LOCAL_NON_TYPE_CHANGE_KIND)))
>             || (is_function_decl_diff(d)
>                 && (!(d->has_local_changes() &
> LOCAL_NON_TYPE_CHANGE_KIND)))
>             || (is_fn_parm_diff(d)
> @@ -11758,12 +11761,19 @@ struct suppression_categorization_visitor :
> public diff_node_visitor
>               canonical_diff->add_to_category(SUPPRESSED_CATEGORY);
>           }
>
> -       if (// We don't propagate "private type"-ness to typedefs
> -           // because defining "public" typedefs of private (opaque)
> -           // types is a common idiom.  So the typedef must stay
> -           // public.
> -           !is_typedef_diff(d)
> -           && has_non_empty_child
> +       // Note that the private-ness of a an underlying type won't be
> +       // propagated to its parent typedef, by virtue of the big "if"
> +       // clause at the beginning of this function.  So we don't have
> +       // to handle that case here.  So the idiom of defining
> +       // typedefs of private (opaque) types will be respected;
> +       // meaning that changes to opaque underlying type will be
> +       // flagged as private and the typedef will be flagged private
> +       // as well, unless the typedef itself has local non-type
> +       // changes.  In the later case, changes to the typedef will be
> +       // emitted because the typedef won't inherit the privateness
> +       // of its underlying type.  So in practise, the typedef
> +       // remains public for the purpose of change reporting.
> +       if (has_non_empty_child
>             && has_private_child
>             && !has_non_private_child)
>           {
> @@ -12223,8 +12233,21 @@ struct redundancy_marking_visitor : public
> diff_node_visitor
>                 // in the same sense as other types.  So we always
>                 // propagate redundancy to them, regardless of if they
>                 // have local changes or not.
> +               //
> +               // We also propagate redundancy to typedef types if
> +               // these /only/ carry changes to their underlying
> +               // type.
> +               //
> +               // Note that changes to the underlying type of a
> +               // typedef is considered local of
> +               // LOCAL_TYPE_CHANGE_KIND kind.  The other changes to the
> +               // typedef itself are considered local of
> +               // LOCAL_NON_TYPE_CHANGE_KIND kind.
>                 || is_pointer_diff(d)
> -               || is_qualified_type_diff(d)))
> +               || is_qualified_type_diff(d)
> +               || (is_typedef_diff(d)
> +                   && (!(d->has_local_changes()
> +                         & LOCAL_NON_TYPE_CHANGE_KIND)))))
>           {
>             bool has_non_redundant_child = false;
>             bool has_non_empty_child = false;
> diff --git a/src/abg-default-reporter.cc b/src/abg-default-reporter.cc
> index f8572f2..0b572db 100644
> --- a/src/abg-default-reporter.cc
> +++ b/src/abg-default-reporter.cc
> @@ -231,7 +231,11 @@ default_reporter::report(const enum_diff& d, ostream&
> out,
>    d.reported_once(true);
>  }
>
> -/// For a @ref typedef_diff node, report the changes that are local.
> +/// For a @ref typedef_diff node, report the local changes to the
> +/// typedef rather the changes to its underlying type.
> +///
> +/// Note that changes to the underlying type are also considered
> +/// local.
>  ///
>  /// @param d the @ref typedef_diff node to consider.
>  ///
> @@ -239,9 +243,9 @@ default_reporter::report(const enum_diff& d, ostream&
> out,
>  ///
>  /// @param indent the white space string to use for indentation.
>  void
> -default_reporter::report_local_typedef_changes(const typedef_diff &d,
> -                                              ostream& out,
> -                                              const string& indent) const
> +default_reporter::report_non_type_typedef_changes(const typedef_diff &d,
> +                                                 ostream& out,
> +                                                 const string& indent)
> const
>  {
>    if (!d.to_be_reported())
>      return;
> @@ -283,7 +287,7 @@ default_reporter::report(const typedef_diff& d,
>
>    typedef_decl_sptr f = d.first_typedef_decl(), s =
> d.second_typedef_decl();
>
> -  report_local_typedef_changes(d, out, indent);
> +  report_non_type_typedef_changes(d, out, indent);
>
>    diff_sptr dif = d.underlying_type_diff();
>    if (dif && dif->has_changes())
> diff --git a/src/abg-ir.cc b/src/abg-ir.cc
> index 088eebb..dbb4364 100644
> --- a/src/abg-ir.cc
> +++ b/src/abg-ir.cc
> @@ -15778,9 +15778,11 @@ equals(const typedef_decl& l, const typedef_decl&
> r, change_kind* k)
>
>    if (*l.get_underlying_type() != *r.get_underlying_type())
>      {
> +      // Changes to the underlying type of a typedef are considered
> +      // local, a bit like for pointers.
>        result = false;
>        if (k)
> -       *k |= SUBTYPE_CHANGE_KIND;
> +       *k |= LOCAL_TYPE_CHANGE_KIND;
>        else
>         return false;
>      }
> diff --git a/src/abg-leaf-reporter.cc b/src/abg-leaf-reporter.cc
> index 0875c96..c2a766e 100644
> --- a/src/abg-leaf-reporter.cc
> +++ b/src/abg-leaf-reporter.cc
> @@ -202,7 +202,9 @@ leaf_reporter::report(const typedef_diff& d,
>    if (!diff_to_be_reported(&d))
>      return;
>
> -  report_local_typedef_changes(d, out, indent);
> +  // all changes carried by a typedef_diff are considered local, so
> +  // let's just call the default reporter here.
> +  default_reporter::report(d, out, indent);
>
>    maybe_report_interfaces_impacted_by_diff(&d, out, indent);
>  }
> diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
> index 5e2e8b1..7334db2 100644
> --- a/tests/data/Makefile.am
> +++ b/tests/data/Makefile.am
> @@ -867,6 +867,11 @@ test-diff-filter/test-PR25661-7-report-1.txt \
>  test-diff-filter/test-PR25661-7-report-2.txt \
>  test-diff-filter/test-PR25661-7-report-3.txt \
>  test-diff-filter/test-PR25661-7-report-4.txt \
> +test-diff-filter/test-PR26309-v0.c           \
> +test-diff-filter/test-PR26309-v1.c           \
> +test-diff-filter/test-PR26309-v0.o           \
> +test-diff-filter/test-PR26309-v1.o           \
> +test-diff-filter/test-PR26309-report-0.txt   \
>  \
>  test-diff-suppr/test0-type-suppr-v0.cc \
>  test-diff-suppr/test0-type-suppr-v1.cc \
> diff --git a/tests/data/test-diff-filter/test-PR26309-report-0.txt
> b/tests/data/test-diff-filter/test-PR26309-report-0.txt
> new file mode 100644
> index 0000000..dec3758
> --- /dev/null
> +++ b/tests/data/test-diff-filter/test-PR26309-report-0.txt
> @@ -0,0 +1,13 @@
> +Leaf changes summary: 1 artifact changed
> +Changed leaf types summary: 0 leaf type changed
> +Removed/Changed/Added functions summary: 0 Removed, 1 Changed, 0 Added
> function
> +Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added
> variable
> +
> +1 function with some sub-type change:
> +
> +  [C] 'function void changed_fun(changed)' at test-PR26309-v1.c:3:1 has
> some sub-type changes:
> +    parameter 1 of type 'typedef changed' changed:
> +      underlying type 'int' changed:
> +        type name changed from 'int' to 'long int'
> +        type size changed from 32 to 64 (in bits)
> +
> diff --git a/tests/data/test-diff-filter/test-PR26309-v0.c
> b/tests/data/test-diff-filter/test-PR26309-v0.c
> new file mode 100644
> index 0000000..853084b
> --- /dev/null
> +++ b/tests/data/test-diff-filter/test-PR26309-v0.c
> @@ -0,0 +1,5 @@
> +typedef int changed;
> +
> +void changed_fun(changed y) {
> +  (void) y;
> +}
> diff --git a/tests/data/test-diff-filter/test-PR26309-v0.o
> b/tests/data/test-diff-filter/test-PR26309-v0.o
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..0e6fd52dcd00da7ba74fa6c2e88907223f9a6f3c
> GIT binary patch
> literal 2720
> zcmbVN-D?v;5TCtkY&16X!>>}&6BS$VF0r*GD%EKHs3=k^f-j|9l1*}&4@s`2D&mU>
> zz9|SI_~x_tNBB?p=!<`V&k8!Tx077A7X%k>XZE-Ao0;94-F<QG_H|1L;3nWA^hb;W
> zd{6cDqEd@60poCg`Sp+2&wsCdEg?9hCen((!^v(n8Fdy3Aqv>rL#dz@MpR)((*SHN
> zKBk_4uwBHhw+-ZsiIdrhjGdjzj*0Q%(SwK|06Tj=b0Kqc0==DpOy*+7PGd5jxHK8f
> zoSG!#;-U9fCIM0@u+x#fbqM{QCf_P9l46Zhh(Ss%VsF7T_Coy1j$p6057#Rol-<x>
> zS)E;&%b#;M^IlmoV0Yvn2m3ZH3mN6Q->S*VdUd0fz{%CyO_{5-D-VNQEeLaspyZc=
> znjbWB+INlub#oOz^p)yXf@;;R28~d5Ks$#UckVk&&RMVE%{#OCGxPa{**Rx=RaTs3
> zKa6m9?%a&yHp7jUT&&5K>;z>T`kiupajUT4E}V7T8o-UErSr}-x|{j8w`jq*P_2lk
> zt01;U#nj&6^c&ox!-&&IP3`dI?C2|NX=K+`G>G8Dp2}$me*Q_Cv=b{9i$VWalVjsd
> zQnPgdn{zhoG9fj#VR!(ZTst;o2H1}bz_SDJy*qHMeLRi*L)Xh-XGzA3ueY*2$d9j*
> zBM?r$B!1M!A_0uE-F^c<qQ`jPRHMv#rvUYt2v&aL|J{q>6n9OJiw3TTa9t~=e_jja
> z8N0e*bb$T*0Nl|y<sQ!;yzY}`=$BB2ok-SM3*}Y_9vx2bO5HAa9og``iY#r^)`M2H
> zjdM!(OQnw7+&1lZ=wjGTX#_3F6bu1gx!r6^yfeL4JCq)tOSc>P<wx3)tglx)ev^Ng
> z&i{YK6cTA~c|X}tQ7gSV)8l>xM!+!?gqbJqrnWbQ$v=e#R3HD#KZ9J;C&lFWRT{*a
> zX-lsIJ!{37jZ&kx4!&ue>WUz)WqMhtE#hbTB>W3?*zQH5*PFI0IAdaDY-upxV@8T;
> zdkf*D{J-h^nNRuCZD!_QLCo}t5X@@uDh*=Iv|Y!!r1)ulD0!bKKGl*L|1o0Zlk-GX
> zwM`x9GXq+3e3s7;PvRffhnepicEdcDFEyX%S>D}t88EtYuRdtLY5qV1g+5hjyncq?
> zp^~(JPQw1tYr%ZpS6YV|n|!~~;EG---RtxUnSA;u=Gj9CzH9I(4PuS=jUn=z#4qUl
> KKe3^fCjT$}O}BFZ
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/data/test-diff-filter/test-PR26309-v1.c
> b/tests/data/test-diff-filter/test-PR26309-v1.c
> new file mode 100644
> index 0000000..5f453a3
> --- /dev/null
> +++ b/tests/data/test-diff-filter/test-PR26309-v1.c
> @@ -0,0 +1,5 @@
> +typedef long changed;
> +
> +void changed_fun(changed y) {
> +  (void) y;
> +}
> diff --git a/tests/data/test-diff-filter/test-PR26309-v1.o
> b/tests/data/test-diff-filter/test-PR26309-v1.o
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..4b9339a5972c97a595ff6555ddaf82efea0f975e
> GIT binary patch
> literal 2752
> zcmbtW-D?v;5TDJqHX0jJ>sP7Z35qRvmwr=~)@W_5A4sVRf=In2o8&aPNOIMxh%ZI(
> zO+iq>H~#^j{0ID71RwQH@Ihzpc9QG%M9_iTnfcAv?q+s3&layNrGx+`0rSw+7)8FL
> zD;Gks0E4gxZY;m}vi#!dkK3P%i1vh*beLdUmR+GS8dAiBn8i`1dr1jHg(Q)PoyGg!
> zM<Aq)Q0i3;V+O>r>|jP_hqD7>Phai;;`>2nPi0PLE)HV0qmaqWqa8Ck;>=KFIX*<r
> zg@bR-4FUA_V$H}ubrAEOpxpQYq{ER1o&xL43n-*u1V<r$X8Umr_Yc=AAW-eVURj-(
> znH)Q5KNxpPQP9pPKPr1SQz?un)m*=-%4?N%AL>oNYI%MD`C7A~^5tgvu9vTRLB8%4
> z-J(}@y?S1!%v1h$zU&5WDB5MOQn4#uJy5NL+PVr{xO&4{u%?_@XWE(=8=oGVnV7Uj
> zR#n+rc7q7FCr^%Ab|YB#)m&Bis^yhXbz7y{+{WyTJu_w5Re%c%3#Y6ROgH-XY|+a1
> zMhwK`RS+9FF}%Ak{Svq85aRR^g&Tb~n|q#G=-;u34iTI<QdjlB_g_gyncTS`_0Ma@
> zNVFu8)O~E)Wc(1sj1u}{T33{?v#Z1;37MGI6(#g{m6#*}yE%a*?B0gr5x`T~J$s!d
> z-rkw<9_*}){9{MGQ-N?=jgKGEK*Yk|j_~VR-)G>g<GBloX4W|jD33;Pm?!?bJ29N{
> zSG0cGz<CD>2Ch|D(t=r+*GTIM?;q`#`-A`VA8<?Kw5IsFz-d2f1a1*|(297Cr9f>2
> z;Lsrkr`T?T(^7TUDXZdob<OiDO|*rwTP(KJgDqyaMfb+mkh<q94#5)Ol$wo(!u!PW
> zn}KrhwA$^!E#1?JRBf%&avMrd{6E<+#?#*Nesa2k7CQZ>^=@ZIz!7AGnJ4j*_BWXs
> ze;ggCZ~irY9AnL#RFms--w{S+`qJY;PgeoIIHjYf5WZ=U`ida#ae8=&=I}Ff68@0}
> zq-zoDjHd4jT1<?br3S}3k&$5fUPd@+{y+5mIX;Jebjz9ZFC%8=L<lA|c%GCPnZ9di
> zOR7Jj4<+vt)u(&FtbZRd%E|LYF{FpO&?g2o=laZ_Af6O&>%+|V5U1fd=9@a6?aXg&
> zxeOTHzE|&byy<>N1N09mPP~4`UZaq-e}#noqgRCEd0%NAW^Ky-R)Z^gp>*HV>tx2$
> gZ(^Q3gy5?N50erj-ZzFQZ&LhCJ^v5vsJR*c8;XItO#lD@
>
> literal 0
> HcmV?d00001
>
> diff --git
> a/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt
> b/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt
> index aaf28bf..3e50dc7 100644
> ---
> a/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt
> +++
> b/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt
> @@ -1,6 +1,6 @@
>  ================ changes of 'libspice-server.so.1.8.0'===============
> -Leaf changes summary: 11 artifacts changed (11 filtered out)
> -  Changed leaf types summary: 2 (11 filtered out) leaf types changed
> +Leaf changes summary: 10 artifacts changed (10 filtered out)
> +  Changed leaf types summary: 1 (10 filtered out) leaf types changed
>    Removed/Changed/Added functions summary: 1 Removed, 0 Changed, 8 Added
> functions
>    Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added
> variable
>
> @@ -120,87 +120,5 @@ Leaf changes summary: 11 artifacts changed (11
> filtered out)
>        function int spice_server_set_zlib_glz_compression(SpiceServer*,
> spice_wan_compression_t)
>        function void spice_server_vm_start(SpiceServer*)
>        function void spice_server_vm_stop(SpiceServer*)
> -  'typedef spice_image_compression_t at spice.h:479:1' changed:
> -    typedef name changed from spice_image_compression_t to
> SpiceImageCompression at enums.h:197:1
> -    79 impacted interfaces:
> -      function void spice_qxl_add_memslot(QXLInstance*, QXLDevMemSlot*)
> -      function void spice_qxl_add_memslot_async(QXLInstance*,
> QXLDevMemSlot*, uint64_t)
> -      function void spice_qxl_create_primary_surface(QXLInstance*,
> uint32_t, QXLDevSurfaceCreate*)
> -      function void spice_qxl_create_primary_surface_async(QXLInstance*,
> uint32_t, QXLDevSurfaceCreate*, uint64_t)
> -      function void spice_qxl_del_memslot(QXLInstance*, uint32_t,
> uint32_t)
> -      function void spice_qxl_destroy_primary_surface(QXLInstance*,
> uint32_t)
> -      function void spice_qxl_destroy_primary_surface_async(QXLInstance*,
> uint32_t, uint64_t)
> -      function void spice_qxl_destroy_surface_async(QXLInstance*,
> uint32_t, uint64_t)
> -      function void spice_qxl_destroy_surface_wait(QXLInstance*, uint32_t)
> -      function void spice_qxl_destroy_surfaces(QXLInstance*)
> -      function void spice_qxl_destroy_surfaces_async(QXLInstance*,
> uint64_t)
> -      function void spice_qxl_driver_unload(QXLInstance*)
> -      function void spice_qxl_flush_surfaces_async(QXLInstance*, uint64_t)
> -      function void spice_qxl_loadvm_commands(QXLInstance*,
> QXLCommandExt*, uint32_t)
> -      function void spice_qxl_monitors_config_async(QXLInstance*,
> QXLPHYSICAL, int, uint64_t)
> -      function void spice_qxl_oom(QXLInstance*)
> -      function void spice_qxl_reset_cursor(QXLInstance*)
> -      function void spice_qxl_reset_image_cache(QXLInstance*)
> -      function void spice_qxl_reset_memslots(QXLInstance*)
> -      function void spice_qxl_set_max_monitors(QXLInstance*, unsigned int)
> -      function void spice_qxl_start(QXLInstance*)
> -      function void spice_qxl_stop(QXLInstance*)
> -      function void spice_qxl_update_area(QXLInstance*, uint32_t,
> QXLRect*, QXLRect*, uint32_t, uint32_t)
> -      function void spice_qxl_update_area_async(QXLInstance*, uint32_t,
> QXLRect*, uint32_t, uint64_t)
> -      function void spice_qxl_wakeup(QXLInstance*)
> -      function int spice_server_add_client(SpiceServer*, int, int)
> -      function int spice_server_add_interface(SpiceServer*,
> SpiceBaseInstance*)
> -      function int spice_server_add_renderer(SpiceServer*, const char*)
> -      function int spice_server_add_ssl_client(SpiceServer*, int, int)
> -      function void
> spice_server_char_device_wakeup(SpiceCharDeviceInstance*)
> -      function void spice_server_destroy(SpiceServer*)
> -      function spice_image_compression_t
> spice_server_get_image_compression(SpiceServer*)
> -      function int spice_server_get_num_clients(SpiceServer*)
> -      function int spice_server_get_peer_info(SpiceServer*, sockaddr*,
> socklen_t*)
> -      function int spice_server_get_sock_info(SpiceServer*, sockaddr*,
> socklen_t*)
> -      function int spice_server_init(SpiceServer*, SpiceCoreInterface*)
> -      function int spice_server_is_server_mouse(SpiceServer*)
> -      function int spice_server_migrate_connect(SpiceServer*, const
> char*, int, int, const char*)
> -      function int spice_server_migrate_end(SpiceServer*, int)
> -      function int spice_server_migrate_info(SpiceServer*, const char*,
> int, int, const char*)
> -      function int spice_server_migrate_start(SpiceServer*)
> -      function int spice_server_migrate_switch(SpiceServer*)
> -      function SpiceServer* spice_server_new()
> -      function void
> spice_server_playback_get_buffer(SpicePlaybackInstance*, uint32_t**,
> uint32_t*)
> -      function void
> spice_server_playback_put_samples(SpicePlaybackInstance*, uint32_t*)
> -      function void
> spice_server_playback_set_mute(SpicePlaybackInstance*, uint8_t)
> -      function void
> spice_server_playback_set_volume(SpicePlaybackInstance*, uint8_t, uint16_t*)
> -      function void spice_server_playback_start(SpicePlaybackInstance*)
> -      function void spice_server_playback_stop(SpicePlaybackInstance*)
> -      function void spice_server_port_event(SpiceCharDeviceInstance*,
> uint8_t)
> -      function uint32_t
> spice_server_record_get_samples(SpiceRecordInstance*, uint32_t*, uint32_t)
> -      function void spice_server_record_set_mute(SpiceRecordInstance*,
> uint8_t)
> -      function void spice_server_record_set_volume(SpiceRecordInstance*,
> uint8_t, uint16_t*)
> -      function void spice_server_record_start(SpiceRecordInstance*)
> -      function void spice_server_record_stop(SpiceRecordInstance*)
> -      function void spice_server_set_addr(SpiceServer*, const char*, int)
> -      function int spice_server_set_agent_copypaste(SpiceServer*, int)
> -      function int spice_server_set_agent_file_xfer(SpiceServer*, int)
> -      function int spice_server_set_agent_mouse(SpiceServer*, int)
> -      function int spice_server_set_channel_security(SpiceServer*, const
> char*, int)
> -      function int spice_server_set_compat_version(SpiceServer*,
> spice_compat_version_t)
> -      function int spice_server_set_exit_on_disconnect(SpiceServer*, int)
> -      function int spice_server_set_image_compression(SpiceServer*,
> spice_image_compression_t)
> -      function int spice_server_set_jpeg_compression(SpiceServer*,
> spice_wan_compression_t)
> -      function int spice_server_set_listen_socket_fd(SpiceServer*, int)
> -      function void spice_server_set_name(SpiceServer*, const char*)
> -      function int spice_server_set_noauth(SpiceServer*)
> -      function int spice_server_set_playback_compression(SpiceServer*,
> int)
> -      function int spice_server_set_port(SpiceServer*, int)
> -      function int spice_server_set_sasl(SpiceServer*, int)
> -      function int spice_server_set_sasl_appname(SpiceServer*, const
> char*)
> -      function void spice_server_set_seamless_migration(SpiceServer*, int)
> -      function int spice_server_set_streaming_video(SpiceServer*, int)
> -      function int spice_server_set_ticket(SpiceServer*, const char*,
> int, int, int)
> -      function int spice_server_set_tls(SpiceServer*, int, const char*,
> const char*, const char*, const char*, const char*, const char*)
> -      function void spice_server_set_uuid(SpiceServer*, const uint8_t*)
> -      function int spice_server_set_zlib_glz_compression(SpiceServer*,
> spice_wan_compression_t)
> -      function void spice_server_vm_start(SpiceServer*)
> -      function void spice_server_vm_stop(SpiceServer*)
>  ================ end of changes of
> 'libspice-server.so.1.8.0'===============
>
> diff --git a/tests/test-diff-filter.cc b/tests/test-diff-filter.cc
> index bdf7d41..b3b1e91 100644
> --- a/tests/test-diff-filter.cc
> +++ b/tests/test-diff-filter.cc
> @@ -766,6 +766,13 @@ InOutSpec in_out_specs[] =
>     "data/test-diff-filter/test-PR25661-7-report-4.txt",
>     "output/test-diff-filter/test-PR25661-7-report-4.txt",
>    },
> +  {
> +   "data/test-diff-filter/test-PR26309-v0.o",
> +   "data/test-diff-filter/test-PR26309-v1.o",
> +   "--no-default-suppression --leaf-changes-only",
> +   "data/test-diff-filter/test-PR26309-report-0.txt",
> +   "output/test-diff-filter/test-PR26309-report-0.txt",
> +  },
>    // This should be the last entry
>    {NULL, NULL, NULL, NULL, NULL}
>  };
> --
> 1.8.3.1
>
>
Looks good to me.

Reviewed-by: Giuliano Procida <gprocida@google.com>


>
> --
>                 Dodji
>
>
Dodji Seketeli Sept. 15, 2020, 7:32 a.m. UTC | #2
Hello,

Giuliano Procida <gprocida@google.com> writes:

[...]

> Looks good to me.
>
> Reviewed-by: Giuliano Procida <gprocida@google.com>

Thanks, applied to master.

Cheers,
diff mbox series

Patch

diff --git a/include/abg-reporter.h b/include/abg-reporter.h
index bf113f0..6aec6a6 100644
--- a/include/abg-reporter.h
+++ b/include/abg-reporter.h
@@ -175,9 +175,9 @@  public:
 	 const std::string& indent = "") const;
 
   void
-  report_local_typedef_changes(const typedef_diff &d,
-			       std::ostream& out,
-			       const std::string& indent) const;
+  report_non_type_typedef_changes(const typedef_diff &d,
+				  std::ostream& out,
+				  const std::string& indent) const;
 
   virtual void
   report(const typedef_diff& d, std::ostream& out,
diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
index 7f91753..11f506f 100644
--- a/src/abg-comparison.cc
+++ b/src/abg-comparison.cc
@@ -10901,14 +10901,15 @@  struct leaf_diff_node_marker_visitor : public diff_node_visitor
 	// Similarly, a *local* change describing a type that changed
 	// its nature doesn't make sense.
 	&& !is_distinct_diff(d)
-	// Similarly, a pointer (or reference or array) or qualified
-	// type change in itself doesn't make sense.  It's would
-	// rather make sense to show that pointer change as part of
-	// the variable change whose pointer type changed, for
+	// Similarly, a pointer (or reference or array), a typedef or
+	// qualified type change in itself doesn't make sense.  It
+	// would rather make sense to show that pointer change as part
+	// of the variable change whose pointer type changed, for
 	// instance.
 	&& !is_pointer_diff(d)
 	&& !is_reference_diff(d)
 	&& !is_qualified_type_diff(d)
+	&& !is_typedef_diff(d)
 	&& !is_array_diff(d)
 	// Similarly a parameter change in itself doesn't make sense.
 	// It should have already been reported as part of the change
@@ -11670,16 +11671,16 @@  struct suppression_categorization_visitor : public diff_node_visitor
 	//  2/ and has no local change (unless it's a pointer,
 	//  reference or qualified diff node).
 	//
-	//  Note that qualified type diff nodes are a bit special.
-	//  The local changes of the underlying type are considered
-	//  local for the qualified type, just like for
-	//  pointer/reference types.  But then the qualified type
-	//  itself can have local changes of its own, and those
-	//  changes are of the kind LOCAL_NON_TYPE_CHANGE_KIND.  So a
-	//  qualified type which have local changes that are *NOT* of
-	//  LOCAL_NON_TYPE_CHANGE_KIND (or that has no local changes
-	//  at all) and which is in the PRIVATE_TYPE_CATEGORY or
-	//  SUPPRESSED_CATEGORY can see these categories be
+	//  Note that qualified type and typedef diff nodes are a bit
+	//  special.  The local changes of the underlying type are
+	//  considered local for the qualified/typedef type, just like
+	//  for pointer/reference types.  But then the qualified or
+	//  typedef type itself can have local changes of its own, and
+	//  those changes are of the kind LOCAL_NON_TYPE_CHANGE_KIND.
+	//  So a qualified type which have local changes that are
+	//  *NOT* of LOCAL_NON_TYPE_CHANGE_KIND (or that has no local
+	//  changes at all) and which is in the PRIVATE_TYPE_CATEGORY
+	//  or SUPPRESSED_CATEGORY can see these categories be
 	//  propagated.
 	//
 	// Note that all pointer/reference diff node changes are
@@ -11687,7 +11688,7 @@  struct suppression_categorization_visitor : public diff_node_visitor
 	// pointed-to-type are considered local to the pointer itself.
 	//
 	// Similarly, changes local to the type of function parameters,
-	// variables (and data members) and classe (that are not of
+	// variables (and data members) and classes (that are not of
 	// LOCAL_NON_TYPE_CHANGE_KIND kind) and that have been
 	// suppressed can propagate their SUPPRESSED_CATEGORY-ness to
 	// those kinds of diff node.
@@ -11697,6 +11698,8 @@  struct suppression_categorization_visitor : public diff_node_visitor
 	    || is_reference_diff(d)
 	    || (is_qualified_type_diff(d)
 		&& (!(d->has_local_changes() & LOCAL_NON_TYPE_CHANGE_KIND)))
+	    || (is_typedef_diff(d)
+		&& (!(d->has_local_changes() & LOCAL_NON_TYPE_CHANGE_KIND)))
 	    || (is_function_decl_diff(d)
 		&& (!(d->has_local_changes() & LOCAL_NON_TYPE_CHANGE_KIND)))
 	    || (is_fn_parm_diff(d)
@@ -11758,12 +11761,19 @@  struct suppression_categorization_visitor : public diff_node_visitor
 	      canonical_diff->add_to_category(SUPPRESSED_CATEGORY);
 	  }
 
-	if (// We don't propagate "private type"-ness to typedefs
-	    // because defining "public" typedefs of private (opaque)
-	    // types is a common idiom.  So the typedef must stay
-	    // public.
-	    !is_typedef_diff(d)
-	    && has_non_empty_child
+	// Note that the private-ness of a an underlying type won't be
+	// propagated to its parent typedef, by virtue of the big "if"
+	// clause at the beginning of this function.  So we don't have
+	// to handle that case here.  So the idiom of defining
+	// typedefs of private (opaque) types will be respected;
+	// meaning that changes to opaque underlying type will be
+	// flagged as private and the typedef will be flagged private
+	// as well, unless the typedef itself has local non-type
+	// changes.  In the later case, changes to the typedef will be
+	// emitted because the typedef won't inherit the privateness
+	// of its underlying type.  So in practise, the typedef
+	// remains public for the purpose of change reporting.
+	if (has_non_empty_child
 	    && has_private_child
 	    && !has_non_private_child)
 	  {
@@ -12223,8 +12233,21 @@  struct redundancy_marking_visitor : public diff_node_visitor
 		// in the same sense as other types.  So we always
 		// propagate redundancy to them, regardless of if they
 		// have local changes or not.
+		//
+		// We also propagate redundancy to typedef types if
+		// these /only/ carry changes to their underlying
+		// type.
+		//
+		// Note that changes to the underlying type of a
+		// typedef is considered local of
+		// LOCAL_TYPE_CHANGE_KIND kind.  The other changes to the
+		// typedef itself are considered local of
+		// LOCAL_NON_TYPE_CHANGE_KIND kind.
 		|| is_pointer_diff(d)
-		|| is_qualified_type_diff(d)))
+		|| is_qualified_type_diff(d)
+		|| (is_typedef_diff(d)
+		    && (!(d->has_local_changes()
+			  & LOCAL_NON_TYPE_CHANGE_KIND)))))
 	  {
 	    bool has_non_redundant_child = false;
 	    bool has_non_empty_child = false;
diff --git a/src/abg-default-reporter.cc b/src/abg-default-reporter.cc
index f8572f2..0b572db 100644
--- a/src/abg-default-reporter.cc
+++ b/src/abg-default-reporter.cc
@@ -231,7 +231,11 @@  default_reporter::report(const enum_diff& d, ostream& out,
   d.reported_once(true);
 }
 
-/// For a @ref typedef_diff node, report the changes that are local.
+/// For a @ref typedef_diff node, report the local changes to the
+/// typedef rather the changes to its underlying type.
+///
+/// Note that changes to the underlying type are also considered
+/// local.
 ///
 /// @param d the @ref typedef_diff node to consider.
 ///
@@ -239,9 +243,9 @@  default_reporter::report(const enum_diff& d, ostream& out,
 ///
 /// @param indent the white space string to use for indentation.
 void
-default_reporter::report_local_typedef_changes(const typedef_diff &d,
-					       ostream& out,
-					       const string& indent) const
+default_reporter::report_non_type_typedef_changes(const typedef_diff &d,
+						  ostream& out,
+						  const string& indent) const
 {
   if (!d.to_be_reported())
     return;
@@ -283,7 +287,7 @@  default_reporter::report(const typedef_diff& d,
 
   typedef_decl_sptr f = d.first_typedef_decl(), s = d.second_typedef_decl();
 
-  report_local_typedef_changes(d, out, indent);
+  report_non_type_typedef_changes(d, out, indent);
 
   diff_sptr dif = d.underlying_type_diff();
   if (dif && dif->has_changes())
diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index 088eebb..dbb4364 100644
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -15778,9 +15778,11 @@  equals(const typedef_decl& l, const typedef_decl& r, change_kind* k)
 
   if (*l.get_underlying_type() != *r.get_underlying_type())
     {
+      // Changes to the underlying type of a typedef are considered
+      // local, a bit like for pointers.
       result = false;
       if (k)
-	*k |= SUBTYPE_CHANGE_KIND;
+	*k |= LOCAL_TYPE_CHANGE_KIND;
       else
 	return false;
     }
diff --git a/src/abg-leaf-reporter.cc b/src/abg-leaf-reporter.cc
index 0875c96..c2a766e 100644
--- a/src/abg-leaf-reporter.cc
+++ b/src/abg-leaf-reporter.cc
@@ -202,7 +202,9 @@  leaf_reporter::report(const typedef_diff& d,
   if (!diff_to_be_reported(&d))
     return;
 
-  report_local_typedef_changes(d, out, indent);
+  // all changes carried by a typedef_diff are considered local, so
+  // let's just call the default reporter here.
+  default_reporter::report(d, out, indent);
 
   maybe_report_interfaces_impacted_by_diff(&d, out, indent);
 }
diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index 5e2e8b1..7334db2 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -867,6 +867,11 @@  test-diff-filter/test-PR25661-7-report-1.txt \
 test-diff-filter/test-PR25661-7-report-2.txt \
 test-diff-filter/test-PR25661-7-report-3.txt \
 test-diff-filter/test-PR25661-7-report-4.txt \
+test-diff-filter/test-PR26309-v0.c           \
+test-diff-filter/test-PR26309-v1.c           \
+test-diff-filter/test-PR26309-v0.o           \
+test-diff-filter/test-PR26309-v1.o           \
+test-diff-filter/test-PR26309-report-0.txt   \
 \
 test-diff-suppr/test0-type-suppr-v0.cc	\
 test-diff-suppr/test0-type-suppr-v1.cc	\
diff --git a/tests/data/test-diff-filter/test-PR26309-report-0.txt b/tests/data/test-diff-filter/test-PR26309-report-0.txt
new file mode 100644
index 0000000..dec3758
--- /dev/null
+++ b/tests/data/test-diff-filter/test-PR26309-report-0.txt
@@ -0,0 +1,13 @@ 
+Leaf changes summary: 1 artifact changed
+Changed leaf types summary: 0 leaf type changed
+Removed/Changed/Added functions summary: 0 Removed, 1 Changed, 0 Added function
+Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
+
+1 function with some sub-type change:
+
+  [C] 'function void changed_fun(changed)' at test-PR26309-v1.c:3:1 has some sub-type changes:
+    parameter 1 of type 'typedef changed' changed:
+      underlying type 'int' changed:
+        type name changed from 'int' to 'long int'
+        type size changed from 32 to 64 (in bits)
+
diff --git a/tests/data/test-diff-filter/test-PR26309-v0.c b/tests/data/test-diff-filter/test-PR26309-v0.c
new file mode 100644
index 0000000..853084b
--- /dev/null
+++ b/tests/data/test-diff-filter/test-PR26309-v0.c
@@ -0,0 +1,5 @@ 
+typedef int changed;
+
+void changed_fun(changed y) {
+  (void) y;
+}
diff --git a/tests/data/test-diff-filter/test-PR26309-v0.o b/tests/data/test-diff-filter/test-PR26309-v0.o
new file mode 100644
index 0000000000000000000000000000000000000000..0e6fd52dcd00da7ba74fa6c2e88907223f9a6f3c
GIT binary patch
literal 2720
zcmbVN-D?v;5TCtkY&16X!>>}&6BS$VF0r*GD%EKHs3=k^f-j|9l1*}&4@s`2D&mU>
zz9|SI_~x_tNBB?p=!<`V&k8!Tx077A7X%k>XZE-Ao0;94-F<QG_H|1L;3nWA^hb;W
zd{6cDqEd@60poCg`Sp+2&wsCdEg?9hCen((!^v(n8Fdy3Aqv>rL#dz@MpR)((*SHN
zKBk_4uwBHhw+-ZsiIdrhjGdjzj*0Q%(SwK|06Tj=b0Kqc0==DpOy*+7PGd5jxHK8f
zoSG!#;-U9fCIM0@u+x#fbqM{QCf_P9l46Zhh(Ss%VsF7T_Coy1j$p6057#Rol-<x>
zS)E;&%b#;M^IlmoV0Yvn2m3ZH3mN6Q->S*VdUd0fz{%CyO_{5-D-VNQEeLaspyZc=
znjbWB+INlub#oOz^p)yXf@;;R28~d5Ks$#UckVk&&RMVE%{#OCGxPa{**Rx=RaTs3
zKa6m9?%a&yHp7jUT&&5K>;z>T`kiupajUT4E}V7T8o-UErSr}-x|{j8w`jq*P_2lk
zt01;U#nj&6^c&ox!-&&IP3`dI?C2|NX=K+`G>G8Dp2}$me*Q_Cv=b{9i$VWalVjsd
zQnPgdn{zhoG9fj#VR!(ZTst;o2H1}bz_SDJy*qHMeLRi*L)Xh-XGzA3ueY*2$d9j*
zBM?r$B!1M!A_0uE-F^c<qQ`jPRHMv#rvUYt2v&aL|J{q>6n9OJiw3TTa9t~=e_jja
z8N0e*bb$T*0Nl|y<sQ!;yzY}`=$BB2ok-SM3*}Y_9vx2bO5HAa9og``iY#r^)`M2H
zjdM!(OQnw7+&1lZ=wjGTX#_3F6bu1gx!r6^yfeL4JCq)tOSc>P<wx3)tglx)ev^Ng
z&i{YK6cTA~c|X}tQ7gSV)8l>xM!+!?gqbJqrnWbQ$v=e#R3HD#KZ9J;C&lFWRT{*a
zX-lsIJ!{37jZ&kx4!&ue>WUz)WqMhtE#hbTB>W3?*zQH5*PFI0IAdaDY-upxV@8T;
zdkf*D{J-h^nNRuCZD!_QLCo}t5X@@uDh*=Iv|Y!!r1)ulD0!bKKGl*L|1o0Zlk-GX
zwM`x9GXq+3e3s7;PvRffhnepicEdcDFEyX%S>D}t88EtYuRdtLY5qV1g+5hjyncq?
zp^~(JPQw1tYr%ZpS6YV|n|!~~;EG---RtxUnSA;u=Gj9CzH9I(4PuS=jUn=z#4qUl
KKe3^fCjT$}O}BFZ

literal 0
HcmV?d00001

diff --git a/tests/data/test-diff-filter/test-PR26309-v1.c b/tests/data/test-diff-filter/test-PR26309-v1.c
new file mode 100644
index 0000000..5f453a3
--- /dev/null
+++ b/tests/data/test-diff-filter/test-PR26309-v1.c
@@ -0,0 +1,5 @@ 
+typedef long changed;
+
+void changed_fun(changed y) {
+  (void) y;
+}
diff --git a/tests/data/test-diff-filter/test-PR26309-v1.o b/tests/data/test-diff-filter/test-PR26309-v1.o
new file mode 100644
index 0000000000000000000000000000000000000000..4b9339a5972c97a595ff6555ddaf82efea0f975e
GIT binary patch
literal 2752
zcmbtW-D?v;5TDJqHX0jJ>sP7Z35qRvmwr=~)@W_5A4sVRf=In2o8&aPNOIMxh%ZI(
zO+iq>H~#^j{0ID71RwQH@Ihzpc9QG%M9_iTnfcAv?q+s3&layNrGx+`0rSw+7)8FL
zD;Gks0E4gxZY;m}vi#!dkK3P%i1vh*beLdUmR+GS8dAiBn8i`1dr1jHg(Q)PoyGg!
zM<Aq)Q0i3;V+O>r>|jP_hqD7>Phai;;`>2nPi0PLE)HV0qmaqWqa8Ck;>=KFIX*<r
zg@bR-4FUA_V$H}ubrAEOpxpQYq{ER1o&xL43n-*u1V<r$X8Umr_Yc=AAW-eVURj-(
znH)Q5KNxpPQP9pPKPr1SQz?un)m*=-%4?N%AL>oNYI%MD`C7A~^5tgvu9vTRLB8%4
z-J(}@y?S1!%v1h$zU&5WDB5MOQn4#uJy5NL+PVr{xO&4{u%?_@XWE(=8=oGVnV7Uj
zR#n+rc7q7FCr^%Ab|YB#)m&Bis^yhXbz7y{+{WyTJu_w5Re%c%3#Y6ROgH-XY|+a1
zMhwK`RS+9FF}%Ak{Svq85aRR^g&Tb~n|q#G=-;u34iTI<QdjlB_g_gyncTS`_0Ma@
zNVFu8)O~E)Wc(1sj1u}{T33{?v#Z1;37MGI6(#g{m6#*}yE%a*?B0gr5x`T~J$s!d
z-rkw<9_*}){9{MGQ-N?=jgKGEK*Yk|j_~VR-)G>g<GBloX4W|jD33;Pm?!?bJ29N{
zSG0cGz<CD>2Ch|D(t=r+*GTIM?;q`#`-A`VA8<?Kw5IsFz-d2f1a1*|(297Cr9f>2
z;Lsrkr`T?T(^7TUDXZdob<OiDO|*rwTP(KJgDqyaMfb+mkh<q94#5)Ol$wo(!u!PW
zn}KrhwA$^!E#1?JRBf%&avMrd{6E<+#?#*Nesa2k7CQZ>^=@ZIz!7AGnJ4j*_BWXs
ze;ggCZ~irY9AnL#RFms--w{S+`qJY;PgeoIIHjYf5WZ=U`ida#ae8=&=I}Ff68@0}
zq-zoDjHd4jT1<?br3S}3k&$5fUPd@+{y+5mIX;Jebjz9ZFC%8=L<lA|c%GCPnZ9di
zOR7Jj4<+vt)u(&FtbZRd%E|LYF{FpO&?g2o=laZ_Af6O&>%+|V5U1fd=9@a6?aXg&
zxeOTHzE|&byy<>N1N09mPP~4`UZaq-e}#noqgRCEd0%NAW^Ky-R)Z^gp>*HV>tx2$
gZ(^Q3gy5?N50erj-ZzFQZ&LhCJ^v5vsJR*c8;XItO#lD@

literal 0
HcmV?d00001

diff --git a/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt b/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt
index aaf28bf..3e50dc7 100644
--- a/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt
+++ b/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt
@@ -1,6 +1,6 @@ 
 ================ changes of 'libspice-server.so.1.8.0'===============
-Leaf changes summary: 11 artifacts changed (11 filtered out)
-  Changed leaf types summary: 2 (11 filtered out) leaf types changed
+Leaf changes summary: 10 artifacts changed (10 filtered out)
+  Changed leaf types summary: 1 (10 filtered out) leaf types changed
   Removed/Changed/Added functions summary: 1 Removed, 0 Changed, 8 Added functions
   Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
 
@@ -120,87 +120,5 @@  Leaf changes summary: 11 artifacts changed (11 filtered out)
       function int spice_server_set_zlib_glz_compression(SpiceServer*, spice_wan_compression_t)
       function void spice_server_vm_start(SpiceServer*)
       function void spice_server_vm_stop(SpiceServer*)
-  'typedef spice_image_compression_t at spice.h:479:1' changed:
-    typedef name changed from spice_image_compression_t to SpiceImageCompression at enums.h:197:1
-    79 impacted interfaces:
-      function void spice_qxl_add_memslot(QXLInstance*, QXLDevMemSlot*)
-      function void spice_qxl_add_memslot_async(QXLInstance*, QXLDevMemSlot*, uint64_t)
-      function void spice_qxl_create_primary_surface(QXLInstance*, uint32_t, QXLDevSurfaceCreate*)
-      function void spice_qxl_create_primary_surface_async(QXLInstance*, uint32_t, QXLDevSurfaceCreate*, uint64_t)
-      function void spice_qxl_del_memslot(QXLInstance*, uint32_t, uint32_t)
-      function void spice_qxl_destroy_primary_surface(QXLInstance*, uint32_t)
-      function void spice_qxl_destroy_primary_surface_async(QXLInstance*, uint32_t, uint64_t)
-      function void spice_qxl_destroy_surface_async(QXLInstance*, uint32_t, uint64_t)
-      function void spice_qxl_destroy_surface_wait(QXLInstance*, uint32_t)
-      function void spice_qxl_destroy_surfaces(QXLInstance*)
-      function void spice_qxl_destroy_surfaces_async(QXLInstance*, uint64_t)
-      function void spice_qxl_driver_unload(QXLInstance*)
-      function void spice_qxl_flush_surfaces_async(QXLInstance*, uint64_t)
-      function void spice_qxl_loadvm_commands(QXLInstance*, QXLCommandExt*, uint32_t)
-      function void spice_qxl_monitors_config_async(QXLInstance*, QXLPHYSICAL, int, uint64_t)
-      function void spice_qxl_oom(QXLInstance*)
-      function void spice_qxl_reset_cursor(QXLInstance*)
-      function void spice_qxl_reset_image_cache(QXLInstance*)
-      function void spice_qxl_reset_memslots(QXLInstance*)
-      function void spice_qxl_set_max_monitors(QXLInstance*, unsigned int)
-      function void spice_qxl_start(QXLInstance*)
-      function void spice_qxl_stop(QXLInstance*)
-      function void spice_qxl_update_area(QXLInstance*, uint32_t, QXLRect*, QXLRect*, uint32_t, uint32_t)
-      function void spice_qxl_update_area_async(QXLInstance*, uint32_t, QXLRect*, uint32_t, uint64_t)
-      function void spice_qxl_wakeup(QXLInstance*)
-      function int spice_server_add_client(SpiceServer*, int, int)
-      function int spice_server_add_interface(SpiceServer*, SpiceBaseInstance*)
-      function int spice_server_add_renderer(SpiceServer*, const char*)
-      function int spice_server_add_ssl_client(SpiceServer*, int, int)
-      function void spice_server_char_device_wakeup(SpiceCharDeviceInstance*)
-      function void spice_server_destroy(SpiceServer*)
-      function spice_image_compression_t spice_server_get_image_compression(SpiceServer*)
-      function int spice_server_get_num_clients(SpiceServer*)
-      function int spice_server_get_peer_info(SpiceServer*, sockaddr*, socklen_t*)
-      function int spice_server_get_sock_info(SpiceServer*, sockaddr*, socklen_t*)
-      function int spice_server_init(SpiceServer*, SpiceCoreInterface*)
-      function int spice_server_is_server_mouse(SpiceServer*)
-      function int spice_server_migrate_connect(SpiceServer*, const char*, int, int, const char*)
-      function int spice_server_migrate_end(SpiceServer*, int)
-      function int spice_server_migrate_info(SpiceServer*, const char*, int, int, const char*)
-      function int spice_server_migrate_start(SpiceServer*)
-      function int spice_server_migrate_switch(SpiceServer*)
-      function SpiceServer* spice_server_new()
-      function void spice_server_playback_get_buffer(SpicePlaybackInstance*, uint32_t**, uint32_t*)
-      function void spice_server_playback_put_samples(SpicePlaybackInstance*, uint32_t*)
-      function void spice_server_playback_set_mute(SpicePlaybackInstance*, uint8_t)
-      function void spice_server_playback_set_volume(SpicePlaybackInstance*, uint8_t, uint16_t*)
-      function void spice_server_playback_start(SpicePlaybackInstance*)
-      function void spice_server_playback_stop(SpicePlaybackInstance*)
-      function void spice_server_port_event(SpiceCharDeviceInstance*, uint8_t)
-      function uint32_t spice_server_record_get_samples(SpiceRecordInstance*, uint32_t*, uint32_t)
-      function void spice_server_record_set_mute(SpiceRecordInstance*, uint8_t)
-      function void spice_server_record_set_volume(SpiceRecordInstance*, uint8_t, uint16_t*)
-      function void spice_server_record_start(SpiceRecordInstance*)
-      function void spice_server_record_stop(SpiceRecordInstance*)
-      function void spice_server_set_addr(SpiceServer*, const char*, int)
-      function int spice_server_set_agent_copypaste(SpiceServer*, int)
-      function int spice_server_set_agent_file_xfer(SpiceServer*, int)
-      function int spice_server_set_agent_mouse(SpiceServer*, int)
-      function int spice_server_set_channel_security(SpiceServer*, const char*, int)
-      function int spice_server_set_compat_version(SpiceServer*, spice_compat_version_t)
-      function int spice_server_set_exit_on_disconnect(SpiceServer*, int)
-      function int spice_server_set_image_compression(SpiceServer*, spice_image_compression_t)
-      function int spice_server_set_jpeg_compression(SpiceServer*, spice_wan_compression_t)
-      function int spice_server_set_listen_socket_fd(SpiceServer*, int)
-      function void spice_server_set_name(SpiceServer*, const char*)
-      function int spice_server_set_noauth(SpiceServer*)
-      function int spice_server_set_playback_compression(SpiceServer*, int)
-      function int spice_server_set_port(SpiceServer*, int)
-      function int spice_server_set_sasl(SpiceServer*, int)
-      function int spice_server_set_sasl_appname(SpiceServer*, const char*)
-      function void spice_server_set_seamless_migration(SpiceServer*, int)
-      function int spice_server_set_streaming_video(SpiceServer*, int)
-      function int spice_server_set_ticket(SpiceServer*, const char*, int, int, int)
-      function int spice_server_set_tls(SpiceServer*, int, const char*, const char*, const char*, const char*, const char*, const char*)
-      function void spice_server_set_uuid(SpiceServer*, const uint8_t*)
-      function int spice_server_set_zlib_glz_compression(SpiceServer*, spice_wan_compression_t)
-      function void spice_server_vm_start(SpiceServer*)
-      function void spice_server_vm_stop(SpiceServer*)
 ================ end of changes of 'libspice-server.so.1.8.0'===============
 
diff --git a/tests/test-diff-filter.cc b/tests/test-diff-filter.cc
index bdf7d41..b3b1e91 100644
--- a/tests/test-diff-filter.cc
+++ b/tests/test-diff-filter.cc
@@ -766,6 +766,13 @@  InOutSpec in_out_specs[] =
    "data/test-diff-filter/test-PR25661-7-report-4.txt",
    "output/test-diff-filter/test-PR25661-7-report-4.txt",
   },
+  {
+   "data/test-diff-filter/test-PR26309-v0.o",
+   "data/test-diff-filter/test-PR26309-v1.o",
+   "--no-default-suppression --leaf-changes-only",
+   "data/test-diff-filter/test-PR26309-report-0.txt",
+   "output/test-diff-filter/test-PR26309-report-0.txt",
+  },
   // This should be the last entry
   {NULL, NULL, NULL, NULL, NULL}
 };