Uploaded image for project: 'libmusicbrainz'
  1. libmusicbrainz
  2. LMB-43

Fix visibility of exceptions from HTTPFetch

    • Icon: Bug Bug
    • Resolution: Unresolved
    • Icon: Normal Normal
    • None
    • 5.1.0
    • None
    • FreeBSD / clang

      This issue shows up in particular in the FreeBSD packaging of KDE Applications that use libkcddb, which in turn uses libmusicbrainz5 for CD lookup. Applications that look up a CD where the lookup returns an error (e.g. connection problem, or unknown CD), crash with an unhandled exception. The code structures are roughly this:

      • libmusicbrainz5 throws a CExceptionBase subclass (from HTTPFetch.c; classes defined in HTTPFetch.h),
      • libkcddb catches these exceptions (e.g. CConnectionError and other CExceptionBase subclasses) and returns an error code instead (the bug is that this doesn't happen and the exception is propagated)
      • application interprets error code (but instead gets an unexpected exception)

      The libkcddb code can be seen at https://github.com/KDE/libkcddb/blob/master/libkcddb/musicbrainz/musicbrainzlookup.cpp#L177 .

      The problem shows up when symbol visibility is thrown into the mix. With -fvisibility=hidden, at least with clang++ (FreeBSD doesn't use g++), libkcddb doesn't get the symbols from libmusicbrainz5. The catch statements in the code linked above do not actually catch the CExceptionBase-derived exceptions. At best, libkcddb could catch std::exception – but it doesn't, and so the exception propagates out to the calling application, which doesn't catch either, and the whole thing crashes.

      This can be solved in several places: not using -fvisibility=hidden when building libkcddb; catching std::exception and ignoring the CExceptionBase subclasses; ensuring that the C++ symbols for the exception classes are visible even with -fvisibility=hidden.

      The first is undesirable because of the performance benefit (runtime-linking-wise) of -fvisibility=hidden in general with C++; the second messes around with application logic and drops the distinction between the various errors that libmusicbrainz5 throws; that leaves the third approach.

      The gcc wiki has a good explanation at https://gcc.gnu.org/wiki/Visibility .

      The attached patch, which takes the visibility-declarations from the gcc wiki, ensures that the C++ symbols for the exceptions thrown by libmusicbrainz5 are always visible, so that libraries calling in to libmusicbrainz5 can reliably catch those exceptions.

      The patch as attached drops Windows support (not relevant on FreeBSD) and adds clang; doing a cut-and-paste again from the gcc wiki and adding clang alongside _GNUC_ > 4 might be better (if Windows is also on your list of target platforms).

            adhawkins Andy Hawkins
            adridg Adriaan de Groot
            Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

              Created:
              Updated:

                Version Package