[Bug default/20365] New: suppress apparently harmless restrict type qualification change

[Bug default/20365] New: suppress apparently harmless restrict type qualification change

[woodard at redhat dot com]

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

            Bug ID: 20365
           Summary: suppress apparently harmless restrict type
                    qualification change
           Product: libabigail
           Version: unspecified
            Status: NEW
          Severity: minor
          Priority: P2
         Component: default
          Assignee: dodji at redhat dot com
          Reporter: woodard at redhat dot com
                CC: libabigail at sourceware dot org
  Target Milestone: ---

Currently abidiff warns that the type has changed when the restrict qualifier
is omitted by different compilers. For example:

 [C]'function int xfprintf(FILE* restrict, const char* restrict, ...)' at
xprintf.c:58:1 has some indirect sub-type changes:
   parameter 1 of type 'FILE* restrict' changed:
     entity changed from 'FILE* restrict' to 'FILE*'
   parameter 2 of type 'const char* restrict' changed:
     entity changed from 'const char* restrict' to 'const char*'

After a bit of discussion, we decided that this has no impact on ABI and so we
should suppress this output by default.

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

[Bug default/20365] suppress apparently harmless restrict type qualification change

[mjw at redhat dot com]

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

Mark Wielaard <mjw at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mjw at redhat dot com

--- Comment #3 from Mark Wielaard <mjw at redhat dot com> ---
(In reply to Ben Woodard from comment #0)
> After a bit of discussion, we decided that this has no impact on ABI and so
> we should suppress this output by default.

Would you mind posting the discussion or conclusion? It is a little surprising
that adding or dropping the restrict qualifier from the function pointer
arguments doesn't indicate to the programmer that the API has changed. Is it
totally an compiler/implementation detail? Is there never a reason to want to
know about the appearance/disappearance of restrict qualifiers?

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

[Bug default/20365] suppress apparently harmless restrict type qualification change

[woodard at redhat dot com]

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

--- Comment #2 from Ben Woodard <woodard at redhat dot com> ---
Created attachment 9388
  --> https://sourceware.org/bugzilla/attachment.cgi?id=9388&action=edit
icc object

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

[Bug default/20365] suppress apparently harmless restrict type qualification change

[woodard at redhat dot com]

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

--- Comment #4 from Ben Woodard <woodard at redhat dot com> ---
This discussion was with Dodji. He pointed out that C++ function mangling and
the C++ function matching remain the same despite the addition or loss of
restrict qualifier. This differs markedly in comparison to the const qualifier
which leads to a different function fingerprint.

Personally, I remain somewhat unconvinced but have no counter arguments.
Unequipped with any arguments my position is that we should document the
decision and if someone argues with it as you apparently are, then we discuss
it further taking it all the way to the language standards bodies or the
maintainers of the ABI standard if necessary.

Also, if at some point in the future we discover a case where the addition or
removal of restrict does impact run time linking between objects then the issue
most likely should be specified either in the language or the ABI for the
platform.

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

[Bug default/20365] suppress apparently harmless restrict type qualification change

[dodji at redhat dot com]

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

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

> --- Comment #4 from Ben Woodard <woodard at redhat dot com> ---
> This discussion was with Dodji. He pointed out that C++ function mangling and
> the C++ function matching remain the same despite the addition or loss of
> restrict qualifier.

My point was rather that in *C*, the name of the symbol of the function
taking a parameter of a qualified type wouldn't change if the qualifier
is not present.

In C++, though, qualifiers do have an effect on the mangling of the
function symbol name.  So if a qualifier is dropped from the type of a
function parameter, libabigail doesn't even have to compare types to
spot it.  It'll spot it by just looking at functions and variables
symbols names.

There are cases, though, where qualifiers won't matter from an ABI point
of view: it's when we are only looking at the type of a data member, as
opposed to looking at the type of a function parameter.  And this is
what I had in mind.

So, if we can infer that a qualified type is *never* used as a function
parameter type (or as the type of a global variable) in a C++ program
and that the only qualifier changes are seen when looking at data
members then we might want to suppress those qualifier changes.

Mark does have a point, though.  It's that a qualifier change, *is* a
type change.  It's does at least have an impact at the API level.  So,
in general, I think we want to show those changes.  It's only on a
carefully selected small number of instances that we might want to
suppress those changes, IMHO.

Note that today, the only cases where libabigail is dropping qualifiers
today, is on the "const void" type (because I think showing that change
is of little practical value) and on const references, because a
reference is always const.  These two cases were leading to lots of
useless spurious change reports.

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

[Bug default/20365] suppress apparently harmless restrict type qualification change

[woodard at redhat dot com]

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

--- Comment #1 from Ben Woodard <woodard at redhat dot com> ---
Created attachment 9387
  --> https://sourceware.org/bugzilla/attachment.cgi?id=9387&action=edit
gcc object

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

[Bug default/20365] suppress apparently harmless restrict type qualification change

[woodard at redhat dot com]

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

--- Comment #6 from Ben Woodard <woodard at redhat dot com> ---
I requested that my contact for Intel compiler support file a bug with the
Intel to get this fixed.

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

[Bug default/20365] suppress apparently harmless restrict type qualification change

[dodji at redhat dot com]

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

--- Comment #7 from dodji at redhat dot com ---
There have been new development about this topic.

Right now, for function parameter and return types, as well as global variable
types, libabigail now automatically suppresses changes in CV qualifiers (e.g,
const, volatile, restrict, etc) by default.  Users who wants to still see these
can tell abidiff to show them harmless output.

Comparing the m4.icc against the m4.gcc still yields changes, though.  Notably:

type of 'bool_bitfield symbol::blind_no_args' changed:
  underlying type 'bool' changed:
    type name changed from 'bool' to 'unsigned char'
    type size hasn't changed

In this case, because the type size hasn't change I think we could add a
categorizer in libabigail's comparison engine to make categorize this kind of
change as being harmless.

The other kind of changes we have is this one:

type of variable changed:
 entity changed from 'const char* const[] const' to 'const char* const[9]'
 type size changed from 0 to 576 (in bits)

In C and C++ I think we can say that this kind of change is harmless, so it
would be useful that libabigail recognizes and categorises this kind of array
change as being harmless.

We can either open specific bug reports about these two feature enhancement
requests or we can just leave this one open.

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