Bug 163897 – [CMake] RelWithDebInfo builds are super broken at runtime

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Michael Catanzaro]

Several Mageia users are complaining that Evolution is not displaying mails properly. For these users, it seems Epiphany is broken in the same manner.

Please see:

Screenshot of bug in Evolution: https://bug772692.bugzilla-attachments.gnome.org/attachment.cgi?id=337312
Screenshot of bug in Epiphany: https://bug772692.bugzilla-attachments.gnome.org/attachment.cgi?id=337574
glxinfo of first affected user: https://bug772692.bugzilla-attachments.gnome.org/attachment.cgi?id=338287
glxinfo of second affected user: https://bug772692.bugzilla-attachments.gnome.org/attachment.cgi?id=338310

The graphics stacks look totally different. At first I thought there might be an ABI issue in Mageia, but they've rebuilt everything recently so that's unlikely.

There is some more debug information in the downstream bug, including a valgrind log, but unfortunately nothing that looked useful to me.

[Robert Fox]

For background on the bug filed on Evolution bugzilla:
https://bugzilla.gnome.org/show_bug.cgi?id=772692

Just wanted to report that this issue is independent of what graphics card (happens on Intel based laptop as well as NVidia based PC using either nouveau or NVidia proprietary drivers)

[Robert Fox]

Here's the Mageia bug related to this issue:
https://bugs.mageia.org/show_bug.cgi?id=19470

I'm the the one who originated this bug - and I'm still affected

[Michael Catanzaro]

I wouldn't expect any progress on this since it only seems to affect Mageia users and no WebKit developers are using Mageia, sorry. :/

[Michael Catanzaro]

Good news for you, this started affecting Fedora. :)

[Michael Catanzaro]

The Fedora maintainer (Tom) reports that the bug (really strangely) disappears when doing a Release build. (You're probably doing a RelWithDebInfo build, because that's what all distros want to use.) So you might try that and see if it works as a workaround until we figure out what's going on.

[Michael Catanzaro]

(In reply to comment #5)
> The Fedora maintainer (Tom) reports that the bug (really strangely)
> disappears when doing a Release build. (You're probably doing a
> RelWithDebInfo build, because that's what all distros want to use.)

Turns out Fedora and Debian are both (IMO incorrectly) using Release builds instead of RelWithDebInfo builds. So that explains why it's only broken for Mageia.

[Carlos Garcia Campos]

I might be wrong but I understand Release is a production build and therefore what distros should do. At least I hope Debian keeps doing so.

[Carlos Alberto Lopez Perez]

Why RelWithDebInfo builds are broken???

[Michael Catanzaro]

(In reply to comment #7)
> I might be wrong but I understand Release is a production build and
> therefore what distros should do. At least I hope Debian keeps doing so.

Release is a production build WITHOUT debuginfo, i.e. with debuginfo explicitly disabled. All Linux distros want RelWithDebInfo, which is production build WITH debuginfo. Turns out only Mageia was doing this properly, so only Mageia was broken.

It makes no real difference, because all distros override the CFLAGS with -O2 -g anyway so you get a release build with debuginfo regardless of build type, but it seems good to pick the build type that matches what you semantically want to do.

(In reply to comment #8)
> Why RelWithDebInfo builds are broken???

Because our CMake files check the build type and add necessary compiler flags only if it is "Release" or "Debug" in various places. We need to audit the whole thing.

mcatanzaro:  annulen: Not possible to grep for something? CMAKE_BUILD_TYPE?
annulen:  CMAKE_BUILD_TYPE and _RELEASE or _DEBUG variables
annulen:  and decide what should be done in each case

(A related bug is that RelWithDebInfo will not define NDEBUG, which will enable assertions, which we don't want. That's a bug in our choice of how to decide when to enable assertions. They should be dependent on DEVELOPER_MODE rather than NDEBUG. Mageia was defining NDEBUG manually to avoid this problem.)

[Michael Catanzaro]

(In reply to comment #4)
> Good news for you, this started affecting Fedora. :)

(For the record: what happened is Tom decided to do a RelWithDebInfo build for some reason, and showed me the screenshots of the breakage that resulted. He picked DuckDuckGo, which happened to be the same website that the Mageia users took screenshots of so it looked exactly like the screenshots above. So we got really lucky to figure this out.)

[Fujii Hironori]

Created attachment 300514 [details]
annulen's patch

[Michael Catanzaro]

Looks good to me, but why were these flags not previously required in debug mode?

[Konstantin Tokarev]

-fstrict-aliasing is enabled in GCC since -O2 or -Os levels only, so debug builds don't require -fno-strict-aliasing flag for correct work

[Michael Catanzaro]

OK.

Do you want to submit a patch for review?

[Michael Catanzaro]

Comment on attachment 300514 [details]
annulen's patch

r=me because this is clearly better than what we're doing now.

I'm a bit concerned that we're adding our flags after ${CMAKE_C_FLAGS} rather than before. That is, we're overriding the CFLAGS and CXXFLAGS set by the user. That's generally considered to be bad form in Autotools projects, where the user is always supposed to be override our build flags. But maybe it's better that we don't give the user control over these particular flags. E.g. Linux distros use -fexceptions in order to get better backtraces, but that's really mostly intended for C projects, not C++ projects that don't use exceptions; I imagine it could lead to significant code size bloat in WebKit for insufficient benefit, and that change would be immediately noticed if we were to change this. So I dunno.

[Konstantin Tokarev]

Enabling exceptions may also have negative effect on performance, as they can prevent compiler optimizations (I encountered this in practise)

[Michael Catanzaro]

I think the right answer is this:

(In reply to comment #15)
> maybe it's better that we don't give the user control over these particular flags.

So let's use your patch with no changes.

[Konstantin Tokarev]

Comment on attachment 300514 [details]
annulen's patch

This is certainly not the last thing to do to support configurations different from "Release" and "Debug", I'll try to provide necessary changes soon

[Konstantin Tokarev]

Comment on attachment 300514 [details]
annulen's patch

Oops, changelog

[Konstantin Tokarev]

Committed r211635: <http://trac.webkit.org/changeset/211635>