[Bug default/20199] New: disregard harmless type synonyms

[Bug default/20199] New: disregard harmless type synonyms

[woodard at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=20199

            Bug ID: 20199
           Summary: disregard harmless type synonyms
           Product: libabigail
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: default
          Assignee: dodji at redhat dot com
          Reporter: woodard at redhat dot com
                CC: libabigail at sourceware dot org
  Target Milestone: ---

I compiled up libelf with gcc 6.1.0 and Intel 16.0.2 and then ran them through
abidiff

ipa15@ben: abidiff
opt/spack/chaos_5_x86_64_ib/gcc-6.1.0/libelf-0.8.13-ftzwve3mpopzjgv55lt5iisid2ts5j4r/lib/libelf.so
opt/spack/chaos_5_x86_64_ib/intel-16.0.2/libelf-0.8.13-kextzapiqvjhjqkph2m2utoabt5pd7im/lib/libelf.so


And there are a lot of warnings along the lines of:

Functions changes summary: 0 Removed, 11 Changed (103 filtered out), 0 Added
functions
Variables changes summary: 0 Removed, 1 Changed (2 filtered out), 0 Added
variable

11 functions with some indirect sub-type change:

  [C]'function size_t _elf32_xltsize(const Elf_Data*, unsigned int, unsigned
int, int)' at 32.xlatetof.c:336:1 has some indirect sub-type changes:
    return type changed:
      underlying type 'long unsigned int' changed:
        type name changed from 'long unsigned int' to 'unsigned long'
...

To me a human, this sounds com pletely kosher. We can easily know that "long
unsigned int" and "unsigned long" are the same length and I can't think of any
way in the C language where they could actually be different. Abidiff should
suppress those warnings.

The object files that led to this report are attached to PR:20194

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/20199] disregard harmless type synonyms

[woodard at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=20199

--- Comment #3 from Ben Woodard <woodard at redhat dot com> ---
(In reply to dodji from comment #2)
> The way libabigail is designed to deal with these spurious differences is to
> consider the two types as being different and then filter out that
> difference later at reporting type.

Why aren't they the same internally?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/20199] disregard harmless type synonyms

[dodji at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=20199

dodji at redhat dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/20199] disregard harmless type synonyms

[dodji at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=20199

--- Comment #2 from dodji at redhat dot com ---
Yes, this would indeed be a useful feature.

If I understand correctly, I think the existence of these synonyms is due to
the fact that type specifiers can be present in any order and that the
"unsigned" and "signed" type specifiers can be used alone (i.e, not
conceptually follow or precede another type specifier), in which case they mean
"unsigned or signed int".  So doing a naive lexicographic string comparison of
type names to detect if they are different doesn't work in this case.

The way libabigail is designed to deal with these spurious differences is to
consider the two types as being different and then filter out that difference
later at reporting type.

Note that when we are in presence of a diff between two types that have
different names, that diff is represented in the Abigail internal
representation (IR) by the class abigail::comparison::distinct_diff, documented
at
https://sourceware.org/libabigail/apidoc/classabigail_1_1comparison_1_1distinct__diff.html.

So I am guessing that we could add code that would detect that those two
distinct types (once their types qualifiers have been stripped off) are base
types (represented in the IR by the class abigail::ir::type_decl, documented at
https://sourceware.org/libabigail/apidoc/classabigail_1_1ir_1_1type__decl.html)
and parse the type-specifiers of the type into a bitmap, each bit representing
one type-specifier.  If the two bitmaps are equal then the two types are
synonyms.  I think this detection should be restricted to binaries coming from
C and C++ languages only.

This work would happen in the pass that walks the graph of diff nodes to detect
harmless diff nodes, in abg-comp-filter.cc, especially in the member function 
harmless_filter::visit().  When two subjects of a diff are detected as synonyms
then the diff node should be categorized as HARMLESS_DECL_NAME_CHANGE_CATEGORY.

Note that that "filtering" passes where different filters are applied to the
graph of diff nodes are triggered the member function
corpus_diff::apply_filters_and_suppressions_before_reporting() that is itself
invoked in the member function corpus_diff::report().

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/20199] disregard harmless type synonyms

[woodard at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=20199

Ben Woodard <woodard at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P2                          |P1
           Severity|normal                      |critical

--- Comment #5 from Ben Woodard <woodard at redhat dot com> ---
This is the other very important bug. We get this and the void* idiom issue
sorted out and then the signal to noise ratio for looking at real problems gets
VASTLY better.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/20199] disregard harmless type synonyms

[woodard at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=20199

--- Comment #6 from Ben Woodard <woodard at redhat dot com> ---
Most of these problems are unsigned to long unsigned and things like that but
remember to also take care of:

type name changed from '_Bool' to 'unsigned char'

This is a new one that I've been seeing

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/20199] consider integral type synonyms as being equal

[dodji at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=20199

dodji at redhat dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|disregard harmless type     |consider integral type
                   |synonyms                    |synonyms as being equal

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/20199] consider integral type synonyms as being equal

[dodji at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=20199

dodji at redhat dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #8 from dodji at redhat dot com ---
This issue should be fixed by commit
https://sourceware.org/git/?p=libabigail.git;a=commit;h=ca133d551bc209f5e2e9bc73cd87d2bc3400dc25
in the master branch of the Git repository.

It should be available in the 1.0.rc6 release.

Thank you for reporting this problem!

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/20199] disregard harmless type synonyms

[dodji at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=20199

--- Comment #4 from dodji at redhat dot com ---
"woodard at redhat dot com" <sourceware-bugzilla@sourceware.org> writes:

> Why aren't they the same internally?

I think that for real synonyms like "unsigned" and "unsigned int", or
like "long unsigned int" and "unsigned long", they should be the same
internally :-)

The reason why they are not the same internally today (which is a bug, I
think) is that from DWARF, we just get their name as a string.  We just
get "long unsigned int" for one type and "unsigned long" for the second.

There is some extra work to do to figure out that they are the same
type, and we haven't done that work yet.  And I think we need to do it
to fix the issue.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/20199] consider integral type synonyms as being equal

[dodji at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=20199

--- Comment #7 from dodji at redhat dot com ---
(In reply to Ben Woodard from comment #6)
> but remember to also take care of:
> 
> type name changed from '_Bool' to 'unsigned char'

This is a different problem.

_Bool and unsigned char are different built-in types.  _Bool appeared in C11
(http://en.cppreference.com/w/c/types/boolean).  So I think they ought to
compare different.

Whereas 'unsigned short' and "short unsigned int" are exactly the same type,
and so should compare equal, even if the strings representing their names are
different.

In my opinion the '_Bool' to 'unsigned char' change is potentially a real
problem, at least from an API point of view.  So I think it's useful that
abidiff tells users about it.  If, in a particular context of a particular
project users (after carefully reviewing all the reported instances of changes
from _Bool to unsigned char) decide that they want the tool to suppress those
change reports, then I think they should use a suppression specification
instead.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/20199] disregard harmless type synonyms

[woodard at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=20199

--- Comment #1 from Ben Woodard <woodard at redhat dot com> ---
A very similar problem is seen between PGI 13.6 and gcc 6.1

For example:
  [C]'function void __elf_assert(const char*, unsigned int, const char*)' at
assert.c:29:1 has some\
 indirect sub-type changes:
    parameter 2 of type 'unsigned int' changed:
      type name changed from 'unsigned int' to 'unsigned'

Will you need me to enumerate all the synonyms or can you figure it out?

-- 
You are receiving this mail because:
You are on the CC list for the bug.