[Bug default/29144] New: abidiff does not report base class reordering

[Bug default/29144] New: abidiff does not report base class reordering

[gprocida at google dot com]

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

            Bug ID: 29144
           Summary: abidiff does not report base class reordering
           Product: libabigail
           Version: unspecified
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: default
          Assignee: dodji at redhat dot com
          Reporter: gprocida at google dot com
                CC: libabigail at sourceware dot org
  Target Milestone: ---

Hi Dodji.

We noticed this issue when adding support for C++ classes in our own diff tool.

Consider these two source files:

struct A {
  int x;
};

struct B {
  int y;
};

struct C {
  int z;
};

struct D: A, B, C {
  int d;
};

D order;

and:

struct A {
  int x;
};

struct B {
  int y;
};

struct C {
  int z;
};

struct D: B, A, C {
  int d;
};

D order;

The only difference is in D's base class order.

This is an ABI break because the offsets of fields x and y in D are swapped
between the two versions of the code.

The generated XML reflects this correctly and abidiff detects the difference.
However, it does not say what the difference is:

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

1 Changed variable:

  [C] 'D order' was changed at multiple_inheritance_order.1.cpp:17:1:
    type of variable changed:
      type size hasn't changed

and in leaf mode:

Leaf changes summary: 1 artifact changed
Changed leaf types summary: 1 leaf type changed
Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function
Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable

'struct D at multiple_inheritance_order.0.cpp:13:1' changed:
  type size hasn't changed

Regards,
Giuliano.

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

[Bug default/29144] abidiff does not report base class reordering

[dodji at redhat dot com]

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

dodji at redhat dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2022-06-03
     Ever confirmed|0                           |1

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

Re: [Bug default/29144] New: abidiff does not report base class reordering

[Dodji Seketeli]

Thanks for reporting this issue.

I have posted a candidate patch for this issue to
https://sourceware.org/pipermail/libabigail/2022q2/004398.html.

Would that patch address the issue?

-- 
		Dodji

[Bug default/29144] abidiff does not report base class reordering

[dodji at seketeli dot org]

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

--- Comment #1 from dodji at seketeli dot org ---
Thanks for reporting this issue.

I have posted a candidate patch for this issue to
https://sourceware.org/pipermail/libabigail/2022q2/004398.html.

Would that patch address the issue?

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

[Bug default/29144] abidiff does not report base class reordering

[gprocida at google dot com]

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

--- Comment #2 from gprocida at google dot com ---
Hi Dodji.

I think this works, at least to some degree. However, we were wondering if it
would make sense to mention *offsets* rather than just the *order* of base
classes.

There is also a second thing, one which I haven't had time to look into at all,
beyond vaguely remembering the distinction and compiling some code. Something
like the following. Or perhaps with s/class/struct/.

class base { int x; };
class a : base { int y;  };
class b : base { int z; };

class derived : a, b { };

vs

class derived : virtual a, virtual b { };

I wonder if it's possible to get in a situation where layout changes are
missed.

Regards,
Giuliano.

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

[Bug default/29144] abidiff does not report base class reordering

[dodji at redhat dot com]

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

--- Comment #3 from dodji at redhat dot com ---
(In reply to gprocida from comment #2)
> Hi Dodji.
> 
> I think this works, at least to some degree. However, we were wondering if
> it would make sense to mention *offsets* rather than just the *order* of
> base classes.
> 
> There is also a second thing, one which I haven't had time to look into at
> all, beyond vaguely remembering the distinction and compiling some code.
> Something like the following. Or perhaps with s/class/struct/.
> 
> class base { int x; };
> class a : base { int y;  };
> class b : base { int z; };
> 
> class derived : a, b { };
> 
> vs
> 
> class derived : virtual a, virtual b { };
> 
> I wonder if it's possible to get in a situation where layout changes are
> missed.

These are all valid points that I think we should revisit later when we are
more sure about what we want here.  For now, I have applied that patch to
master to at least detect something, rather than nothing.  I'll be glad to
explore this further when we are more sure about what we want.

I'll look at the later case you noted to see if we report anything at all. 
Thanks!

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