Bug 153117 – [GTK OSX] libGObjectDOMBindings.a is not linked into libwebkit2gtk-4.0.37.dylib

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Jeremy Huddleston Sequoia]

As reported at https://trac.macports.org/ticket/50334

We configure using:

cmake -DCMAKE_INSTALL_PREFIX=/opt/local -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_COLOR_MAKEFILE=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_BUILD_WITH_INSTALL_RPATH=ON -DCMAKE_INSTALL_RPATH=/opt/local/lib -DCMAKE_INSTALL_NAME_DIR=/opt/local/lib -DCMAKE_SYSTEM_PREFIX_PATH="/opt/local;/usr" -DCMAKE_MODULE_PATH=/opt/local/share/cmake/Modules -DCMAKE_FIND_FRAMEWORK=LAST -Wno-dev -DPORT=GTK -DENABLE_X11_TARGET=ON -DENABLE_QUARTZ_TARGET=OFF -DENABLE_INTROSPECTION=OFF -DLLVM_CONFIG_EXE=/opt/local/bin/llvm-config-mp-3.7 -DENABLE_VIDEO=ON -DENABLE_PLUGIN_PROCESS_GTK2=ON -DCMAKE_C_FLAGS_RELEASE="-DNDEBUG" -DCMAKE_CXX_FLAGS_RELEASE="-DNDEBUG" -DCMAKE_OSX_ARCHITECTURES="x86_64" -DCMAKE_OSX_DEPLOYMENT_TARGET="10.11" -DCMAKE_OSX_SYSROOT="/"

And install libwebkit2gtk-4.0.37.13.0.dylib by copying it from the lib directory to workaround bug #152651.

The resulting libwebkit2gtk-4.0.37.13.0.dylib is missing some symbols, and dependent projects fail to link:

Undefined symbols for architecture x86_64:
  "_webkit_dom_document_get_elements_by_tag_name", referenced from:
      _ephy_web_overview_document_loaded in libephywebextension_la-ephy-web-overview.o
      _ephy_web_dom_utils_get_application_title in libephymisc.a(libephymisc_la-ephy-web-dom-utils.o)
      _ephy_web_dom_utils_get_best_icon in libephymisc.a(libephymisc_la-ephy-web-dom-utils.o)
  "_webkit_dom_dom_window_webkit_message_handlers_post_message", referenced from:
      _request_decision_on_storing in libephywebextension_la-ephy-web-extension.o
  "_webkit_dom_html_input_element_is_edited", referenced from:
      _ephy_web_dom_utils_has_modified_forms in libephymisc.a(libephymisc_la-ephy-web-dom-utils.o)
  "_webkit_dom_html_text_area_element_is_edited", referenced from:
      _ephy_web_dom_utils_has_modified_forms in libephymisc.a(libephymisc_la-ephy-web-dom-utils.o)

---

$ nm /opt/local/lib/libwebkit2gtk-4.0.dylib | egrep '(_webkit_dom_document_get_elements_by_tag_name|_webkit_dom_dom_window_webkit_message_handlers_post_message|_webkit_dom_html_input_element_is_edited|_webkit_dom_html_text_area_element_is_edited)'
0000000000d68f42 T _webkit_dom_document_get_elements_by_tag_name_as_html_collection
0000000000d6968f T _webkit_dom_document_get_elements_by_tag_name_ns_as_html_collection

[Jeremy Huddleston Sequoia]

Created attachment 269033 [details]
full build log

[Jeremy Huddleston Sequoia]

Those symbols are found in:

Source/WebCore/bindings/gobject/WebKitDOMDeprecated.cpp
Source/WebCore/bindings/gobject/WebKitDOMCustom.cpp

which are linked into libGObjectDOMBindings.a, which is not linked into libwebkit2gtk-4.0.37.13.0.dylib

[Jeremy Huddleston Sequoia]

Created attachment 269035 [details]
build log

[Jeremy Huddleston Sequoia]

Actually, libGObjectDOMBindings.a is on the link line, but we're missing -all_load.

[Jeremy Huddleston Sequoia]

This was caused by an incorrect fix for bug #144557

[Jeremy Huddleston Sequoia]

Created attachment 269043 [details]
patch to add -Wl,-all_load

[Michael Catanzaro]

Comment on attachment 269043 [details]
patch to add -Wl,-all_load

View in context: https://bugs.webkit.org/attachment.cgi?id=269043&action=review

Thanks for your patch! In the future, when you want a review for your patch, please set the r? flag for your patch. If you want us to commit your patch (which you do if you don't have committer access), set the cq? flag. You'll want to set cq? after answering this question:

> Source/cmake/OptionsGTK.cmake:477
> +        list(APPEND ${_list_name} -Wl,-all_load)

Should this be -Wl,-all_load ${library} -Wl,-no-all_load, or does it not work the same as --whole-archive?

[Jeremy Huddleston Sequoia]

> Should this be -Wl,-all_load ${library} -Wl,-no-all_load

No.

> or does it not work the same as --whole-archive?

It does not.  There is nothing exactly analogous.  There are two similar options in ld64:

-all_load
Loads all members of static archive libraries.

-force_load path_to_archive
Loads all members of the specified static archive library.  Note: -all_load forces all members
of all archives to be loaded.  This option allows you to target a specific archive.


-force_load would be ideal if we had a contract that the arguments to the macro were always static archives that we built for inclusion in the library.  I tried that first, and it didn't work because the macro is called with arguments other than just static archives that we had built.  -all_load is thus the ideal choice, and it's also used elsewhere in the build already.

[Philip Chimento]

Do -all_load and/or -force_load work for you? For me, using either one did allow the build to complete, but MiniBrowser segfaulted on startup (see https://bugs.webkit.org/show_bug.cgi?id=144557#c1)

[Jeremy Huddleston Sequoia]

yes, they work for me.  They do what they are supposed to do and symbols are included in the library.

Without this change, clients fail to link due to the missing symbols.

With this change in place, clients are able to link, and we're seeing crashes at launch (https://trac.macports.org/ticket/50339) which we are starting to look at now.  I suspect your crash is likely the same as that one.

[Michael Catanzaro]

Hm, I just found this bit of code in Source/WebKit2/PlatformGTK.cmake:

# Manually add some libraries on OSX because we don't have the --whole-archive flag
if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
    set(INTROSPECTION_ADDITIONAL_LIBRARIES --library=c++)
    set(INTROSPECTION_ADDITIONAL_LDFLAGS -lGObjectDOMBindings)
endif ()

Is that still needed?

[Philip Chimento]

Probably it can be removed if -all_load works as expected.

[Jeremy Huddleston Sequoia]

Yeah, I saw that as well.  I believe that is specific to the gobject-introspection code, but that is currently not working on OSX anyways.  We've disabled it in our (MacPorts) build.  See bug #152650.

[WebKit Commit Bot]

Comment on attachment 269043 [details]
patch to add -Wl,-all_load

Rejecting attachment 269043 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 269043, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
153117.
Found no modified ChangeLogs, cannot create a commit message.
All changes require a ChangeLog.  See:
 http://webkit.org/coding/contributing.html
Found no modified ChangeLogs, cannot create a commit message.
All changes require a ChangeLog.  See:
 http://webkit.org/coding/contributing.html
Found no modified ChangeLogs, cannot create a commit message.
All changes require a ChangeLog.  See:
 http://webkit.org/coding/contributing.html
Updating OpenSource
Current branch master is up to date.

Full output: http://webkit-queues.webkit.org/results/696742

[Jeremy Huddleston Sequoia]

Created attachment 269157 [details]
Updated patch with ChangeLog entry

[Jeremy Huddleston Sequoia]

Created attachment 269159 [details]
Updated patch to also include feedback for removal of now-irrelevant build hack for gobject-intorspection

[Michael Catanzaro]

Comment on attachment 269159 [details]
Updated patch to also include feedback for removal of now-irrelevant build hack for gobject-intorspection

View in context: https://bugs.webkit.org/attachment.cgi?id=269159&action=review

> Source/WebKit2/ChangeLog:7
> +        Reviewed by Philip Chimento.

Philip is not a reviewer; this line has official significance, so please don't list people here unless they set the r+ flag in Bugzilla.

> Source/cmake/OptionsGTK.cmake:475
>  macro(ADD_WHOLE_ARCHIVE_TO_LIBRARIES _list_name)

It occurs to me that this macro does not belong in OptionsGTK.cmake as it has nothing to do with build options. Once we manage to land this patch, I will move it in a follow-up patch.

[Jeremy Huddleston Sequoia]

Created attachment 269168 [details]
Updated reviewers in patch

[WebKit Commit Bot]

Comment on attachment 269168 [details]
Updated reviewers in patch

Clearing flags on attachment: 269168

Committed r195174: <http://trac.webkit.org/changeset/195174>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.