Re: [Bug default/21486] New: missing const when comparing C++ object files

Re: [Bug default/21486] New: missing const when comparing C++ object files

[Dodji Seketeli]

"woodard at redhat dot com" <sourceware-bugzilla@sourceware.org> a
écrit:


> Doing some introspection on libabigail itself with different compilers it flags
> this error:
>
>   [C]'function void abigail::dump(abigail::ir::translation_unit_sptr,
> std::ostream&, bool)' at abg-writer.cc:4091:1 has some indirect sub-type
> changes:
>     parameter 1 of type 'typedef abigail::ir::translation_unit_sptr' changed:
>       entity changed from 'typedef abigail::ir::translation_unit_sptr' to
> 'const abigail::ir::translation_unit_sptr'
>       type size hasn't changed
>     parameter 3 of type 'bool' changed:
>       entity changed from 'bool' to 'const bool'
>       type size hasn't changed

Could you please attach the exact two binaries you were comparing, so
that I am sure to see exactly what you were seeing?

Thanks.

-- 
		Dodji

[Bug default/21486] missing const when comparing C++ object files

[woodard at redhat dot com]

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

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

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

[Bug default/21486] top-level const qualifiers on types of function parameters are not ignored

[dodji at redhat dot com]

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

--- Comment #11 from dodji at redhat dot com ---
This issue is addressed in the dodji/kabidiff branch by commit
https://sourceware.org/git/gitweb.cgi?p=libabigail.git;a=commit;h=56be011037bdca65ca965755aa2241a548c4398e.

In particular, that patch makes libabigail filter out changes about top
const/volatile qualifiers of function parameter types because they are now
considered as being harmless.

Users can still see those changes, should they want to do so, (as those changes
might have API consequences, even if they are not harmful as far as ABI is
concerned) if they use the --harmless option of abidiff.

Please note however that for abidiff to consider that the two binaries
(attached to this bug) have no functions sub-type changes, 3 patches are
needed, including the one cited above.  They are all present in the
dodji/kabidiff branch:

08d1fa8 Symbols with the same zero value are not aliases
8c48e8d Support ELF symbol visibility property
56be011 Filter top cv qualifier changes on function parameter types

Please note that this branch is soon going to be merged in the master branch. I
will thus close the bug when that merge happens.

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

[Bug default/21486] missing const when comparing C++ object files

[woodard at redhat dot com]

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

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

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

[Bug default/21486] missing const when comparing C++ object files

[woodard at redhat dot com]

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

--- Comment #4 from Ben Woodard <woodard at redhat dot com> ---
The problem with libabigail is that it drops the "const" which is an ABI
artifact in both objects. 

It is not clear to me that clang's statement that the type is typedef'd is in
fact an artifact. That could be a problem with clang. I'm not sure.

The second problem with parameter 3 may be a GCC problem:
 [ 41147]      subprogram
               external             (flag_present) yes
               name                 (strp) "dump"
               decl_file            (data1) 2
               decl_line            (data2) 4069
               linkage_name         (strp)
"abigail::dump(abigail::ir::translation_unit const&, std::basic_ostream<char,
std::char_traits<char> >&, bool)"
               declaration          (flag_present) yes
               sibling              (ref4) [ 41167]
 [ 41157]        formal_parameter
                 type                 (ref4) [ 6b4b9]
 [ 4115c]        formal_parameter
                 type                 (ref4) [ 42e87]
 [ 41161]        formal_parameter
                 type                 (ref4) [ 3c1a1]

 [ 3c1a1]    base_type
             byte_size            (data1) 1
             encoding             (data1) boolean (2)
             name                 (strp) "bool"

To me this looks like it should be 3c1a9 which is:
 [ 3c1a9]    const_type
             type                 (ref4) [ 3c1a1]

If you agree then I will submit a gcc PR regarding that.

Clang seems to get this correct:

 [ 6a5cd]        formal_parameter
                 location             (sec_offset) location list [ 1118f]
                 name                 (strp) "annotate"
                 decl_file            (data1) 36
                 decl_line            (data2) 4091
                 type                 (ref4) [ 6ad8e]

 [ 6ad8e]    const_type
             type                 (ref4) [ 2064a]
 [ 2064a]    base_type
             name                 (strp) "bool"
             encoding             (data1) boolean (2)
             byte_size            (data1) 1

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

[Bug default/21486] top-level const qualifiers on types of function parameters are not ignored

[dodji at redhat dot com]

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

dodji at redhat dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|missing const when          |top-level const qualifiers
                   |comparing C++ object files  |on types of function
                   |                            |parameters are not ignored

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

[Bug default/21486] missing const when comparing C++ object files

[woodard at redhat dot com]

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

--- Comment #3 from Ben Woodard <woodard at redhat dot com> ---
This is the clang debug info:

 [ 6a594]      subprogram
               low_pc               (addr) .text+0x00000000000083d0
<abigail::dump(std::tr1::shared_ptr<abigail::ir::translation_unit>,
std::basic_ostream<char, std::char_traits<char> >&, bool)>
               high_pc              (data4) 17 (.text+0x00000000000083e1)
               frame_base           (exprloc) 
                [   0] reg7
               linkage_name         (strp)
"abigail::dump(std::tr1::shared_ptr<abigail::ir::translation_unit>,
std::basic_ostream<char, std::char_traits<char> >&, bool)"
               name                 (strp) "dump"
               decl_file            (data1) 36
               decl_line            (data2) 4091
               external             (flag_present) yes
 [ 6a5ae]        formal_parameter
                 location             (sec_offset) location list [ 1116b]
                 name                 (strp) "t"
                 decl_file            (data1) 36
                 decl_line            (data2) 4091
                 type                 (ref4) [ 6b0cc]
 [ 6a5be]        formal_parameter
                 location             (exprloc) 
                  [   0] reg4
                 name                 (strp) "o"
                 decl_file            (data1) 36
                 decl_line            (data2) 4091
                 type                 (ref4) [ 6bee7]
 [ 6a5cd]        formal_parameter
                 location             (sec_offset) location list [ 1118f]
                 name                 (strp) "annotate"
                 decl_file            (data1) 36
                 decl_line            (data2) 4091
                 type                 (ref4) [ 6ad8e]

[ 6b0cc]    const_type
             type                 (ref4) [ 29757]

[29757]        typedef
                 type                 (ref4) [  3848]
                 name                 (strp) "translation_unit_sptr"
                 decl_file            (data1) 8
                 decl_line            (data1) 121
[  3848]        class_type
                 name                 (strp)
"shared_ptr<abigail::ir::translation_unit>"
                 byte_size            (data1) 16
                 decl_file            (data1) 10
                 decl_line            (data2) 983

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

[Bug default/21486] missing const when comparing C++ object files

[dodji at redhat dot com]

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

dodji at redhat dot com changed:

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

--- Comment #8 from dodji at redhat dot com ---
(In reply to dodji from comment #7)
> Could you please attach the exact two binaries you were comparing, so
> that I am sure to see exactly what you were seeing?


Grrr, sorry Ben, I have just seen right now that you've attached the binary
already.  I should learn to look closer I guess :)

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

[Bug default/21486] missing const when comparing C++ object files

[dodji at seketeli dot org]

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

--- Comment #7 from dodji at seketeli dot org ---
"woodard at redhat dot com" <sourceware-bugzilla@sourceware.org> a
écrit:


> Doing some introspection on libabigail itself with different compilers it flags
> this error:
>
>   [C]'function void abigail::dump(abigail::ir::translation_unit_sptr,
> std::ostream&, bool)' at abg-writer.cc:4091:1 has some indirect sub-type
> changes:
>     parameter 1 of type 'typedef abigail::ir::translation_unit_sptr' changed:
>       entity changed from 'typedef abigail::ir::translation_unit_sptr' to
> 'const abigail::ir::translation_unit_sptr'
>       type size hasn't changed
>     parameter 3 of type 'bool' changed:
>       entity changed from 'bool' to 'const bool'
>       type size hasn't changed

Could you please attach the exact two binaries you were comparing, so
that I am sure to see exactly what you were seeing?

Thanks.

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

[Bug default/21486] New: missing const when comparing C++ object files

[woodard at redhat dot com]

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

            Bug ID: 21486
           Summary: missing const when comparing C++ object files
           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: ---

Doing some introspection on libabigail itself with different compilers it flags
this error:

  [C]'function void abigail::dump(abigail::ir::translation_unit_sptr,
std::ostream&, bool)' at abg-writer.cc:4091:1 has some indirect sub-type
changes:
    parameter 1 of type 'typedef abigail::ir::translation_unit_sptr' changed:
      entity changed from 'typedef abigail::ir::translation_unit_sptr' to
'const abigail::ir::translation_unit_sptr'
      type size hasn't changed
    parameter 3 of type 'bool' changed:
      entity changed from 'bool' to 'const bool'
      type size hasn't changed

[ben@localhost build]$ g++ --version
g++ (GCC) 6.3.1 20161221 (Red Hat 6.3.1-1)
Copyright (C) 2016 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

[ben@localhost build]$ clang++ --version
clang version 3.9.1 (tags/RELEASE_391/final)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

The function definition does seem to have a const:
void
dump(const translation_unit_sptr t, std::ostream& o, const bool annotate)
{
  if (t)
    dump(*t, o, annotate);
}


digging through the readelf output it looks like it is a libabigail bug:

GCC's debug-info looks correct:
 [ 41147]      subprogram
               external             (flag_present) yes
               name                 (strp) "dump"
               decl_file            (data1) 2
               decl_line            (data2) 4069
               linkage_name         (strp)
"abigail::dump(abigail::ir::translation_unit const&, std::basic_ostream<char,
std::char_traits<char> >&, bool)"
               declaration          (flag_present) yes
               sibling              (ref4) [ 41167]
 [ 41157]        formal_parameter
                 type                 (ref4) [ 6b4b9]
 [ 4115c]        formal_parameter
                 type                 (ref4) [ 42e87]
 [ 41161]        formal_parameter
                 type                 (ref4) [ 3c1a1]

 [ 6b4b9]    reference_type
             byte_size            (data1) 8
             type                 (ref4) [ 3d90e]
 [ 3d90e]        const_type
                 type                 (ref4) [ 3d7dd]
 [ 3d7dd]        class_type
                 name                 (strp) "translation_unit"
                 declaration          (flag_present) yes
                 sibling              (ref4) [ 3d90e]

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

[Bug default/21486] missing const when comparing C++ object files

[woodard at redhat dot com]

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

--- Comment #5 from Ben Woodard <woodard at redhat dot com> ---
According to Jason, const-ness is not an ABI artifact.

<jason> it shouldn't be part of the ABI
<neb_> IIRC didn't you say that it was part of the ABI because foo( type a);
and foo( const type a); were different functions.
<jason> I don't remember saying that, and they aren't
<jason> as jakub said, the 'const' only affects the body of the function, it's
not part of the function's type
<mjw> OK, so it is both a bug in libabigail (for treating it as an ABI issue)
and in gcc (for not emitting it as DWARF type - except we cannot replicate - I
just tried jakub's example with g++ 4.8.5 and it does emit the const type in
DWARF)
<jason> "After producing the list of parameter types, any top-level
cv-qualifiers modifying a parameter type are deleted when forming the function
type."

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

[Bug default/21486] top-level const qualifiers on types of function parameters are not ignored

[dodji at redhat dot com]

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

--- Comment #9 from dodji at redhat dot com ---
I started to hack on this.  I am categorizing changes in which top-level const
qualifiers of function parameter types as being harmless.  As a result, those
changes are filtered out (by default) by libabigail.  Note that users can still
see those harmless changes by doing using the --harmless option of abidiff.

Looking at the result of comparing the two attached binaries, it appears to me
that there are other things (not related to the title of this opened bug) to
fix, so that abidiff concludes that the two binaries have equivalent ABIs.

Ideally, I'd thus have to construct smaller test cases that expose the problems
separately.  That way, I can use each of the small test cases as regression
tests in each of the patches that resolve the problem they expose.

I should thus build a version of Clang that exhibits those issues and maybe
file separate bugs for the issues I am seeing.

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

[Bug default/21486] top-level const qualifiers on types of function parameters are not ignored

[woodard at redhat dot com]

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

--- Comment #10 from Ben Woodard <woodard at redhat dot com> ---
Note: if you spin up a F25 VM you can have direct access to the same version of
clang and gcc that I was using when I spotted this problem. Having a
pre-compiled clang in the distro is handy.

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

[Bug default/21486] missing const when comparing C++ object files

[dodji at redhat dot com]

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

--- Comment #6 from dodji at redhat dot com ---
This is one of those cases where we need to categorize a certain kind of change
as being harmless.  In this particular case, we need to categorize a const or
volatile qualifier change as being harmless, by default.

Changes categorized as harmless are filtered out from the output, by default.

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

[Bug default/21486] top-level const qualifiers on types of function parameters are not ignored

[dodji at redhat dot com]

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

dodji at redhat dot com changed:

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

--- Comment #12 from dodji at redhat dot com ---
The fix for this issue has been merged into master.

Thanks for filling this issue.

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