[Bug default/24731] abidiff: Incorrect ABI difference reporting for union changing order of members

[Bug default/24731] abidiff: Incorrect ABI difference reporting for union changing order of members

[dodji at redhat dot com]

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

dodji at redhat dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2019-06-25
     Ever confirmed|0                           |1

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

[Bug default/24731] abidiff: Incorrect ABI difference reporting for union changing order of members

[woodard at redhat dot com]

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

--- Comment #2 from Ben Woodard <woodard at redhat dot com> ---
Further supporting that supposition that there are two bugs the corpus looks
correct:

$ abidw test1.o | sed -e 's/test1/test2/' > test1.dw # so the filenames don't
add noise
$ abidw test2.o > test2.dw
$ diff -u test1.dw test2.dw 
--- test1.dw    2019-06-25 13:38:41.510614347 -0700
+++ test2.dw    2019-06-25 13:36:17.003307489 -0700
@@ -9,15 +9,15 @@
         <var-decl name='a' type-id='type-id-3' visibility='default'
filepath='/home/ben/Work/libabigail/test/test2.c' line='1' column='1'/>
       </data-member>
       <data-member access='private'>
-        <var-decl name='c' type-id='type-id-4' visibility='default'
filepath='/home/ben/Work/libabigail/test/test2.c' line='1' column='1'/>
+        <var-decl name='s' type-id='type-id-4' visibility='default'
filepath='/home/ben/Work/libabigail/test/test2.c' line='1' column='1'/>
       </data-member>
       <data-member access='private'>
-        <var-decl name='s' type-id='type-id-5' visibility='default'
filepath='/home/ben/Work/libabigail/test/test2.c' line='1' column='1'/>
+        <var-decl name='c' type-id='type-id-5' visibility='default'
filepath='/home/ben/Work/libabigail/test/test2.c' line='1' column='1'/>
       </data-member>
     </union-decl>
     <type-decl name='int' size-in-bits='32' id='type-id-3'/>
-    <type-decl name='char' size-in-bits='8' id='type-id-4'/>
-    <type-decl name='short int' size-in-bits='16' id='type-id-5'/>
+    <type-decl name='short int' size-in-bits='16' id='type-id-4'/>
+    <type-decl name='char' size-in-bits='8' id='type-id-5'/>
     <function-decl name='test_func' mangled-name='test_func'
filepath='/home/ben/Work/libabigail/test/test2.c' line='3' column='1'
visibility='default' binding='global' size-in-bits='64'
elf-symbol-id='test_func'>
       <parameter type-id='type-id-2' name='var'
filepath='/home/ben/Work/libabigail/test/test2.c' line='3' column='1'/>
       <return type-id='type-id-1'/>

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

[Bug default/24731] abidiff: Incorrect ABI difference reporting for union changing order of members

[dodji at redhat dot com]

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

dodji at redhat dot com changed:

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

--- Comment #3 from dodji at redhat dot com ---
(In reply to Matthias Maennich from comment #0)
> Consider the following reproducer:
> 
> test1.c:
> 
>  union u { int a; char c; short s; };
> 
>  void test_func(union u var) {
>   (void) var;
>  }
> 
> test2.c:
>  
>  union u { int a; short s; char c; };
> 
>  void test_func(union u var) {
>   (void) var;
>  }
> 
> $ gcc -c -g test1.c test2.c && abidiff test1.o test2.o
> 
> Functions changes summary: 0 Removed, 1 Changed, 0 Added function
> Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
> 
> 1 function with some indirect sub-type change:
> 
>   [C]'function void short int(u)' at test2.c:3:1 has some indirect sub-type
> changes:
>     Please note that the exported symbol of this function is test_func
>     'function void short int(u) {test_func}' now becomes 'function void
> test_func(u) {test_func}'
>     parameter 1 of type 'union u' has sub-type changes:
>       type size hasn't changed
>       2 data member changes:
>        type of 'short int u::c' changed:
>          type name changed from 'short int' to 'test_func'
>          type size changed from 8 to 16 (in bits)
> 
>        type of 'short int u::s' changed:
>          type name changed from 'short int' to 'test_func'
>          type size changed from 8 to 16 (in bits)
> 
> 
> abigail incorrectly reports a difference.

With what we have in master[1], I couldn't reproduce the exact same output. 
Here is what I am seeing:

$ gcc -c -g test1.c test2.c && abidiff test1.o test2.o
Functions changes summary: 0 Removed, 1 Changed, 0 Added function
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

1 function with some indirect sub-type change:

  [C]'function void test_func(u)' at test2.c:3:1 has some indirect sub-type
changes:
    parameter 1 of type 'union u' has sub-type changes:
      type size hasn't changed


So, abidiff is trying to show the (harmless) difference but for some reasons
(that I will elide for the sake of brevity) it falls flat trying to do so.  It
should rather show nothing, we do agree.

So to fix the issue I am seeing, I have committed
https://sourceware.org/git/gitweb.cgi?p=libabigail.git;a=commit;h=a11a0068ea333a5149aeed6bd92cb2c6b9523afa.

Now, abidiff should avoid showing any difference by default for that case.

[1]: the master commit I tried to reproduce with is:
bf100fc Fully account for anonymous-ness of scopes when comparing decl names.

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

[Bug default/24731] abidiff: Incorrect ABI difference reporting for union changing order of members

[woodard at redhat dot com]

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

Ben Woodard <woodard at redhat dot com> changed:

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

--- Comment #1 from Ben Woodard <woodard at redhat dot com> ---
This is interesting. It looks like two bugs. One is as you say saying that this
is an ABI impactful change. 

The other is confusing test_func for a type name when it prints: "type name
changed from 'short int' to 'test_func'" I wonder if one causes the other.

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

[Bug default/24731] New: abidiff: Incorrect ABI difference reporting for union changing order of members

[maennich at google dot com]

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

            Bug ID: 24731
           Summary: abidiff: Incorrect ABI difference reporting for union
                    changing order of members
           Product: libabigail
           Version: unspecified
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: default
          Assignee: dodji at redhat dot com
          Reporter: maennich at google dot com
                CC: libabigail at sourceware dot org
  Target Milestone: ---

Consider the following reproducer:

test1.c:

 union u { int a; char c; short s; };

 void test_func(union u var) {
  (void) var;
 }

test2.c:

 union u { int a; short s; char c; };

 void test_func(union u var) {
  (void) var;
 }

$ gcc -c -g test1.c test2.c && abidiff test1.o test2.o

Functions changes summary: 0 Removed, 1 Changed, 0 Added function
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

1 function with some indirect sub-type change:

  [C]'function void short int(u)' at test2.c:3:1 has some indirect sub-type
changes:
    Please note that the exported symbol of this function is test_func
    'function void short int(u) {test_func}' now becomes 'function void
test_func(u) {test_func}'
    parameter 1 of type 'union u' has sub-type changes:
      type size hasn't changed
      2 data member changes:
       type of 'short int u::c' changed:
         type name changed from 'short int' to 'test_func'
         type size changed from 8 to 16 (in bits)

       type of 'short int u::s' changed:
         type name changed from 'short int' to 'test_func'
         type size changed from 8 to 16 (in bits)


abigail incorrectly reports a difference.

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

[Bug default/24731] abidiff: Incorrect ABI difference reporting for union changing order of members

[maennich at google dot com]

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

Matthias Maennich <maennich at google dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |VERIFIED

--- Comment #4 from Matthias Maennich <maennich at google dot com> ---
I confirm the fix working!

Thanks!

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