RESOLVED FIXED 114901
[GTK] Plugin process broken due to a missing symbol
https://bugs.webkit.org/show_bug.cgi?id=114901
Summary [GTK] Plugin process broken due to a missing symbol
Carlos Garcia Campos
Reported 2013-04-20 07:03:02 PDT
Programs/WebKitPluginProcess /usr/lib/mozilla/plugins/flashplugin-alternative.so WebKitBuild/Release/Programs/.libs/lt-WebKitPluginProcess: symbol lookup error: WebKitBuild/Release/Programs/.libs/lt-WebKitPluginProcess: undefined symbol: _ZTVN7WebCore14DelayDSPKernelE I guess it only happens with web-audio enabled. I don't know when it started because unit tests are broken in the bots and I didn't know it.
Attachments
WIP (10.49 KB, patch)
2013-04-22 01:39 PDT, Zan Dobersek
no flags
Patch (5.54 KB, patch)
2013-04-25 02:00 PDT, Zan Dobersek
no flags
Provisional updated patch (6.16 KB, patch)
2013-04-25 03:05 PDT, Zan Dobersek
no flags
Patch (5.85 KB, patch)
2013-04-25 10:13 PDT, Zan Dobersek
no flags
Zan Dobersek
Comment 1 2013-04-20 07:32:47 PDT
Will fix.
Carlos Garcia Campos
Comment 2 2013-04-20 10:02:11 PDT
Building without web-audio I still get undefined symbols: Programs/WebKitPluginProcess /usr/lib/mozilla/plugins/flashplugin-alternative.so WebKitBuild/Release/Programs/.libs/lt-WebKitPluginProcess: symbol lookup error: WebKitBuild/Release/Programs/.libs/lt-WebKitPluginProcess: undefined symbol: _ZN7WebCore21WebKitFontFamilyNames14standardFamilyE Have you guys changed the libraries recently? I'm quite sure I tested it when I added libWebKit2Platform.
Zan Dobersek
Comment 3 2013-04-21 07:57:51 PDT
No, no such changes were made recently. The following build is the first occurrence of this problem (the crashing WebKitPluginProcess due to undefined symbols): http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release/builds/36869 That makes this revision the most likely suspect: http://trac.webkit.org/changeset/148519 Nothing certain yet though, might be some earlier commit that only started causing problems from this build onwards (which would be messed up, really).
Carlos Garcia Campos
Comment 4 2013-04-21 08:30:40 PDT
(In reply to comment #3) > No, no such changes were made recently. > > The following build is the first occurrence of this problem (the crashing WebKitPluginProcess due to undefined symbols): > http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release/builds/36869 > > That makes this revision the most likely suspect: > http://trac.webkit.org/changeset/148519 > > Nothing certain yet though, might be some earlier commit that only started causing problems from this build onwards (which would be messed up, really). Indeed, reverting r148519 fixes the issue, thanks for looking at it, not let's see why.
Carlos Garcia Campos
Comment 5 2013-04-21 09:46:54 PDT
(In reply to comment #4) > (In reply to comment #3) > > No, no such changes were made recently. > > > > The following build is the first occurrence of this problem (the crashing WebKitPluginProcess due to undefined symbols): > > http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release/builds/36869 > > > > That makes this revision the most likely suspect: > > http://trac.webkit.org/changeset/148519 > > > > Nothing certain yet though, might be some earlier commit that only started causing problems from this build onwards (which would be messed up, really). > > Indeed, reverting r148519 fixes the issue, thanks for looking at it, not let's see why. I don't understand why, but simply using ScriptController (not necessarily to initialize threading, but any use of ScriptController) makes the symbols to be defined in the binary. I've tried moving the order of the libraries in Programs_WebKitPluginProcess_LDADD like Gustavo did when we had similar issues while splitting SVG, but I haven't managed to make it work :-(
Zan Dobersek
Comment 6 2013-04-22 01:21:15 PDT
I've invested some time yesterday evening looking at this and I believe the general problem here is trying to cram every required static library into the WebKitPluginProcess. The current situation itself shows that this isn't really working out, at least not with the current setup. I find the use of passing the '--unresolved-symbols=ignore-in-object-files' flag to the linker a bit problematic - I had to add multiple WK2 source files (like Shared/soup/PlatformCertificateInfo.*, UIProcess/Launcher/ProcessLauncher.*) to the build to remove some of the unresolved symbols. Again, this workaround is most likely in use due to throwing every static library that's required into the plugin process binary. I've worked around this problem by creating a new shared library, at the moment called libwebkit2gtk-pluginlib-3.0.la. It's built from the same sources as the plugin process was, except the PluginMainUnix.cpp which is built into the new plugin process binary. This setup very much resembles how the WebKitWebProcess is built. I'm not sure if having a separate shared library is a viable option, though. I'll upload the patch for reference and we can discuss it further.
Zan Dobersek
Comment 7 2013-04-22 01:39:57 PDT
Created attachment 199000 [details] WIP Just the raw WIP patch, showing the approach to setting up a new shared library that's used by the slimmed-down plugin process binary.
Martin Robinson
Comment 8 2013-04-22 08:21:30 PDT
(In reply to comment #6) > I've worked around this problem by creating a new shared library, at the moment called libwebkit2gtk-pluginlib-3.0.la. It's built from the same sources as the plugin process was, except the PluginMainUnix.cpp which is built into the new plugin process binary. This setup very much resembles how the WebKitWebProcess is built. I'm not sure I totally comprehend how linking is different if you compile many static libraries into a binary versus many static libraries into one giant static library and then that into a binary. In the past we've found that introducing more intermediary static libraries has slowed down the build and introduced problems.
Gustavo Noronha (kov)
Comment 9 2013-04-22 13:07:43 PDT
I don't understand it either, but it does indeed work, while linking the static libraries directly into the PluginProcess binary always causes unresolved symbols to show up... I also considered making a private 'pluginprocess' library to solve this in the past, I think it's a valid option for now tbh. I hope that problem will go away when we ditch libtool =/
Zan Dobersek
Comment 10 2013-04-25 02:00:15 PDT
Zan Dobersek
Comment 11 2013-04-25 02:21:27 PDT
(In reply to comment #10) > Created an attachment (id=199636) [details] > Patch Right, that fixes it. It works around _a lot_ of libtool problems. Carlos recommended using the ld's --start-group and --end-group flags. I've put quite some effort into these but couldn't get them to work. My guess is they are only functional for shared libraries, though I haven't investigated the problem enough to be sure. The required static libraries are listed up to three times, but in three different ways, just so libtool actually passess the complete and non-simplified list to the linker. Libtool provides the --preserve-dup-deps flag that should sort this out, but again, it doesn't. The suspicion I have is again that this only works for shared libraries. All in all this just shows the layer violations are obviously present and need fixing, but are rarely showing through and causing problems. Though, when eventually they do show up, the solution is an ugly workaround that no one would like to see, but I guess is still required.
Zan Dobersek
Comment 12 2013-04-25 02:50:55 PDT
Needs polishing. Still sticking behind the approach though.
Zan Dobersek
Comment 13 2013-04-25 03:05:00 PDT
Created attachment 199647 [details] Provisional updated patch
Zan Dobersek
Comment 14 2013-04-25 09:13:29 PDT
(In reply to comment #13) > Created an attachment (id=199647) [details] > Provisional updated patch The patch sets up a phony rule for the .libs/libPlatformGtk2.la and .libs/libWebCoreGtk2.la files. EWS still complains, but I've compiled clean builds with the patch successfully using both the ld and the gold linker, so I think clean builds will be required after the patch lands.
Zan Dobersek
Comment 15 2013-04-25 10:13:48 PDT
Zan Dobersek
Comment 16 2013-04-25 10:15:00 PDT
Turns out the EWS error was a valid one, should be fixed now.
Gustavo Noronha (kov)
Comment 17 2013-04-25 13:06:21 PDT
Comment on attachment 199680 [details] Patch I don't feel too dirty r+ing this.
Zan Dobersek
Comment 18 2013-04-25 23:48:57 PDT
Comment on attachment 199680 [details] Patch Clearing flags on attachment: 199680 Committed r149169: <http://trac.webkit.org/changeset/149169>
Zan Dobersek
Comment 19 2013-04-25 23:49:03 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.