C++ ABI breakage in gpgme-1.17.0?
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	thesamesam
	Feb 14 2022, 10:13 PM

Description

Forwarding a report from downstream Gentoo. A user reported crashes with KDE's KMail in Gentoo and others have noticed the same upstream on KDE's bugzilla.

Running libabigail's abidiff on 1.16.0 vs 1.17.0, I get (truncated output below):

* 20 Removed function symbols not referenced by debug info:
* 
*   [D] _ZN6QGpgME24QGpgMESecretKeyExportJob10slotCancelEv
*   [D] _ZN6QGpgME24QGpgMESecretKeyExportJob10slotStderrEv
*   [D] _ZN6QGpgME24QGpgMESecretKeyExportJob10slotStdoutEv
*   [D] _ZN6QGpgME24QGpgMESecretKeyExportJob11qt_metacallEN11QMetaObject4CallEiPPv
*   [D] _ZN6QGpgME24QGpgMESecretKeyExportJob11qt_metacastEPKc
*   [D] _ZN6QGpgME24QGpgMESecretKeyExportJob17slotProcessExitedEiN8QProcess10ExitStatusE
*   [D] _ZN6QGpgME24QGpgMESecretKeyExportJob5startERK11QStringList
*   [D] _ZN6QGpgME24QGpgMESecretKeyExportJobC1EbRK7QString, aliases _ZN6QGpgME24QGpgMESecretKeyExportJobC2EbRK7QString
*   [D] _ZN6QGpgME24QGpgMESecretKeyExportJobC2EbRK7QString
*   [D] _ZN6QGpgME24QGpgMESecretKeyExportJobD0Ev
*   [D] _ZN6QGpgME24QGpgMESecretKeyExportJobD1Ev, aliases _ZN6QGpgME24QGpgMESecretKeyExportJobD2Ev
*   [D] _ZN6QGpgME24QGpgMESecretKeyExportJobD2Ev
*   [D] _ZN9QtPrivate11QSlotObjectIMN6QGpgME24QGpgMESecretKeyExportJobEFvvENS_4ListIJEEEvE4implEiPNS_15QSlotObjectBaseEP7QObjectPPvPb
*   [D] _ZNK6QGpgME24QGpgMESecretKeyExportJob10metaObjectEv
*   [D] _ZNSt11_Tuple_implILm1EJN5GpgME3KeyE9QDateTimeSt6vectorINS0_6SubkeyESaIS4_EEEED1Ev, aliases _ZNSt11_Tuple_implILm1EJN5GpgME3KeyE9QDateTimeSt6vectorINS0_6SubkeyESaIS4_EEEED2Ev
*   [D] _ZNSt11_Tuple_implILm1EJN5GpgME3KeyE9QDateTimeSt6vectorINS0_6SubkeyESaIS4_EEEED2Ev
*   [D] _ZNSt17_Function_handlerIFSt5tupleIJN5GpgME12ImportResultE7QStringNS1_5ErrorEEEvESt5_BindIFS7_IFPFS5_PNS1_7ContextERK10QByteArrayESt12_PlaceholderILi1EESA_EES9_EEE10_M_managerERSt9_Any_dataRKSM_St18_Manager_operation
*   [D] _ZNSt17_Function_handlerIFSt5tupleIJN5GpgME12ImportResultE7QStringNS1_5ErrorEEEvESt5_BindIFS7_IFPFS5_PNS1_7ContextERK10QByteArrayESt12_PlaceholderILi1EESA_EES9_EEE9_M_invokeERKSt9_Any_data
*   [D] _ZNSt17_Function_handlerIFSt5tupleIJN5GpgME5ErrorE7QStringS2_EEvESt5_BindIFS6_IFPFS4_PNS1_7ContextERKNS1_3KeyERK9QDateTimeRKSt6vectorINS1_6SubkeyESaISG_EEESt12_PlaceholderILi1EES9_SC_SI_EES8_EEE10_M_managerERSt9_Any_dataRKSU_St18_Manager_operation
*   [D] _ZNSt17_Function_handlerIFSt5tupleIJN5GpgME5ErrorE7QStringS2_EEvESt5_BindIFS6_IFPFS4_PNS1_7ContextERKNS1_3KeyERK9QDateTimeRKSt6vectorINS1_6SubkeyESaISG_EEESt12_PlaceholderILi1EES9_SC_SI_EES8_EEE9_M_invokeERKSt9_Any_data

Note that in the KDE upstream bug linked, folks mentioned that rebuilding consumers against new gpgme (1.17.0) fixes their issues.

Details

External Link: https://bugs.kde.org/show_bug.cgi?id=449891
Version: 1.17.0

Revisions and Commits

rM GPGME
	rMad3aabdd8a64 qt: Fix ABI compatibility with 1.16.0

Related Objects

Mentioned In: T5871: GPGME: New Qt util.cpp (1.17.0) FTBFS for i386 only
T5872: Release GPGME 1.17.1

Event Timeline

thesamesam created this task.Feb 14 2022, 10:13 PM

thesamesam set External Link to https://bugs.kde.org/show_bug.cgi?id=449891.

I assumed that changes to internal classes wouldn't break the ABI, but apparently the symbols were still exported. I'll keep this in mind for the next release.

FWIW, the internal class in question was completely rewritten. Since the damage has been done already, I'll close this report. We won't readd symbols to dead code. Sorry, for the inconvenience.

In T5834#154975, @ikloecker wrote:

I assumed that changes to internal classes wouldn't break the ABI, but apparently the symbols were still exported. I'll keep this in mind for the next release.

FWIW, the internal class in question was completely rewritten. Since the damage has been done already, I'll close this report. We won't readd symbols to dead code. Sorry, for the inconvenience.

That's no bother. I wouldn't expect you to re-add the symbols. Can I ask if you'd consider bumping SONAME in a 1.17.1 release then please? It'd help downstreams considerably.

Sure. We'll bump the SONAME.

Why can't we hide internal symbols in c++ as we are doing in other libs for ages? Were the internal symbols only accidentally exposed?

The actual problem isn't the removed internal symbols, but

'method virtual QGpgME::KeyForMailboxJob* QGpgME::Protocol::keyForMailboxJob() const' has some sub-type changes:
  the vtable offset of method virtual QGpgME::KeyForMailboxJob* QGpgME::Protocol::keyForMailboxJob() const changed from 28 to 31
    note that this is an ABI incompatible change to the vtable of class QGpgME::Protocol

KMail calls keyForMailboxJob(), but because of the changed index in the vtable it called addUserIDJob() which ultimately caused the crash.

@werner Please release a gpgme-1.17.1 with

diff --git a/configure.ac b/configure.ac
index f6d4b50e..57e6ea2e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -64,8 +64,8 @@ LIBGPGMEPP_LT_CURRENT=20
 LIBGPGMEPP_LT_AGE=14
 LIBGPGMEPP_LT_REVISION=0
 
-LIBQGPGME_LT_CURRENT=14
-LIBQGPGME_LT_AGE=7
+LIBQGPGME_LT_CURRENT=8
+LIBQGPGME_LT_AGE=0
 LIBQGPGME_LT_REVISION=0
 ################################################

to bump the SONAME of libqgpgme from libqgpgme.so.7 to libqgpgme.so.8 or with

diff --git a/configure.ac b/configure.ac
index f6d4b50e..57e6ea2e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -64,8 +64,8 @@ LIBGPGMEPP_LT_CURRENT=20
 LIBGPGMEPP_LT_AGE=14
 LIBGPGMEPP_LT_REVISION=0
 
-LIBQGPGME_LT_CURRENT=14
-LIBQGPGME_LT_AGE=7
+LIBQGPGME_LT_CURRENT=15
+LIBQGPGME_LT_AGE=0
 LIBQGPGME_LT_REVISION=0
 ################################################

for strictly increasing LIBQGPGME_LT_CURRENT.

Luckily, so far there weren't any commit to gpgme since the release of 1.17.0.

• ikloecker triaged this task as High priority.Feb 16 2022, 7:26 PM

Yeah, please do issue a new release as soon as possible if you can, as otherwise downstream we're in an awkward position where we have to rebuild everything without a SONAME bump, then do it again once the release is out.

i.e. if such a release is going to happen, it should happen without any additional non-SONAME changes, and without delay, because otherwise we end up in the worst of both worlds.

Thanks for your understanding both!

As for symbols: right, I only quoted some of the abidiff output. KDE has quite a nice summary of C++ ABI footguns. I think removed symbols, even if unused by applications, can affect layout, and hence ABI, but it'd be good to verify that only intended symbols are even exported too.

(Aside: I know some projects like OpenZFS include abidiff into their CI / pre-release process. Might be worth looking into somehow.)

• ikloecker added a commit: rMad3aabdd8a64: qt: Fix ABI compatibility with 1.16.0.Feb 17 2022, 10:29 AM

• werner mentioned this in T5872: Release GPGME 1.17.1.Mar 6 2022, 5:47 PM

Fixed in 1.17.1

• werner mentioned this in T5871: GPGME: New Qt util.cpp (1.17.0) FTBFS for i386 only.Mar 6 2022, 6:19 PM