https://bugs.kde.org/show_bug.cgi?id=387820

            Bug ID: 387820
           Summary: KConfigGui misbehave at runtime when link time
                    optimizations are enabled
           Product: frameworks-kconfig
           Version: unspecified
          Platform: Other
                OS: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: NOR
         Component: general
          Assignee: matt...@mjdsystems.ca
          Reporter: elv1...@gmail.com
                CC: kdelibs-b...@kde.org
  Target Milestone: ---

## Context

In the past few months, I have been fixing all frameworks to support static
linking, link time optimizations and profile guided optimizations. So far the
patches are a little crude and are not in mergeable shape. Since yesterday I
cleaned up a few of them to begin the upstreaming process. There's already an
elv13/fix_static branch for 2 frameworks to discuss the changes. LTO and static
mode enables KDE apps to be built in a single binary that's about ~10mb for
most applications, 20mb for big ones due to the assets, making it small enough
for even low end mobile devices. It's also starting much faster thanks to the
PGOs.

Most frameworks "work" after some boring build system patches such as fixing
common "there is a cmake macro/variable for that, don't write your own code"
type of bugs. There's also the need to make QtTest optional everywhere to avoid
having hundreds of gigabytes worth of tests being built (and it takes a
looooong time to link). However some frameworks are really broken such as the
ones with plugins and those abusing of undefined behavior or static
initialization behavior, such as KConfigGui.

## The bug

This warning is hit and kcolorsheme fail to load, rendering all applications
black on black (invalid QColor).

 372     case QMetaType::QColor:
 373     case QMetaType::QFont:
 374         qWarning() << "KConfigGroup::readEntry was passed GUI type '"
 375                    << aDefault.typeName()
 376                    << "' but KConfigGui isn't linked! If it is linked to
your program, "
 377                    "this is a platform bug. Please inform the KDE
developers";
 378         break;

This is due to several C++ broken features used together.

1) The visitor KConfig uses to have types provided by KConfigGui and not
KConfigCore is using static initialization in the form of:

193 static int initKConfigGroupGui()
194 {
195     _kde_internal_KConfigGroupGui.readEntryGui = readEntryGui;
196     _kde_internal_KConfigGroupGui.writeEntryGui = writeEntryGui;
197     return 42;                  // because 42 is nicer than 1 or 0
198 }
199 
200 #ifdef Q_CONSTRUCTOR_FUNCTION
201 Q_CONSTRUCTOR_FUNCTION(initKConfigGroupGui)
202 #else
203 static int dummyKConfigGroupGui = initKConfigGroupGui();
204 #endif

In a CPP file. This is problematic for several reasons:

1) Static initialization order is undefined behavior and changes from compilers
to compilers and the level of optimizations used. Having fixed "most" common
scenarios with "most" common compilers does not make it predictable. In this
case it isn't called at all (and this is actually valid, not a bug due to point
1, 2 and 3 being combined)

2) This is implemented as a `static` variable in a `cpp` file. Static elements
in a compilation units are never exported (as defined in the C standard). Even
adding __attribute(used), volatile or an EXPORT macro in front wont export it.
The compiler is free to apply dead code elimination on these variables if it
thinks they are unused or used in UB ways only.

3) The macro is in a cpp file with no header and no exported symbols at all.
The link time optimizer uses a reacheability tree purge unused compilation
units and sections. There is nothing at all pointing to this CU, it's
eliminated from the final executable in GCC and probably all other compilers.

## Is it a compiler bug?

No. It is a certain interpretation of the standard where GCC takes some extra
liberty to purge code even if it does "something". It probably fail to resolve
that the code actually affects static initialization of other CUs. Given that's
undefined anyway, then so be it.

## How can it be fixed

My crude hack is to put `int dummyKConfigGroupGui` in a dummy class and
exporting it then print dummyKConfigGroupGui from main.cpp. It fixes the
framework and applications are no longer black on black. Obviously it's a bad
hack.

The proper way to fix this would be to share createColorEntry in a private
KConfigGui header and turn the int into a `static std::atomic_flag
KCongigGui::init_flag {ATOMIC_FLAG_INIT};`. Then, in KConfigSkeleton
constructor, check the flag and initialize it if it's not already initialized.

This would provide an internal reachability path for `dummyKConfigGroupGui`
from the outside of `src/gui/kconfiggroupgui.cpp` and avoid it from being
dead-code-eliminated by the compiler.

## tl;dr

Q_CONSTRUCTOR_FUNCTION can't be used in a .cpp file that exports nothing at
all. I don't think there's anything that can be done to fix the macro, the
compiler will skip it anyway.

-- 
You are receiving this mail because:
You are watching all bug changes.

Reply via email to