Clang in Fedora 24

I’m only ever using a bleeding edge Clang trunk build myself. So when Fedora enabled use of the GCC abi_tag attribute in libstdc++ (i.e., switched the _GLIBCXX_USE_CXX11_ABI macro to 1 by default), it was only a matter of applying a set of (evolving) patches to Clang to keep things working.

However, others who wanted to use the Clang version included in Fedora were in for more of a problem. The typical symptom being that what you compiled with Clang fails to link against system libraries, most notably libstdc++.

For Fedora 23, we included a somewhat slapdash early version of the relevant Clang patches, but that didn’t survive the split-out of Clang from the overall LLVM package in Fedora 24. But now that the Clang upstream patches are deemed stable (and have been integrated into trunk towards Clang 3.9), I backported them to the Clang 3.8 in Fedora 24 (and rawhide).

And a lot of thanks to all of those who created these patches!

Plugin, Flamed

Just how much is our Clang plugin slowing down builds of LibreOffice? We are constantly adding more and more useful functionality to it, and some of it is well known to be written in a way that is likely performing not too well. But I never dared look at the performance implications too closely, for fear of being disappointed. The build always seemed to go along at more or less the same speed, so who cares for details?

Enter FlameGraph, generating beautiful and easy-to-digest graphs on top of the Linux perf tool’s (or others’) output. So I gathered data about half a minute of CPU activity during the LibreOffice build (randomly picked a time when all the cores were busy compiling .cxx files in the sw module, to keep noise contributed by other processes down to a minimum), and passed it through FlameGraph. And sobering set in:flamegraph98.56% of all the time was spent in the clang-3.9 process (i.e., there was almost no noise from unrelated processes). And 19.22% of all time was spent in our plugin. That is, the plugin slows down compilation by about 25%. Quite a number.

Zooming in and breaking this down further per individual plugin, the distribution is

  • 1.47% CommaOperator
  • 1.04% VCLWidgets
  • 0.91% BodyNotInBlock
  • 0.79% SalBool
  • 0.69% PtrVector
  • 0.65% RefCounting
  • 0.63% FailedDynCast
  • 0.62% ReservedId
  • 0.61% Nullptr
  • 0.59% RedundantCast
  • 0.56% StringConstant
  • 0.56% ImplicitBoolConversion
  • 0.55% ExternAndNotDefined
  • 0.50% PassStuffByRef
  • 0.47% DefaultParams
  • 0.46% StringConcat
  • 0.45% UnusedVariableCheck
  • 0.45% SalLogAreas
  • 0.45% LiteralToBoolConversion
  • 0.44% UnrefFun
  • 0.44% ReturnByRef
  • 0.43% StaticCall
  • 0.43% BadStatics
  • 0.42% Override
  • 0.41% StaticAnonymous
  • 0.41% InlineVisible
  • 0.38% SimplifyBool
  • 0.38% FpComparison
  • 0.37% SfxPoolItem
  • 0.35% StaticMethods
  • 0.35% PrivateBase
  • 0.35% BadVectorInit
  • 0.34% DerefNullPtr
  • 0.34% CStyleCast
  • 0.32% RangeForCopy
  • 0.32% LoopVarTooSmall
  • 0.29% OnceVar

The recursive nature of RecursiveASTVisitor makes it a bit hard to easily spot each individual plugin’s hotspots, but one thing sticks out: CommaOperator is a rather simple piece of code (that shouldn’t need much heavy computation), but does use lopluign::Pluign::parentStmt, which rebuilds the AST to determine a node’s parent, and is really, really expensive.

Room for improvement. Time to clean up.

(At first, the generated graph looked way less interesting. One thing that was necessary was to build both LLVM/Clang and our plugin with -fno-omit-frame-pointer. The other thing was to teach FlameGraph to support “(anonymous namespace)” in C++ function names.)

LibreOffice 5.2 Beta Flatpak

Flatpak (formerly known under its working title “xdg-app”) is a cool new way of distributing images of Linux applications that run on a wide range of different distros, and run there in a secure way. I have blogged and spoken about making LibreOffice available in this format before in various places (see, e.g., these two previous blog posts and this conference talk).

Now is the time to announce availability of a LibreOffice.flatpak bundle of LibreOffice 5.2 Beta at download.documentfoundation.org/libreoffice/flatpak/5.2.0/LibreOffice.flatpak!

To try it out, do the following:

  • First, you need to install Flatpak itself. See here for details.
  • Second, you need to add the Flatpak repository that contains the GNOME runtime (on which the LibreOffice.flatpak depends; see here for details):
      $ wget https://sdk.gnome.org/keys/gnome-sdk.gpg
      $ flatpak remote-add --user --gpg-import=gnome-sdk.gpg gnome https://sdk.gnome.org/repo/
      $ flatpak install --user gnome org.gnome.Platform 3.20

    Update: If you use another locale than en-US, and want LibreOffice to automatically come up with a matching UI localization, additionally

      $ flatpak install --user gnome org.gnome.Platform.Locale 3.20
  • Then, with the LibreOffice.flatpak file downloaded from the link above, do
      $ flatpak install --user --bundle LibreOffice.flatpak

    to install it. (Don’t worry, it does not interfere with any other LibreOffice you might already have installed on your machine, like the one shipped with your Linux distro.)

  • And now you can run it, either from the command line
      $ flatpak run org.libreoffice.LibreOffice

    or by clicking on one of its icons (LibreOffice Start Center, Writer, etc.) in your distro’s application launcher.

(And if you happen to have an older version of xdg-app installed, installing the LibreOffice.flatpak bundle into it should work, too.)

Behind the scenes, the LibreOffice.flatpak links to a repository on the download.documentfoundation.org server that will get updated with later versions of LibreOffice Fresh. So once a new version of LibreOffice Fresh is released (starting with LibreOffice 5.2 proper), you can simply update your installation with

  $ flatpak update --user org.libreoffice.LibreOffice

A few items to note:

  • LibreOffice.flatpak is based on the GNOME 3.20 runtime, using LibreOffice’s much-improved GTK3 backend. This should offer the most complete set of features in LibreOffice, paving the way to Wayland etc.
  • The single bundle contains all localizations available for LibreOffice (as it doesn’t really affect the bundle’s overall size that much).
  • Flatpak-based apps are not yet able to reach out to other applications installed on the machine (like browsers) to e.g. open URLs through them. That means that e.g. clicking on a hyperlink in a LibreOffice Writer document might not work yet. And since the LibreOffice.flatpak does not contain all the help documentation (for all the localizations!), but instead wants to use the online help through a browser, pressing F1 does not yet work, either.
  • The LibreOffice.flatpak does not include a Java Runtime Environment (JRE). That means that some LibreOffice functionality that requires a JRE will not work.
  • At this time, there is no Flatpak version of the LibreOffice Software Development Kit (SDK).

LibreOffice.flatpak

Flatpak is the new name of xdg-app. So following up on my previous LibreOffice in a Box post, here is some more technical detail on how an upcoming LibreOffice 5.2 will be available as a Flatpak bundle.

There is now a shell script demonstrating how to do all the downloading of sources, building, and bundling up. I decided against using flatpak-builder (nee xdg-app-builder) for that mostly out of convenience, mainly because the way LibreOffice fetches its external dependencies is so different from the typical way of building a Linux app. So I felt more comfortable doing these things manually, calling the raw xdg-app build steps from a script. But given enough pressure (e.g., to make the LibreOfficeKit widget buried within the LibreOffice installation set available to other Flatpak apps), this might get revisited.

The script proceeds in six steps (see the above link for details):

First, building LibreOffice requires an Archive-Zip Perl module not found in the org.gnome.Sdk we build against. The easiest solution (easier than to try and get rid of that Perl dependency in the LibreOffice build machinery, that is) is to download it and make it available to the build via the PERLLIB environment variable.

Second, we need the LibreOffice sources themselves. The current master branch has all the necessary changes needed for a Flatpak’ed LibreOffice (as will the coming libreoffice-5-2 branch).

Third, we need to fetch LibreOffice’s external dependencies. LibreOffice has a long list of external projects that it can either use from the underlying system or build/bundle itself. Those external projects covered by Flatpak’s org.gnome.Platform runtime are taken from there, but whatever remains is bundled with the LibreOffice app. The sources for these external projects are normally downloaded during the build, but an xdg-app build does not have network connectivity, so they need to be downloaded upfront. So bootstrap enough of a LibreOffice build here to execute the make fetch part.

Fourth, LibreOffice is built in xdg-app. The choice of building against the org.gnome.Platform 3.20 is somewhat arbitrarty, but it’s the latest revision available. And given LibreOffice’s strong GTK3 support (which e.g. also enables running on Wayland), this should make the resulting app more versatile than basing it on the more fundamental org.freedesktop.Platform. Building LibreOffice doesn’t exactly follow the make && make install mantra, but there happened to already be a make distro-pack-install target that produces the build artifacts in (almost) the right way for our purposes here.

Fifth, the build artifacts from the previous step are assembled in the way xdg-app expects them. Some of the metadata desktop and icon files need to be renamed, to match xdg-app’s expectation that metadata files belonging to the org.libreoffice.LibreOffice app have names actually starting with “org.libreoffice.LibreOffice”.

Finally, everything is bundled up into a LibreOffice.flatpak that can be distributed to users, who can then install it with xdg-app install --bundle LibreOffice.flatpak.

The bundle contains all the available LibreOffice localizations. It doesn’t make that much of a difference for the size of the resulting LibreOffice.flatpak, and should be easier to handle for both producers and consumers than to provide dedicated single-language apps. For reasons of size, the help content is not bundled, though; the intent is to (transparently) use the online help content. However, one caveat with the current state of xdg-app is that an app cannot reach out to other applications on the machine via xdg-open (a portal will be needed for that). When asked to process “external” URLs (e.g., when clicking on a hyperlink in a text document, but also when accessing the online help), LibreOffice tries to pass those URLs to xdg-open, but that doesn’t work yet in xdg-app.

Between an assert and a [[fallthrough]]

LibreOffice now has Clang -Wimplicit-fallthrough enabled to catch places where a break has been missing between cases in a switch statement. See “Re: clang -Wimplicit-fallthrough and missing breaks” for some further details.

Of course, a code base as large as LibreOffice has quite a number of places where such fall-through from one case to the next is intentional, and where warnings thus need to be suppressed. The solution is the [[clang::fallthrough]] annotation (which will be available as plain [[fallthrough]] in C++17), wrapped up as a SAL_FALLTHROUGH macro for now (so it doesn’t confuse other compilers).

All fine and dandy.

Until you realise that there is quite a number of places like the following in the code base:

Foo * p;
switch (kind) {
default:
    assert(false); // cannot happen
case Kind1:
    p = new Foo(1);
    break;
case Kind2:
    p = new Foo(2);
    break;
case Kind3:
    p = new Foo(3);
    break;
};
p->foo();

Maybe kind is of an enumeration type that can only legitimately take on one of the values Kind1Kind3 (see “”enum union” with set of values restricted to exactly its enumerators?” for a discussion how such a type cannot reasonably be modeled in C++), or it is a type that can take on a wider set of values, but some invariant guarantee is that it will only take on values Kind1Kind3 here. Regardless, some compilers will warn about unhandled cases, and/or about p being used uninitialized in some cases. So the default assert part got added at some point in time, logcially marking any unhandled cases as unreachable. If a compiler/NDEBUG-configuration combo is smart enough to conclude that the assert is non-returning, it will not emit a warning. If, on the other hand, the compiler/NDEBUG-configuration combo assumes fall-through to case Kind1, it will not emit a warning about p being used uninitialized, either.

All fine and dandy.

Except that, in the NDEBUG case, Clang will now warn about a missing [[fallthrough]] annotation between the default branch and case Kind1. Under non-NDEBUG configuration (i.e., assert actually expands to some code), it is smart enough to conclude that the assert(false) will never return, so it concludes that there will be no fall-through, so will not warn. But under NDEBUG configuration (i.e., the assert expands to nothing), it sees an empty statement in the default branch, causing fall-through, causing a -Wimplicit-fallthrough warning.

But when you add a [[fallthrough]] annotation to appease the NDEBUG configuration, the non-NDEBUG configuration will start to complain about an unreachable [[fallthrough]] annotation (as it considers the assert to be non-returning, see above).

Caught between a rock and a hard place.

One way out is to rewrite the code as follows:

Foo * p;
switch (kind) {
case Kind1:
    p = new Foo(1);
    break;
case Kind2:
    p = new Foo(2);
    break;
case Kind3:
    p = new Foo(3);
    break;
default:
    for (;;) std::abort(); // cannot happen
};
p->foo();

The std::abort marks the default branch as dead independent of NDEBUG. The infinite loop around it is there to make even the most lame compiler aware that that statement will not return, so it will refrain from emitting a warning about p being used uninitialized. (And the default branch can now go at the end, where it feels more naturally at home.)

And no, there should be no fears that the second variant will behave differently now in production NDEBUG builds. In the first variant, should kind ever take on an invalid value, it would silently treat it as Kind1, proceeding (though, of course, proceeding in the most unfortunate of “defensive programming” ways), not (immediately) crashing in the user’s face. In the second variant, it would abort outright (“crashing in the user’s face!”, one might fear). But that’s a red herring. The assert (and, similarly, the std::abort) codifies the invariant that kind must be one of Kind1Kind3 here. So if it ever did have a different value, all bets about the state of the program would be off, anyway. And it would better fail fast.

libc++/libc++abi on Linux

LibreOffice bug report “Build failure with clang++/libc++ on Linux x86_64” and reports about a similar failure when trying to build on NetBSD made me look at building LibreOffice on Linux with Clang against libc++/libc++abi (instead of the usual libstdc++) myself.

The libc++ web site warns: “Unfortunately you can’t simply run clang with ‘-stdlib=libc++’ at this point [when you have built Clang to be able to use libc++ in combination with libc++abi], as clang is set up to link for libc++ linked to libsupc++.” But that appears to no longer be true (at least on current LLVM trunk). All it takes is to include the libcxx and libcxxabi repositories in the projects sub-directory of the LLVM/Clang source tree and build it. The resulting libc++.so includes the necessary -lc++abi, so building LibreOffice with CXX=clang++ -stdlib=libc++ just works. (You need to make sure that the libc++.so.1 gets found at runtime, of course, e.g., by setting LD_LIBRARY_PATH.)

One remaining problem is that libc++abi and libstdc++ (resp. its ABI-related libsupc++ part) differ in how they determine type equivalence. On the one hand, libc++abi sticks to the original C++ ABI semantics: “It is intended that two type_info pointers point to equivalent type descriptions if and only if the pointers are equal.” (That is, type equivalence is determined by comparing _ZTI* type_info pointers for equality.) On the other hand, libstdc++ relaxed this quite a while ago to comparing type_infos’ name fields (_ZTS*) for string equality (via strcmp).

While both approaches have pros and cons, they do lead to different behaviour. One example is when a dynamic library is dlopen’ed RTLD_LOCAL. If code in that library (or any library that gets loaded dependent of it) throws an exception of type T, and code beyond that RTLD_LOCAL “wall” shall catch it, then that will work with the libstdc++ approach but fail with the libc++abi approach (even if all the relevant _ZTI*, _ZTS* symbols are weakly exported).

That is the reason why e.g. the UNO runtime makes sure to dlopen all relevant libraries as RTLD_GLOBAL, but LibreOfficeKit/LibreOfficeKitInit.h somewhat ignorantly tries to get away with dlopen’ing the sofficeapp library as RTLD_LOCAL. Now LibreOffice’s cppunittester executable (used to run CppUnit tests during the LibreOffice build) links against the sal library. So by the time CppunitTest_libreofficekit_tiledrendering dlopen’s the sofficeapp library as RTLD_LOCAL, the sal library has already been loaded into the process (implicitly as RTLD_GLOBAL), so any UNO components that happen to get instantiated in that process (via the dlopen in sal’s osl_loadModule) will be outside that “sofficeapp RTLD_LOCAL walled garden,” so that when code in ucb/source/ucp/file/bc.cxx throws a css::ucb::CommandFailedException, code in ucbhelper/source/client/content.cxx (inside the walled garden) won’t catch it (but would instead proceed to std::unexpected, std::terminate, std::abort).

That problematic dlopen with RTLD_LOCAL had already been addressed for UBSan builds (where runtime type identification code in at least Clang’s UBSan implementation always operates under the original C++ ABI semantics, regardless of compiler ABI library used), “Ensure RTTI symbol visibility for Linux Clang -fsanitize=function,vptr.” The only problem this time is to find a suitable #if condition to distinguish usage of libc++abi from usage of other compiler ABI libraries, “[cfe-dev] Feature-test macro whether compiling against libc++abi?”

GCC 6

Some notes on building LibreOffice master (towards LO 5.2) with recent GCC trunk (towards GCC 6.0):

  • I ran into two open bugs with GCC that for now need workaround patches to be applied to LO:
  • (There was also another test breaker that took a while to track down as “Make sure desktop under LOK does not see osl_setCommandArgs CommandLineArgs.” But that one was not caused by any sort of mis-compilation, but rather by the fact that LO, when mistakenly asked to open a dynamic library as a document, happens to auto-detect an ELF library built by GCC 6 as a MacPaint document, while it auto-detected such libraries built with older GCC versions as plain text Writer documents.)
  • All the other (minor) fixes and tweaks have gone into the LO code base by now, and make check succeeded for me for both an (implicitly) --disable-debug as well as an --enable-dbgutil build (on some random Fedora 22 Linux box).
  • (With the caveat of having to --disable-firebird-sdbc in the --disable-debug case due to some error during building of that notorious external module,
    make -f ../gen/Makefile.refDatabases empty_db
    make[4]: Entering directory 'lo/core/workdir/UnpackedTarball/firebird/gen'
    make -f ../gen/Makefile.static.createdb
    make[5]: Entering directory 'lo/core/workdir/UnpackedTarball/firebird/gen'
    make[5]: Nothing to be done for 'all'.
    make[5]: Leaving directory 'lo/core/workdir/UnpackedTarball/firebird/gen'
    rm -f empty.fdb
    ../gen/firebird/bin/create_db empty.fdb
    invalid request BLR at offset 24
    -Too many Contexts of Relation/Procedure/Views. Maximum allowed is 255
    ../gen/Makefile.refDatabases:66: recipe for target 'empty.fdb' failed
    make[4]: *** [empty.fdb] Error 254
    make[4]: Leaving directory 'lo/core/workdir/UnpackedTarball/firebird/gen'
    Makefile:275: recipe for target 'empty_db' failed
    make[3]: *** [empty_db] Error 2
    make[3]: Leaving directory 'lo/core/workdir/UnpackedTarball/firebird/gen'
    Makefile:6: recipe for target 'firebird_embedded' failed
    make[2]: *** [firebird_embedded] Error 2
    make[2]: Leaving directory 'lo/core/workdir/UnpackedTarball/firebird'
    lo/core/external/firebird/ExternalProject_firebird.mk:29: recipe for target 'lo/core/workdir/ExternalProject/firebird/build' failed
    make[1]: *** [lo/core/workdir/ExternalProject/firebird/build] Error 1
    

    which I have not been able to track down yet.)

  • The most common new warning encountered was -Werror=misleading-indentation. We already have a Clang loplugin:bodynotinblock (“compiler plugin check for if/while/true bodies with possibly {} missing”), so this new GCC warning did not find any more errors, but it does warn about cases where an additional statement follows a complete if statement on the same line, like in “-Werror=misleading-indentation (GCC 6).”
  • The diagnostic information presented by the compiler when it finds an error in the code has improved still with GCC 6, but unfortunately the most dreaded
    lo/core/sw/source/core/text/txtfrm.cxx: In member function ‘virtual bool SwTextFrame::Prepare(PrepareHint, const void*, bool)’:
    lo/core/sw/source/core/text/txtfrm.cxx:689:0: error: assuming signed overflow does not occur when assuming that (X + c) < X is always false [-Werror=strict-overflow]
          if( nLen != COMPLETE_STRING && GetOfst() > nPos + nLen ) // the range preceded us
    

    emitted when building with optimizations enabled still leaves you completely clueless about what is going on, like in “Silence some odd -Werror=strict-overflow (GCC 6).”

  • (And PR66460 “ICE using __func__ in constexpr function” is still open, so we still have the somewhat unfortunate situation that we detect HAVE_CXX14_CONSTEXPR=1 in the --disable-debug case, but HAVE_CXX14_CONSTEXPR=0 in the --enable-debug case—or its --enable-dbgutil superset, or its --enable-assert-always-abort subset.)

(Update 2016-02-25: Current LibreOffice master builds fine now with recent GCC trunk rev. 233691 out of the box; no more workarounds needed on either side.)