Summary: | [GTK] REGRESSION(r231170) Build broken with Clang 5.0 | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Alberto Lopez Perez <clopez> | ||||||||||||||||
Component: | WebKitGTK | Assignee: | Zan Dobersek <zan> | ||||||||||||||||
Status: | REOPENED --- | ||||||||||||||||||
Severity: | Normal | CC: | achristensen, aperez, bugs-noreply, calvaris, cgarcia, ddkilzer, ews-watchlist, jbicha, jfbastien, lantw44, loic.yhuel, mcatanzaro, ysuzuki, zan | ||||||||||||||||
Priority: | P2 | ||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||
See Also: | https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=8209 | ||||||||||||||||||
Bug Depends on: | |||||||||||||||||||
Bug Blocks: | 185135, 185947 | ||||||||||||||||||
Attachments: |
|
Description
Carlos Alberto Lopez Perez
2018-05-02 08:15:47 PDT
JFTR, building with Clang 6.0 is not possible anymore as well. The error in this case is: ../Source/WTF/wtf/Optional.h:224:26: error: explicit specialization of non-template class 'optional' template <class T> class optional<T&>; ^ ~~~~ I tried debugging a bit, and the <optional> header does get included with Clang 6.0, but that is as far as I can go right now. I may look into this again when I'm less busy. It looks the failure started to happen when we passed the flag "-std=c++17" to the build. Trying to build r231169 with this simple patch --- a/Source/cmake/WebKitCompilerFlags.cmake +++ b/Source/cmake/WebKitCompilerFlags.cmake @@ -102,7 +102,7 @@ if (COMPILER_IS_GCC_OR_CLANG) -Wno-unknown-argument) else () WEBKIT_APPEND_GLOBAL_COMPILER_FLAGS(-fno-exceptions) - WEBKIT_APPEND_GLOBAL_CXX_FLAGS(-std=c++14 + WEBKIT_APPEND_GLOBAL_CXX_FLAGS(-std=c++17 -fno-rtti) if (WIN32) Makes it already fail (even when no c++17 feature is still used as that was introduced in r231170). Looking at what gcc and clang define with -std=c++14 vs -std=c++17 I see some differences. $ touch x.cpp $ g++-6 -std=c++14 -dM -E x.cpp |nc termbin.com 9999 http://termbin.com/sx9x $ g++-6 -std=c++17 -dM -E x.cpp |nc termbin.com 9999 http://termbin.com/8ff5 $ clang++-5.0 -std=c++14 -dM -E x.cpp |nc termbin.com 9999 http://termbin.com/6k6p $ clang++-5.0 -std=c++17 -dM -E x.cpp |nc termbin.com 9999 http://termbin.com/y4jy One that rings a bell is For example clang-5.0 with -std=c++17 sets "#define __cplusplus 201703L" meanwhile GCC 6.3 with -std=c++17 sets "#define __cplusplus 201500L". Created attachment 339322 [details]
wip patch
This allows for me the GTK build to pass with clang-5.0. No idea if it will break anything else. Let's see what the EWS says.
(In reply to Carlos Alberto Lopez Perez from comment #3) > Created attachment 339322 [details] > wip patch > > This allows for me the GTK build to pass with clang-5.0. No idea if it will > break anything else. Let's see what the EWS says. I tried your patch locally with Clang 6.0; but it ends up failing in a different way (error output below). --- In file included from ../../Source/WTF/wtf/persistence/PersistentDecoder.cpp:26: In file included from ../../Source/WTF/config.h:31: In file included from ../../Source/WTF/wtf/FastMalloc.h:26: ../../Source/WTF/wtf/StdLibExtras.h:554:38: error: reference to 'in_place_t' is ambiguous __IN_PLACE_INLINE_VARIABLE constexpr in_place_t in_place { }; ^ ../../Source/WTF/wtf/StdLibExtras.h:551:8: note: candidate found by name lookup is 'std::in_place_t' struct in_place_t { ^ /usr/bin/../include/c++/v1/utility:904:25: note: candidate found by name lookup is 'std::__1::in_place_t' struct _LIBCPP_TYPE_VIS in_place_t { ^ fatal error: too many errors emitted, stopping now [-ferror-limit=] You might want to look at the WIP patch here and import some of the workarounds: https://bugs.webkit.org/show_bug.cgi?id=185176 JF, there's an attempt to fix this in bug #185159, but it broke Apple internal builds. It's simply that redefining std::optional is illegal, and WebKit currently does that. We might need to temporarily revert the switch to C++17 if we can't find a solution quickly. *** This bug has been marked as a duplicate of bug 185159 *** (In reply to Michael Catanzaro from comment #6) > JF, there's an attempt to fix this in bug #185159, but it broke Apple > internal builds. It's simply that redefining std::optional is illegal, and > WebKit currently does that. > > We might need to temporarily revert the switch to C++17 if we can't find a > solution quickly. > > *** This bug has been marked as a duplicate of bug 185159 *** Build with clang continues to be broken. I don't think this is a duplicate of 185159. Our Ubuntu LTS bot which tries to build with clang is still broken Created attachment 339812 [details]
WIP
Attachment 339812 [details] did not pass style-queue:
ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5]
Total errors found: 1 in 14 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 339812 [details] WIP View in context: https://bugs.webkit.org/attachment.cgi?id=339812&action=review > Source/WTF/wtf/Optional.h:219 > +#if defined(__GLIBCXX__) && !defined(_GLIBCXX_RELEASE) What on Earth is this testing for...? Unreleased libstdc++? > Source/WebCore/css/makeSelectorPseudoClassAndCompatibilityElementMap.py:104 > +#define register Ugh, gperf :( > Source/cmake/WebKitCompilerFlags.cmake:117 > + check_cxx_compiler_flag("-std=c++17" CXX_COMPILER_SUPPORTS_CXX17) > + if (CXX_COMPILER_SUPPORTS_CXX17) > + WEBKIT_APPEND_GLOBAL_CXX_FLAGS(-std=c++17) > + else () > + check_cxx_compiler_flag("-std=c++1z" CXX_COMPILER_SUPPORTS_CXX1Z) > + if (CXX_COMPILER_SUPPORTS_CXX1Z) > + WEBKIT_APPEND_GLOBAL_CXX_FLAGS(-std=c++1z) > + else () > + message(FATAL_ERROR "Compiler with C++17 support is required") > + endif () > + endif () Looks good. Created attachment 339822 [details]
WIP
Attachment 339822 [details] did not pass style-queue:
ERROR: Source/WTF/wtf/StdLibExtras.h:587: in_place_t is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5]
Total errors found: 2 in 14 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 339842 [details]
WIP
Attachment 339842 [details] did not pass style-queue:
ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5]
Total errors found: 1 in 17 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 339842 [details] WIP View in context: https://bugs.webkit.org/attachment.cgi?id=339842&action=review > Source/WTF/wtf/StdLibExtras.h:546 > +#if __cplusplus < 201703L || (defined(__GLIBCXX__) && !defined(_GLIBCXX_RELEASE)) Why not just "#if __cplusplus <= 201703L" ? (In reply to Michael Catanzaro from comment #10) > Comment on attachment 339812 [details] > WIP > > View in context: > https://bugs.webkit.org/attachment.cgi?id=339812&action=review > > > Source/WTF/wtf/Optional.h:219 > > +#if defined(__GLIBCXX__) && !defined(_GLIBCXX_RELEASE) > > What on Earth is this testing for...? Unreleased libstdc++? Comment on attachment 339842 [details] WIP View in context: https://bugs.webkit.org/attachment.cgi?id=339842&action=review >> Source/WTF/wtf/StdLibExtras.h:546 >> +#if __cplusplus < 201703L || (defined(__GLIBCXX__) && !defined(_GLIBCXX_RELEASE)) > > Why not just "#if __cplusplus <= 201703L" ? Because with __cplusplus == 201703L, this clashes with std::in_place_t that's defined in <utility>. (In reply to Michael Catanzaro from comment #16) > (In reply to Michael Catanzaro from comment #10) > > Comment on attachment 339812 [details] > > WIP > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=339812&action=review > > > > > Source/WTF/wtf/Optional.h:219 > > > +#if defined(__GLIBCXX__) && !defined(_GLIBCXX_RELEASE) > > > > What on Earth is this testing for...? Unreleased libstdc++? libstdc++ released alongside GCC 6.x, which didn't yet define _GLIBCXX_RELEASE. Created attachment 339946 [details]
Patch
Comment on attachment 339946 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=339946&action=review > Source/WTF/wtf/StdLibExtras.h:548 > +// Provide in_place_t when not building with -std=c++17, or when building with libstdc++ 6 > +// (which doesn't define the _GLIBCXX_RELEASE macro that's been introduced in libstdc++ 7). > +#if __cplusplus < 201703L || (defined(__GLIBCXX__) && !defined(_GLIBCXX_RELEASE)) Wow, it looks like there's really no better way to do this. Yuck. https://stackoverflow.com/questions/31506594/how-do-i-test-the-version-of-libstdc-not-gcc-at-compile-time has no good solutions. (In reply to Zan Dobersek from comment #18) > (In reply to Michael Catanzaro from comment #16) > > (In reply to Michael Catanzaro from comment #10) > > > Comment on attachment 339812 [details] > > > WIP > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=339812&action=review > > > > > > > Source/WTF/wtf/Optional.h:219 > > > > +#if defined(__GLIBCXX__) && !defined(_GLIBCXX_RELEASE) > > > > > > What on Earth is this testing for...? Unreleased libstdc++? > > libstdc++ released alongside GCC 6.x, which didn't yet define > _GLIBCXX_RELEASE. I don't understand why this is needed: GCC 6 isn't supported anymore. We just dropped support for GCC 5. We still need to support GCC 6. This bug is for fixing compilation with clang and libstdc++ (in Zan's case, provided by GCC 6). We can't test GCC version, because we're not using GCC. So I think Zan's patch is the best we can do. C++ 17 is really not going to be as easy as was our transition to C++ 14. :( (In reply to Michael Catanzaro from comment #22) > We just dropped support for GCC 5. We still need to support GCC 6. Oh derp yes, ignore me! :-) Comment on attachment 339946 [details] Patch Attachment 339946 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/7625241 New failing tests: http/tests/security/video-poster-cross-origin-crash2.html Created attachment 339996 [details]
Archive of layout-test-results from ews200 for win-future
The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews200 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment on attachment 339946 [details]
Patch
Seems it will broke the wincairo build:
C:\WebKit-EWS\WebKit\Source\WebCore\platform\ReferrerPolicy.h(54): error C2039: 'optional': is not a member of 'std'
C:\WebKit-EWS\WebKit\WebKitBuild\Release\DerivedSources\ForwardingHeaders\wtf/Forward.h(73): note: see declaration of 'std'
C:\WebKit-EWS\WebKit\Source\WebCore\platform\ReferrerPolicy.h(54): error C2143: syntax error: missing ';' before '<'
C:\WebKit-EWS\WebKit\Source\WebCore\platform\ReferrerPolicy.h(54): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
Created attachment 340303 [details]
Patch for landing
Comment on attachment 340303 [details] Patch for landing Clearing flags on attachment: 340303 Committed r231753: <https://trac.webkit.org/changeset/231753> All reviewed patches have been landed. Closing bug. r231753 fixed the build for the Debian bot which uses clang-3.8. But the Ubuntu one with clang-5.0 remains broken: https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20Ubuntu%20LTS%20%28Build%29/builds/12353/steps/compile-webkit/logs/stdio I already tried ordering a clean build. Didn't helped (In reply to Carlos Alberto Lopez Perez from comment #30) > r231753 fixed the build for the Debian bot which uses clang-3.8. > But the Ubuntu one with clang-5.0 remains broken: > https://build.webkit.org/builders/GTK%20Linux%2064- > bit%20Release%20Ubuntu%20LTS%20%28Build%29/builds/12353/steps/compile-webkit/ > logs/stdio > > I already tried ordering a clean build. Didn't helped Which libstdc++ is used there? (In reply to Zan Dobersek from comment #31) > (In reply to Carlos Alberto Lopez Perez from comment #30) > > r231753 fixed the build for the Debian bot which uses clang-3.8. > > But the Ubuntu one with clang-5.0 remains broken: > > https://build.webkit.org/builders/GTK%20Linux%2064- > > bit%20Release%20Ubuntu%20LTS%20%28Build%29/builds/12353/steps/compile-webkit/ > > logs/stdio > > > > I already tried ordering a clean build. Didn't helped > > Which libstdc++ is used there? - Debian bot, its currently on 8.0 (jessie), its using libstdc++ 4.9 and clang-3.8 (building fine here after r231753) - Ubuntu but, its currently on 16.04 (xenial), its using libstdc++ 5.4 and clang-5.0 (still failing to build after r231753) I think we need to come to some agreement regarding old versions of libstdc++. Our current dependencies policy doesn't mention it specifically, but I think the intent is clear that it's part of the build toolchain and the min version should match GCC.... Surely the bots should be able to static link to libstdc++, right? There is no C++ in the public API, after all, so the build version should not need to match the system version, right? I know we've discussed this before, but I don't remember the conclusion, and something has gone wrong for us to still be testing libstdc++ provided by GCC 4.9 and 5 after we've increased the compiler requirement past that. (In reply to Michael Catanzaro from comment #33) > I think we need to come to some agreement regarding old versions of > libstdc++. Our current dependencies policy doesn't mention it specifically, > but I think the intent is clear that it's part of the build toolchain and > the min version should match GCC.... > > Surely the bots should be able to static link to libstdc++, right? There is > no C++ in the public API, after all, so the build version should not need to > match the system version, right? > > I know we've discussed this before, but I don't remember the conclusion, and > something has gone wrong for us to still be testing libstdc++ provided by > GCC 4.9 and 5 after we've increased the compiler requirement past that. There's an ABI break between libstdc++4.9 and 5, so you want to compile with _GLIBCXX_USE_CXX11_ABI=0 and use -Wabi-tag so that you can compile with a version after 5 yet at runtime use libstdc++4.9 and older. (In reply to JF Bastien from comment #34) > (In reply to Michael Catanzaro from comment #33) > > I think we need to come to some agreement regarding old versions of > > libstdc++. Our current dependencies policy doesn't mention it specifically, > > but I think the intent is clear that it's part of the build toolchain and > > the min version should match GCC.... > > > > Surely the bots should be able to static link to libstdc++, right? There is > > no C++ in the public API, after all, so the build version should not need to > > match the system version, right? > > > > I know we've discussed this before, but I don't remember the conclusion, and > > something has gone wrong for us to still be testing libstdc++ provided by > > GCC 4.9 and 5 after we've increased the compiler requirement past that. > > There's an ABI break between libstdc++4.9 and 5, so you want to compile with > _GLIBCXX_USE_CXX11_ABI=0 and use -Wabi-tag so that you can compile with a > version after 5 yet at runtime use libstdc++4.9 and older. Indeed, there are some cases in which it is desirable to be able to use the libstdc++ 4.9 ABI at runtime. The main case that comes to mind is being able to run WebKit{GTK+,WPE} on embedded systems which have their userland built with GCC <5.0: we can build a newer cross-GCC, and then use that to build WebKit, which will link against system libraries which use the old ABI, and use libstdc++ <5.0 at runtime. I don't think that's even relevant here, because downgrades are never expected to work, right? You can't build against libstdc++ 8 and expect it to work with libstdc++ 5, even with the exact same ABI version, right? There is another problem. There is zero ABI stability until the language standard is marked as non-experimental. So ABI stability for -std=c++17 won't begin until libstdc++ 9 at the *earliest*. (In reply to Michael Catanzaro from comment #33) > I think we need to come to some agreement regarding old versions of > libstdc++. Our current dependencies policy doesn't mention it specifically, > but I think the intent is clear that it's part of the build toolchain and > the min version should match GCC.... > On a GNU/Linux distribution libstdc++ is considered a core library. You can't upgrade it without dist-uprading the whole distribution to the next version of the distribution. AFAIK the only way of keeping WebKit buildable once GCC is not new enough is with Clang, because its not possible to upgrade GCC without updating libstdc++ (which would mean dist-upgrading) So, either we keep doing this (using Clang on this extra year of support promised by our dependencies policy) or we give up on that extra year and we dist-upgrade the bots to the last version of the distro and use newer GCC. (In reply to Michael Catanzaro from comment #33) > There is > no C++ in the public API, after all, so the build version should not need to > match the system version, right? Is that wrong? Practically speaking, we just need some way we can tell these LTS distros to build WebKit with newer build toolchains, and expect it to work at runtime where the system copy of libstdc++ is older. Static linking to libstdc++ seems like the only way it could be possible. Otherwise, we're going to be stuck supporting ancient versions of libstdc++ and that's going to really seriously limit the benefit to bumping the compiler version.... (In reply to Michael Catanzaro from comment #38) > (In reply to Michael Catanzaro from comment #33) > > There is > > no C++ in the public API, after all, so the build version should not need to > > match the system version, right? > > Is that wrong? > > Practically speaking, we just need some way we can tell these LTS distros to > build WebKit with newer build toolchains, and expect it to work at runtime > where the system copy of libstdc++ is older. > Static linking to libstdc++ seems like the only way it could be possible. I have no idea if this is possible. But I'm certainly sure this is far from practical. How do you fetch a newer libstdc++ and build against it? Also.. won't this be a problem when webkit links with system libraries implemented in c++ (like icu) that link with the (older) system libstdc++ ? (In reply to Michael Catanzaro from comment #38) > Static linking to libstdc++ seems like the only way it could be possible. Building with clang and libc++ should be possible. libc++ and libstdc++ can be used in the same process, as long as there isn't any STL object in the API between parts built with the different libraries (or any C++ object if libc++ is built with libc++abi instead of libstdc++(libsupc++)). But Ubuntu 16.04 has libc++ 3.7 (older than the default clang !), which has issues IIRC (header issues which are fixed later upstream and haven't been backported, and missing headers on install). Could we ask Ubuntu to update it ? It doesn't impact the whole distribution like libstdc++, only android-sdk-build-tools seems to use it. (In reply to Loïc Yhuel from comment #40) > (In reply to Michael Catanzaro from comment #38) > > Static linking to libstdc++ seems like the only way it could be possible. > > Building with clang and libc++ should be possible. > libc++ and libstdc++ can be used in the same process, as long as there isn't > any STL object in the API between parts built with the different libraries > (or any C++ object if libc++ is built with libc++abi instead of > libstdc++(libsupc++)). > > But Ubuntu 16.04 has libc++ 3.7 (older than the default clang !), which has > issues IIRC (header issues which are fixed later upstream and haven't been > backported, and missing headers on install). > Could we ask Ubuntu to update it ? It doesn't impact the whole distribution > like libstdc++, only android-sdk-build-tools seems to use it. Well, the intention of providing this extra year of support is (IMHO): 1. To allow developers working in webkit continue using their LTS or Stable distro without being forced to update as soon as a new one is released. I guess we already failed here because WebKit has been broken on Ubuntu 16.04 for weeks already. 2. To allow distributions that want to keep providing security upgrades to their LTS/stable distributions (which means updating to a major version of WebKitGTK+) to be able to do that. I'm already not happy with the idea that we have to tell them to switch the compiler (from GCC to Clang) to be able to do that. But the idea that we also are going to tell them to statically build with another c++ library that nobody has tested in GNU/Linux its kind of crazy (IMHO). There is a clear conflict of interest here between those that want to use the very last standard of C++ (C++-17 which has been released only a year ago) and those that want last WebKit to be buildable on Stable/LTS distributions (which are distributions that were released up to 3 years ago and that don't provide updated toolchains). Jeremy (CC'ed) ... is Ubuntu planning to keep providing security upgrades for WebKitGTK+ beyond 2.20 (shipping 2.22.x) for Ubuntu LTS 16.04? If not we may just give up on this extra year of support. (In reply to Carlos Alberto Lopez Perez from comment #39) > I have no idea if this is possible. But I'm certainly sure this is far from > practical. How do you fetch a newer libstdc++ and build against it? The easiest solution would be to build the compiler (including standard library) as part of the WebKit package build. libc++ would work too. (In reply to Carlos Alberto Lopez Perez from comment #41) > Jeremy (CC'ed) ... is Ubuntu planning to keep providing security upgrades > for WebKitGTK+ beyond 2.20 (shipping 2.22.x) for Ubuntu LTS 16.04? If not we > may just give up on this extra year of support. Well that's the thing, it looks like they want five years of support, and I think we can provide that. So I want to increase the dependencies policy from three years to five for runtime deps. But we surely cannot do more than two years for the build toolchain. (In reply to Carlos Alberto Lopez Perez from comment #41) > I'm already not happy with the idea that we have to tell them to switch the > compiler (from GCC to Clang) to be able to do that. But the idea that we > also are going to tell them to statically build with another c++ library > that nobody has tested in GNU/Linux its kind of crazy (IMHO). > Chromium links statically with its own libc++ (using its own clang) : https://bugs.chromium.org/p/chromium/issues/detail?id=593874 It seems to be used on Ubuntu, while other distributions may do otherwise : - Fedora builds chromium with gcc and libstdc++ - Archlinux uses system clang and libstdc++ (In reply to Michael Catanzaro from comment #36) > I don't think that's even relevant here, because downgrades are never > expected to work, right? You can't build against libstdc++ 8 and expect it > to work with libstdc++ 5, even with the exact same ABI version, right? Yes, it can work: remember that templates are expanded and the code they generate incorporated in the built binaries. That is what one gets from headers, while “libstdc++-.so.<num>” contains non-templated functionality. The ABI version refers to what the latter includes, and by defining “_GLIBCXX_USE_CXX11_ABI=0” what we are telling the C++ headers is that they should use symbols which are available in libstdc++ versions prior to version 5.x. Some more information: https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_macros.html (On a different note, here's a fun fact: it is possible to build exactly the same libstdc++ version with different compile-time options and have a “libstdc++.so.<num>” as result with an incompatible ABI. More here: https://gcc.gnu.org/onlinedocs/libstdc++/manual/configure.html) > There is another problem. There is zero ABI stability until the language > standard is marked as non-experimental. So ABI stability for -std=c++17 > won't begin until libstdc++ 9 at the *earliest*. Not quite. Again: C++ headers can change from one version to another of the compiler/libstdc++, and as long as the ABI of “libstdc++*.so” remains stable, programs built with compiler/headers of different versions will continue to work as long as they target the same ABI for the shared library. The following is a quote from one of the pages linked above: “Although the changes [in the ABI] were made for C++11 conformance, the choice of ABI to use is independent of the -std option used to compile your code, i.e. for a given GCC build the default value of the _GLIBCXX_USE_CXX11_ABI macro is the same for all dialects. This ensures that the -std does not change the ABI, so that it is straightforward to link C++03 and C++11 code together.“ As far as I understand the status quo, there are no plans to change the ABI again, and the C++17 support (In reply to Carlos Alberto Lopez Perez from comment #39) > (In reply to Michael Catanzaro from comment #38) > > (In reply to Michael Catanzaro from comment #33) > > > There is > > > no C++ in the public API, after all, so the build version should not need to > > > match the system version, right? > > > > Is that wrong? > > > > Practically speaking, we just need some way we can tell these LTS distros to > > build WebKit with newer build toolchains, and expect it to work at runtime > > where the system copy of libstdc++ is older. > > Static linking to libstdc++ seems like the only way it could be possible. > > I have no idea if this is possible. But I'm certainly sure this is far from > practical. How do you fetch a newer libstdc++ and build against it? > > Also.. won't this be a problem when webkit links with system libraries > implemented in c++ (like icu) that link with the (older) system libstdc++ ? Yes, that would be indeed a problem. That is also the reason why distributions in general ship shared libraries for libstdc++. One should only link libstdc++ statically if the whole program is linked statically — which is something we would rather not do for WebKit. ...and I suspect that is one of the reasons the “_GLIBCXX_USE_CXX11_ABI=0” escape hatch was introduced as well. Regarding Clang's libc++, I have been using it regularly without any issue at all. If something, it results in WebKit binaries which use slightly less memory. Sometimes I feel like I am one of the few people around building WebKitGTK+ regularly with Clang (and libc++) ;-) (In reply to Adrian Perez from comment #46) > Regarding Clang's libc++, I have been using it regularly without > any issue at all. If something, it results in WebKit binaries > which use slightly less memory. Sometimes I feel like I am one > of the few people around building WebKitGTK+ regularly with > Clang (and libc++) ;-) We do too ;-) (In reply to Carlos Alberto Lopez Perez from comment #32) > (In reply to Zan Dobersek from comment #31) > > (In reply to Carlos Alberto Lopez Perez from comment #30) > > > r231753 fixed the build for the Debian bot which uses clang-3.8. > > > But the Ubuntu one with clang-5.0 remains broken: > > > https://build.webkit.org/builders/GTK%20Linux%2064- > > > bit%20Release%20Ubuntu%20LTS%20%28Build%29/builds/12353/steps/compile-webkit/ > > > logs/stdio > > > > > > I already tried ordering a clean build. Didn't helped > > > > Which libstdc++ is used there? > > - Debian bot, its currently on 8.0 (jessie), its using libstdc++ 4.9 and > clang-3.8 (building fine here after r231753) > - Ubuntu but, its currently on 16.04 (xenial), its using libstdc++ 5.4 and > clang-5.0 (still failing to build after r231753) I switched the Ubuntu bot to use clang-3.8 and now is building fine :) I tested on it this versions of clang that are available on Ubuntu 16.04: clang-3.8: builds ok clang-3.9: builds ok clang-4.0: fails (different failure than 5.0) clang-5.0: fails The default version of clang (the one you get after doing "apt-get install clang") on Ubuntu 16.04 is 3.8 So I guess its ok, as webkit remains buildable there using the default clang version (clang-3.8). At any rate it still would be interesting to fix the build with the other clang versions. This are the macros enabled for each case: clang++-3.8 -std=c++17 -dM -E -> error: invalid value 'c++17' in '-std=c++17' clang++-3.8 -std=c++1z -dM -E -> http://termbin.com/q8sf clang++-3.9 -std=c++17 -dM -E -> error: invalid value 'c++17' in '-std=c++17' clang++-3.9 -std=c++1z -dM -E -> http://termbin.com/l4u2 clang++-4.0 -std=c++17 -dM -E -> error: invalid value 'c++17' in '-std=c++17' clang++-4.0 -std=c++1z -dM -E -> http://termbin.com/4i7f clang++-5.0 -std=c++17 -dM -E -> http://termbin.com/s54p clang++-5.0 -std=c++1z -dM -E -> http://termbin.com/2ceb *** Bug 185947 has been marked as a duplicate of this bug. *** (In reply to Carlos Alberto Lopez Perez from comment #49) > *** Bug 185947 has been marked as a duplicate of this bug. *** Clang 6.0 seems also broken :( (In reply to Carlos Alberto Lopez Perez from comment #50) > (In reply to Carlos Alberto Lopez Perez from comment #49) > > *** Bug 185947 has been marked as a duplicate of this bug. *** > > Clang 6.0 seems also broken :( Yes, Clang 6 cannot be used to compile WebKitGTK+ 2.21.2 and the current WebKit trunk, but the problem described in bug 185947 is slightly different from the LTS and outdated C++ library issue discussed here. The Clang 6 test was done on two systems which both have up to date C++ libraries: libc++ 6 on FreeBSD 11.2-BETA2 and libstdc++ 8 on Fedora 28. Clang 5 works fine with both libc++ 4 on FreeBSD 11.1-RELEASE and libc++ 6 on FreeBSD 11.2-BETA2. Was this bug going to be used to fix the Clang 6.0+ warnings as well, or should we use Bug 185947 instead? (In reply to David Kilzer (:ddkilzer) from comment #52) > Was this bug going to be used to fix the Clang 6.0+ warnings as well or > should we use Bug 185947 instead? Use bug #185947 instead. This bug is probably going to be closed if the build failure ever gets fixed. |