Summary: | www/webkit2-gtk: interactions with C++ dependencies built with clang/libc++ causes WebProcess crash | ||
---|---|---|---|
Product: | Ports & Packages | Reporter: | huanghwh |
Component: | Individual Port(s) | Assignee: | freebsd-gnome (Nobody) <gnome> |
Status: | Closed FIXED | ||
Severity: | Affects Only Me | CC: | eduardo, grahamperrin, jcfyecrayz, junchoon, marklmi26-fbsd, pi, shamaz.mazum, toolchain, vishwin |
Priority: | --- | Flags: | vishwin:
maintainer-feedback+
|
Version: | Latest | ||
Hardware: | amd64 | ||
OS: | Any | ||
See Also: |
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=284258 https://bugs.webkit.org/show_bug.cgi?id=278596 |
||
Bug Depends on: | 284445 | ||
Bug Blocks: |
Description
huanghwh
2025-01-27 00:16:20 UTC
Could you run with 'env WEBKIT_DISABLE_COMPOSITING_MODE=1'? $ env WEBKIT_DISABLE_COMPOSITING_MODE=1 <browser> <url> Ate least here I can render page for a dew seconds. (In reply to Nuno Teixeira from comment #1) This has nothing to do with GLES/compositing. The backtrace shows ICU, which is built with clang/libc++, clashing with libstdc++ that JavaScriptCore is built with. In fact, any execution path from any part of webkitgtk that uses the dynamic link to a C++ dependency built with clang/libc++ will exhibit the same type of crash. Mangling libc++ and libstdc++ doesn't really work by design unfortunately. > https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/String.html
This works on Nyxt with WebKit built with clang indeed. I hope FreeBSD will not go back to GCC because of WebKit ;)
(In reply to shamaz.mazum from comment #3) Do you need this patch ? % diff -u __config.orig __config --- __config.orig 2025-01-29 16:00:23.966673000 +0800 +++ __config 2025-01-29 16:00:57.100693000 +0800 @@ -190,9 +190,9 @@ # endif // Feature macros for disabling pre ABI v1 features. All of these options // are deprecated. -//# if defined(__FreeBSD__) -//# define _LIBCPP_DEPRECATED_ABI_DISABLE_PAIR_TRIVIAL_COPY_CTOR -//# endif +# if defined(__FreeBSD__) +# define _LIBCPP_DEPRECATED_ABI_DISABLE_PAIR_TRIVIAL_COPY_CTOR +# endif // For XCOFF linkers, we have problems if we see a weak hidden version of a symbol // in user code (like you get with -fvisibility-inlines-hidden) and then a strong def // in the library, so we need to always rely on the library version. (In reply to huanghwh from comment #4) Yes. I just want to state that this is probably GCC-clang interop problem (I assume the rest of your system is built with clang. Correct me if I am wrong). (In reply to shamaz.mazum from comment #5) OK, built with system's clang and patched with __config, no WebProcess CRASHED anymore. MiniBrowser works normally. (In reply to huanghwh from comment #6) So fast? That patch implies that all other ports in the system are rebuild with patched clang. It produces ABI-incompatible libcxx, so that's why it is not simply accepted upstream. I'll see if WebKit can be rebuild with clang without that __config patch. Maybe it's possible to patch WebKit itself. (In reply to shamaz.mazum from comment #8) Now I'm confused, did you check last 2 comments on https://reviews.freebsd.org/D35327 ? (In reply to Nuno Teixeira from comment #9) Wait, there is no # define _LIBCPP_DEPRECATED_ABI_DISABLE_PAIR_TRIVIAL_COPY_CTOR in /usr/src/contrib/llvm-project/libcxx/include/__config on stable/14. Does it mean that clang hacks won't be necessary on FreeBSD 14.3? When why building WebKit with GCC? Maybe just wait for FreeBSD 14.3 and try with clang? I was told that _LIBCPP_DEPRECATED_ABI_DISABLE_PAIR_TRIVIAL_COPY_CTOR will not be going anywhere anytime soon. (In reply to shamaz.mazum from comment #10) Well, I will start with comment #6 on main :) (In reply to Nuno Teixeira from comment #11) (...) Building: PORTNAME= webkit DISTVERSION= 2.46.5 -PORTREVISION= 1 +PORTREVISION= 2 CATEGORIES= www MASTER_SITES= https://webkitgtk.org/releases/ PKGNAMESUFFIX= 2-gtk_${FLAVOR} @@ -52,7 +52,6 @@ USES= bison cmake compiler:c++23-lang cpe gettext gl gnome gperf \ USE_GNOME= cairo gdkpixbuf2 introspection:build libxml2 libxslt USE_GL= egl gbm gl glesv2 USE_LDCONFIG= yes -USE_GCC= yes If this goes fine, then I think that USE_GCC should be used *only* for os versions thats needs it. (In reply to Nuno Teixeira from comment #12) Can you test it on CURRENT or 14-STABLE? If not, I can volunteer to upgrade and test on 14-STABLE if drm-kmod works fine there. (In reply to shamaz.mazum from comment #13) In file included from /wrkdirs/usr/ports/www/webkit2-gtk/work-41/.build/WebCore/DerivedSources/unified-sources/UnifiedSource-3c72abbe-53.cpp:4: /wrkdirs/usr/ports/www/webkit2-gtk/work-41/webkitgtk-2.46.5/Source/WebCore/platform/graphics/gstreamer/MediaSampleGStreamer.cpp:128:12: error: call to implicitly-deleted copy constructor of 'PlatformSample' 128 | return sample; | ^~~~~~ /wrkdirs/usr/ports/www/webkit2-gtk/work-41/webkitgtk-2.46.5/Source/WebCore/platform/MediaSample.h:67:7: note: copy constructor of 'PlatformSample' is implicitly deleted because field 'sample' has a deleted copy constructor 67 | } sample; | ^ /wrkdirs/usr/ports/www/webkit2-gtk/work-41/webkitgtk-2.46.5/Source/WebCore/platform/MediaSample.h:66:83: note: copy constructor of '(unnamed union at /wrkdirs/usr/ports/www/webkit2-gtk/work-41/webkitgtk-2.46.5/Source/WebCore/platform/MediaSample.h:62:5)' is implicitly deleted because variant field 'byteRangeSample' has a non-trivial copy constructor 66 | std::pair<MTPluginByteSourceRef, std::reference_wrapper<const TrackInfo>> byteRangeSample; | ^ 1 error generated. (In reply to Nuno Teixeira from comment #14) (...) main-n275036-faa845aab611 (Jan 25) (In reply to Nuno Teixeira from comment #15) (...) Without patching anything. Is there a need to define _LIBCPP_DEPRECATED_ABI_DISABLE_PAIR_TRIVIAL_COPY_CTOR since it's not there? (In reply to shamaz.mazum from comment #13) It will be great if someone do tests on 14-STABLE too. I can deal with main. (In reply to Nuno Teixeira from comment #16) > Is there a need to define _LIBCPP_DEPRECATED_ABI_DISABLE_PAIR_TRIVIAL_COPY_CTOR since it's not there? No. It's imperative that pair copy constructor is trivial. I don't know C++, but I think it means that it's not explicitly provided. I checked main. That definition is simply moved to contrib/llvm-project/libcxx/include/__configuration/abi.h So nothing is changed, really. I'll try to patch WebKit according to these two options: a) Use non-standard pair with trivial copy constructor b) Add explicit copy constructor to PlatfromSample. If any of those works, I'll let you know. Otherwise I am out of ideas. (In reply to Nuno Teixeira from comment #17) Can you take www/webkit2-gtk from here: https://github.com/shamazmazum/freebsd-ports/tree/standard-llvm/www (note standard-llvm branch) Rebuild www/webkit2-gtk@41 and rebuild nyxt without makefile patch? I have added a trivial patch with allows to build webkit with standard (unpatched) LLVM from FreeBSD's base. I have also dropped some unused dependencies and added atexit(3) crash fix. This is what makes it work: https://github.com/shamazmazum/freebsd-ports/blob/standard-llvm/www/webkit2-gtk/files/patch-Source_WebCore_platform_MediaSample.h (In reply to shamaz.mazum from comment #19) > Can you take www/webkit2-gtk from here: > https://github.com/shamazmazum/freebsd-ports/tree/standard-llvm/www > (note standard-llvm branch) done >Rebuild www/webkit2-gtk@41 and rebuild nyxt without makefile patch? done > I have added a trivial patch with allows to build webkit with standard > (unpatched) LLVM from FreeBSD's base. I have also dropped some unused > dependencies and added atexit(3) crash fix. modified: Makefile Untracked files: files/patch-Source_WebCore_platform_MediaSample.h files/patch-Source_WebCore_platform_graphics_PlatformDisplay.cpp files/patch-Source_WebCore_platform_graphics_PlatformDisplay.h Building... (In reply to Nuno Teixeira from comment #20) (...) clang build: 0h 47m gcc build: 1h 50m nyxt run-test: - needed env WEBKIT_DISABLE_COMPOSITING_MODE=1 wirple.com: OK, benchmarks tests OK facebook.com: OK, login OK :) (In reply to shamaz.mazum from comment #19) I tried standard-llvm branch, MiniBrowser works normally! (In reply to Nuno Teixeira from comment #21) (...) 4.1 MiniBrowser: - needed env WEBKIT_DISABLE_COMPOSITING_MODE=1 /usr/local/libexec/webkit2gtk-4.1/MiniBrowser \ https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/String.html OK wirple.com: OK, benchs OK facebook.com: OK, login OK NOTE: In both nyxt and MiniBrowser I was able to copy password from firefox and pasted it in nyxt/Minibrowser by mouse menu. NOTE SELF: remove nyxt run dep on xclip and test (In reply to huanghwh from comment #22) > I tried standard-llvm branch, MiniBrowser works normally! Just for the record: - What freebsd os version you use? - are you using env WEBKIT_DISABLE_COMPOSITING_MODE=1 for normal browser operations? *** Bug 284445 has been marked as a duplicate of this bug. *** (In reply to shamaz.mazum from comment #19) Defining a pair type with copy constructor is a much better idea than what I had in mind, removing the union member entirely and dealing with the relevant PlatformSample uses after preprocessing. However, this looks like the libstdc++ implementation of std::pair, which I'm not easy about including here. Note that the original implementation simply reverted an upstream commit introduced for 2.34, which due to many code changes and reorganisations since then wouldn't apply anymore: https://cgit.freebsd.org/ports/tree/www/webkit2-gtk3/files/patch-revert-11ccaf183fad?id=6437b155725993e93a3163d5f5ca56a2f3961f68 Discussion and possible fixes and such for topics other than C++ standard library clashing, like GLES/compositing, are out of scope for this PR. (In reply to Nuno Teixeira from comment #24) > - What freebsd os version you use? # uname -a FreeBSD mbxp.jopens.cn 14.2-RELEASE FreeBSD 14.2-RELEASE releng/14.2-n269506-c8918d6c7412 GENERIC amd64 > - are you using env WEBKIT_DISABLE_COMPOSITING_MODE=1 for normal browser operations? no (In reply to Nuno Teixeira from comment #1) (In reply to Charlie Li from comment #2) (also: huanghwh@gmail.com ) lang/gcc* for a modern enough * supports use of -stdlib=libc++ instead of the gcc default of -stdlib=libstdc++ . In other words, lang/gcc14 (for example) supports using the system's libc++ instead of the lang/gcc* 's libstdc++ . This gets rid of the problem of using contradictory implementations of the C++ standard library in the same process. So, if you can figure out how to have the relevant package(s) build using -stdlib=libc++ consistently with gcc, you have a chance of it building based on the system libc++ and mixing with other things better. (I've no clue if the implementation is generic with respect to the C++ standard library vs. being tied to a specific implementation.) In some cases, this might be easier than switching from lang/gcc* to system clang (or to a devel/llvm* clang). At least something to explore to see if it helps. (In reply to Charlie Li from comment #26) > However, this looks like the libstdc++ implementation of std::pair, which I'm not easy about including here. I just copy-pasted it from the web. I dunno you can retype it, remove comments, change variable names and get new code without GPL restrictions (if you are afraid of those). These ten lines of code are so simple that every reimplementation of std::pair will look like GPL licensed code from GNU libstdc++. You can also write explicit copy constructor PlatformSample if you wish. This job is also trivial. Building with GCC is not an option, as I see it. (In reply to Charlie Li from comment #26) You can simply use template<class _T1, class _T2> struct mypair { _T1 x; _T2 y; mypair() : x(), y() { } mypair(const _T1& __a, const _T2& __b) : x(__a), y(__b) { } }; (In reply to shamaz.mazum from comment #31) BTW, Do we even need the last line? I'll check that This is enough: template<class _T1, class _T2> struct mypair { _T1 x; _T2 y; mypair() : x(), y() { } }; I updated my port here: https://github.com/shamazmazum/freebsd-ports/tree/standard-llvm/www/webkit2-gtk Also my port is based on Charlie Li's one and poudriere says: Warning: 'bin/WebKitWebDriver-4.1' is not stripped consider trying INSTALL_TARGET=install-strip or using ${STRIP_CMD} Warning: 'lib/libjavascriptcoregtk-4.1.so.0.6.14' is not stripped consider trying INSTALL_TARGET=install-strip or using ${STRIP_CMD} Warning: 'lib/webkit2gtk-4.1/injected-bundle/libwebkit2gtkinjectedbundle.so' is not stripped consider trying INSTALL_TARGET=install-strip or using ${STRIP_CMD} Warning: 'lib/libwebkit2gtk-4.1.so.0.16.7' is not stripped consider trying INSTALL_TARGET=install-strip or using ${STRIP_CMD} Warning: 'libexec/webkit2gtk-4.1/jsc' is not stripped consider trying INSTALL_TARGET=install-strip or using ${STRIP_CMD} Warning: 'libexec/webkit2gtk-4.1/MiniBrowser' is not stripped consider trying INSTALL_TARGET=install-strip or using ${STRIP_CMD} Warning: 'libexec/webkit2gtk-4.1/WebKitWebProcess' is not stripped consider trying INSTALL_TARGET=install-strip or using ${STRIP_CMD} Warning: 'libexec/webkit2gtk-4.1/WebKitNetworkProcess' is not stripped consider trying INSTALL_TARGET=install-strip or using ${STRIP_CMD} I think this should be fixed. Just a quick report. Patch attached at Bug284445 helped for crashes of graphics/atril on startup for me. stable/14, amd64 at commit b40ca26721d70c76a4a4953ebe90b27daf0b3d58. Would try MFC'ing commit 6ee34bca48a9e0867d46b24a78855e225d46ddda and commit 21502f9a926c7e0c24ce230bb029fde4bf570a14 once I could take enough time to try, with and without the patch at Bug284445. Thanks in advance! (In reply to shamaz.mazum from comment #33) Look like nobody use "byteRangeSample", so can totally remove it: # cat patch-Source_WebCore_platform_MediaSample.h --- Source/WebCore/platform/MediaSample.h.orig 2024-08-19 06:28:39 UTC +++ Source/WebCore/platform/MediaSample.h @@ -63,7 +63,6 @@ struct PlatformSample { const MockSampleBox* mockSampleBox; CMSampleBufferRef cmSampleBuffer; GstSample* gstSample; - std::pair<MTPluginByteSourceRef, std::reference_wrapper<const TrackInfo>> byteRangeSample; } sample; }; (In reply to huanghwh from comment #36) Haha, indeed! Good catch! Turns out upstream removed this stuff entirely in main, which looks like a deprecated plugin intended mainly for macOS. This removal should be present in the upcoming 2.48 (currently 2.47). Once I finish some test builds and runtime (with 2.46) I will commit. A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=1eeaaa8e063450d0eb8f08df33a6ba338c348825 commit 1eeaaa8e063450d0eb8f08df33a6ba338c348825 Author: Charlie Li <vishwin@FreeBSD.org> AuthorDate: 2025-02-01 01:58:37 +0000 Commit: Charlie Li <vishwin@FreeBSD.org> CommitDate: 2025-02-01 01:58:37 +0000 www/webkit2-gtk: remove byteRangeSample from PlatformSample The presence of this union member causes the build to fail with our libc++ because version 1 ABI's std::pair does not have a trivial copy constructor (see D35327). However this code is only used for a deprecated plugin only for macOS, and it along with supporting code like this have been removed in the main branch upstream, so this should hold in the upcoming 2.48 series. For now, only remove byteRangeSample as the complete upstream commit does not apply. This allows the port to return to using the base system toolchain, including libc++, fixing crashes from mangling libc++-built dependencies with libstdc++ webkitgtk. While here, remove some unused USE_X11 depends to save another PORTREVISION bump. Reference: https://github.com/WebKit/WebKit/commit/d0527fca8fc20cdda907dfdc293323d7283bd262 PR: 284378 Reported by: huanghwh[at]gmail[dot]com Tested by: eduardo, shamaz.mazum[at]gmail[dot]com (previous iterations) www/webkit2-gtk/Makefile | 5 ++--- .../patch-Source_WebCore_platform_MediaSample.h (new) | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) (In reply to Charlie Li from comment #38) Can you also check if WebKitWebProcess crashes when you close a tab (in any buffer)? I have a crash in one of atexit handlers so I use a workaround: https://github.com/shamazmazum/freebsd-ports/commit/55636ca19a5bfb9665f6f24146335306f36d706d Does it happen for you? I think it may be related to https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=240761 |