[Bug default/27715] New: Fails to detect GObjectClass ABI changes

[Bug default/27715] New: Fails to detect GObjectClass ABI changes

[marcandre.lureau at gmail dot com]

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

            Bug ID: 27715
           Summary: Fails to detect GObjectClass ABI changes
           Product: libabigail
           Version: unspecified
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: default
          Assignee: dodji at redhat dot com
          Reporter: marcandre.lureau at gmail dot com
                CC: libabigail at sourceware dot org
  Target Milestone: ---

GObject (and other similar C object-oriented code) have class structures that
should be checked for ABI breakage. However, abidiff fails to report the
change:


abidiff --headers-dir1 /tmp/old/usr/include/spice-client-glib-2.0
--headers-dir2 /tmp/new/usr/include/spice-client-glib-2.0  --fail-no-debug-info
--no-added-syms                
/tmp/old/usr/lib64/libspice-client-glib-2.0.so.8.7.0
/tmp/new/usr/lib64/libspice-client-glib-2.0.so.8.7.0

diff -u /tmp/old/usr/include/spice-client-glib-2.0/spice-channel.h
/tmp/new/usr/include/spice-client-glib-2.0/spice-channel.h
--- /tmp/old/usr/include/spice-client-glib-2.0/spice-channel.h  2021-04-09
20:40:30.686290157 +0400
+++ /tmp/new/usr/include/spice-client-glib-2.0/spice-channel.h  2021-04-09
20:42:19.934229443 +0400
@@ -106,7 +106,6 @@

     /*< private >*/
     /* virtual method, any context */
-    gpointer deprecated;
     void (*channel_reset)(SpiceChannel *channel, gboolean migrating);
     gpointer deprecated2;


Supposedly more structures/types in the headers should be checked by default
(even if nothing explicitely depend on it).

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

[Bug default/27715] Fails to detect GObjectClass ABI changes

[maennich at android dot com]

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

Matthias Maennich <maennich at android dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2021-04-09
             Status|UNCONFIRMED                 |WAITING
                 CC|                            |maennich at android dot com

--- Comment #1 from Matthias Maennich <maennich at android dot com> ---
Thanks for reporting. Can you please provide a reproducing example? E.g. the
binaries before/after the change or the abi.xml files extracted with abidw from
before/after? Ideal would be some source code that demonstrates the missing
analysis. (you got hold off the relevant object files, maybe?)

In particular, it would be interesting to see how "gpointer deprecated"
participates in the ABI surface of the library.

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

[Bug default/27715] Fails to detect GObjectClass ABI changes

[marcandre.lureau at gmail dot com]

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

--- Comment #2 from Marc-André Lureau <marcandre.lureau at gmail dot com> ---
Reproducer:

https://gitlab.gnome.org/GNOME/glib/

meson setup build && ninja -C build
DESTDIR=/tmp/old ninja -C build install

Modify a base class:

diff --git a/gobject/gtypemodule.h b/gobject/gtypemodule.h
index 5c4025063..f15f18a32 100644
--- a/gobject/gtypemodule.h
+++ b/gobject/gtypemodule.h
@@ -80,7 +80,6 @@ struct _GTypeModuleClass
   void (*reserved1) (void);
   void (*reserved2) (void);
   void (*reserved3) (void);
-  void (*reserved4) (void);
 };


ninja -C build
DESTDIR=/tmp/new ninja -C build install

abidiff --headers-dir1 /tmp/old/usr/include/glib-2.0/gobject --headers-dir2
/tmp/new/usr/include/glib-2.0/gobject/  --fail-no-debug-info --no-added-syms   
             /tmp/old/usr/lib64/libgobject-2.0.so.0.6900.0
/tmp/new/usr/lib64/libgobject-2.0.so.0.6900.0

The starting point is that GTypeModuleClass isn't being used explicitely by
functions. Yet it is the base class / vfunc / private of objects and should
have ABI stability.

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

[Bug default/27715] Fails to detect GObjectClass ABI changes

[maennich at android dot com]

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

--- Comment #3 from Matthias Maennich <maennich at android dot com> ---
I am not familiar with glib and/or meson. Does this build produce debug symbols
in the final binary?

Can you try whether this is reported in the harmless mode (--harmless)?

It would be helpful if you would upload binaries (before/after the change) to
this tracking bug.

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

[Bug default/27715] Fails to detect GObjectClass ABI changes

[marcandre.lureau at gmail dot com]

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

--- Comment #4 from Marc-André Lureau <marcandre.lureau at gmail dot com> ---

> Can you try whether this is reported in the harmless mode (--harmless)?

Yes, no change reported either.

> It would be helpful if you would upload binaries (before/after the change) to this tracking bug.

ok

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

[Bug default/27715] Fails to detect GObjectClass ABI changes

[marcandre.lureau at gmail dot com]

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

--- Comment #5 from Marc-André Lureau <marcandre.lureau at gmail dot com> ---
Created attachment 13363
  --> https://sourceware.org/bugzilla/attachment.cgi?id=13363&action=edit
Old gobject.so

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

[Bug default/27715] Fails to detect GObjectClass ABI changes

[marcandre.lureau at gmail dot com]

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

--- Comment #6 from Marc-André Lureau <marcandre.lureau at gmail dot com> ---
Created attachment 13364
  --> https://sourceware.org/bugzilla/attachment.cgi?id=13364&action=edit
New gobject.so

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

[Bug default/27715] Fails to detect GObjectClass ABI changes

[maennich at android dot com]

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

--- Comment #7 from Matthias Maennich <maennich at android dot com> ---
Thanks for that.

I can see that:
 - _GTypeModuleClass increases in size from old->new in DWARF
 - old and new have debug information sufficient for ABI analysis
 - _GTypeModuleClass is not attached directly or indirectly as a type to any
ELF symbol

Can you please tell me which symbol you would expect to become incompatible
based on a change to this class? How does it relate to any of the exported
symbols?

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

[Bug default/27715] Fails to detect GObjectClass ABI changes

[marcandre.lureau at gmail dot com]

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

--- Comment #8 from Marc-André Lureau <marcandre.lureau at gmail dot com> ---
(In reply to Matthias Maennich from comment #7)
> Thanks for that.
> 
> I can see that:
>  - _GTypeModuleClass increases in size from old->new in DWARF
>  - old and new have debug information sufficient for ABI analysis
>  - _GTypeModuleClass is not attached directly or indirectly as a type to any
> ELF symbol
> 
> Can you please tell me which symbol you would expect to become incompatible
> based on a change to this class? How does it relate to any of the exported
> symbols?

FooClass/vtable (and other object-oriented structures) are often passed around
as void * (or parent types) on the public functions etc. However, the
implementation then cast the void/parent pointer to some concrete type, and
expect the structure to keep the same layout.

So any change in those OO structures is an ABI break, it would be nice to catch
them.

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

[Bug default/27715] Fails to detect GObjectClass ABI changes

[maennich at android dot com]

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

--- Comment #9 from Matthias Maennich <maennich at android dot com> ---
Anything passed to the API via opaque pointers is essentially hidden from ABI
analysis as no type information (other than void*) is attached to it. From an
ABI point of view the ABI of the symbol that passes void* is unchanged. Though
further down this of course can become incompatible.

There are cases that you intentionally want to pass void* to be ABI compatible
even though the type changes. E.g. if your library has such an interface:

void* get_handle(){
  // allocates the resources and returns just a handle to it.
  ...
  return (void*) some_object;
}

void do_something(void* handle) {
  // uses the handle as a reference
}

The ABI is stable even though the type could have changed. The client code
might not even know the concrete type and is also not supposed to cast to it.

So, as much as I would also like to sometimes inspect underlying types of
void*, flagging changes to those types is probably wrong. If you want this type
to be stable, you need to let it participate as a type in the interface.

One way of achieving this is to define a symbol just for the sake of ABI
analysis:

void (struct _GTypeModuleClass obj) { }

This will get picked up by libabigail and the type will participate in the
analysis then.

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

[Bug default/27715] Fails to detect GObjectClass ABI changes

[marcandre.lureau at gmail dot com]

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

--- Comment #10 from Marc-André Lureau <marcandre.lureau at gmail dot com> ---
Isn't the a way to annotate a type in a header for abi check?

That would be useful in other cases, like shared header structures for
protocols and shared structures (common in virtualized hardware for instance)

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

[Bug default/27715] Fails to detect GObjectClass ABI changes

[marcandre.lureau at gmail dot com]

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

--- Comment #11 from Marc-André Lureau <marcandre.lureau at gmail dot com> ---
(In reply to Matthias Maennich from comment #9)
> [..] 
> So, as much as I would also like to sometimes inspect underlying types of
> void*, flagging changes to those types is probably wrong. If you want this
> type to be stable, you need to let it participate as a type in the interface.

This bug is about structures that are exposed in the public header. So they may
be taken as argument, or returned, by void* or parent* pointers, but it would
be nice to annotate the actual type

> One way of achieving this is to define a symbol just for the sake of ABI
> analysis:
> 
> void (struct _GTypeModuleClass obj) { }
> 
> This will get picked up by libabigail and the type will participate in the
> analysis then.

Ok, but I am looking for a more friendly way to express that *all* my types in
my headers must be taken into account without having to define such extra
symbols. Imho it should be an option to the tool somehow.

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