163897 2016-10-24 10:17:37 -0700 [CMake] RelWithDebInfo builds are super broken at runtime 2017-02-03 09:22:50 -0800 1 1 1 Unclassified WebKit WebKitGTK Other PC Linux RESOLVED FIXED https://bugzilla.gnome.org/show_bug.cgi?id=772692 https://bugs.mageia.org/show_bug.cgi?id=19470 P2 Normal --- 1 mcatanzaro webkit-unassigned annulen berto bugs-noreply cgarcia clopez commit-queue fujii mcatanzaro rfox tpopela oldest_to_newest 1243753 0 mcatanzaro 2016-10-24 10:17:37 -0700 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. 1244205 1 rfox 2016-10-25 08:38:26 -0700 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) 1253640 2 rfox 2016-11-25 03:05:41 -0800 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 1253649 3 mcatanzaro 2016-11-25 07:52:27 -0800 I wouldn't expect any progress on this since it only seems to affect Mageia users and no WebKit developers are using Mageia, sorry. :/ 1271678 4 mcatanzaro 2017-01-31 05:48:49 -0800 Good news for you, this started affecting Fedora. :) 1271679 5 mcatanzaro 2017-01-31 05:51:10 -0800 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. 1271686 6 mcatanzaro 2017-01-31 06:25:30 -0800 (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. 1271720 7 cgarcia 2017-01-31 08:04:26 -0800 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. 1271728 8 clopez 2017-01-31 08:28:45 -0800 Why RelWithDebInfo builds are broken??? 1271750 9 mcatanzaro 2017-01-31 09:35:13 -0800 (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.) 1271751 10 mcatanzaro 2017-01-31 09:37:51 -0800 (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.) 1272846 11 300514 fujii 2017-02-03 02:04:17 -0800 Created attachment 300514 annulen's patch 1272868 12 mcatanzaro 2017-02-03 04:07:16 -0800 Looks good to me, but why were these flags not previously required in debug mode? 1272870 13 annulen 2017-02-03 04:15:30 -0800 -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 1272898 14 mcatanzaro 2017-02-03 06:22:57 -0800 OK. Do you want to submit a patch for review? 1272914 15 300514 mcatanzaro 2017-02-03 08:27:12 -0800 Comment on attachment 300514 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. 1272921 16 annulen 2017-02-03 08:31:22 -0800 Enabling exceptions may also have negative effect on performance, as they can prevent compiler optimizations (I encountered this in practise) 1272922 17 mcatanzaro 2017-02-03 08:32:53 -0800 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. 1272925 18 300514 annulen 2017-02-03 08:41:35 -0800 Comment on attachment 300514 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 1272928 19 300514 annulen 2017-02-03 08:44:54 -0800 Comment on attachment 300514 annulen's patch Oops, changelog 1272941 20 annulen 2017-02-03 09:22:50 -0800 Committed r211635: <http://trac.webkit.org/changeset/211635> 300514 2017-02-03 02:04:17 -0800 2017-02-03 08:44:54 -0800 annulen's patch annulen.patch text/plain 842 fujii ZGlmZiAtLWdpdCBhL1NvdXJjZS9jbWFrZS9PcHRpb25zQ29tbW9uLmNtYWtlIGIvU291cmNlL2Nt YWtlL09wdGlvbnNDb21tb24uY21ha2UKaW5kZXggNTliNzEyODJlNTcuLjE2MzAyOWY1MTYxIDEw MDY0NAotLS0gYS9Tb3VyY2UvY21ha2UvT3B0aW9uc0NvbW1vbi5jbWFrZQorKysgYi9Tb3VyY2Uv Y21ha2UvT3B0aW9uc0NvbW1vbi5jbWFrZQpAQCAtMzgsOCArMzgsOCBAQCBzZXRfcHJvcGVydHko R0xPQkFMIFBST1BFUlRZIFVTRV9GT0xERVJTIE9OKQogZGVmaW5lX3Byb3BlcnR5KFRBUkdFVCBQ Uk9QRVJUWSBGT0xERVIgSU5IRVJJVEVEIEJSSUVGX0RPQ1MgImZvbGRlciIgRlVMTF9ET0NTICJJ REUgZm9sZGVyIG5hbWUiKQogCiBpZiAoQ09NUElMRVJfSVNfR0NDX09SX0NMQU5HKQotICAgIHNl dChDTUFLRV9DX0ZMQUdTX1JFTEVBU0UgIiR7Q01BS0VfQ19GTEFHU19SRUxFQVNFfSAtZm5vLWV4 Y2VwdGlvbnMgLWZuby1zdHJpY3QtYWxpYXNpbmciKQotICAgIHNldChDTUFLRV9DWFhfRkxBR1Nf UkVMRUFTRSAiJHtDTUFLRV9DWFhfRkxBR1NfUkVMRUFTRX0gLWZuby1leGNlcHRpb25zIC1mbm8t c3RyaWN0LWFsaWFzaW5nIC1mbm8tcnR0aSIpCisgICAgc2V0KENNQUtFX0NfRkxBR1MgIiR7Q01B S0VfQ19GTEFHU30gLWZuby1leGNlcHRpb25zIC1mbm8tc3RyaWN0LWFsaWFzaW5nIikKKyAgICBz ZXQoQ01BS0VfQ1hYX0ZMQUdTICIke0NNQUtFX0NYWF9GTEFHU30gLWZuby1leGNlcHRpb25zIC1m bm8tc3RyaWN0LWFsaWFzaW5nIC1mbm8tcnR0aSIpCiAgICAgc2V0KENNQUtFX0NYWF9GTEFHUyAi JHtDTUFLRV9DWFhfRkxBR1N9IC1zdGQ9YysrMXkiKQogZW5kaWYgKCkKIAo=