Changeset 226266 in webkit


Ignore:
Timestamp:
Dec 22, 2017 9:18:43 AM (6 years ago)
Author:
Michael Catanzaro
Message:

[GTK] Duplicated symbols in libjavascriptcoregtk and libwebkit2gtk can cause crashes in production builds
https://bugs.webkit.org/show_bug.cgi?id=179914

Reviewed by Carlos Garcia Campos.

.:

  • CMakeLists.txt:
  • Source/cmake/OptionsGTK.cmake:
  • Source/cmake/OptionsJSCOnly.cmake:
  • Source/cmake/OptionsMac.cmake:
  • Source/cmake/OptionsWPE.cmake:
  • Source/cmake/OptionsWin.cmake:
  • Source/cmake/WebKitCompilerFlags.cmake:
  • Source/cmake/wpesymbols.filter: Removed.

Source/JavaScriptCore:

Add a new JavaScriptCoreGTK build target, to build JSC as a shared library. Link the
original JavaScriptCore build target, which is now a static library, to it. Use
--whole-archive to prevent all the JavaScriptCore symbols from being dropped, since none are
used directly by JavaScriptCoreGTK.

The installed libjavascriptcoregtk-4.0 now corresponds to the JavaScriptCoreGTK target,
instead of the JavaScriptCore target. There is almost no difference on the installed system,
except that we now use a version script when linking, to hide private symbols, since they're
no longer needed by libwebkit2gtk-4.0.so.

Also, move the symbols map here.

  • PlatformGTK.cmake:
  • javascriptcoregtk-symbols.map: Added.

Source/WebCore:

  • CMakeLists.txt: Test for WebCore_LIBRARY_TYPE rather than SHARED_CORE.

Source/WebKit:

Mark a few InjectedBundle symbols with default visibility, so they don't get hidden by
-fvisibility=hidden. Also, remove Windows conditionals, since Windows is not supported by
any GLib ports.

Also, move the symbols map to here, and share it between WPE and GTK.

  • CMakeLists.txt:
  • PlatformGTK.cmake:
  • PlatformWPE.cmake:
  • WebProcess/InjectedBundle/API/glib/WebKitExtensionManager.h:
  • WebProcess/InjectedBundle/API/glib/WebKitInjectedBundleMain.cpp:
  • webkitglib-symbols.map: Renamed from Source/cmake/gtksymbols.filter.
Location:
trunk
Files:
1 added
1 deleted
18 edited
1 moved

Legend:

Unmodified
Added
Removed
  • trunk/CMakeLists.txt

    r225168 r226266  
    122122# Default library types
    123123# -----------------------------------------------------------------------------
    124 option(SHARED_CORE "build WebCore as a shared library")
    125 
    126 if (SHARED_CORE)
    127     set(WebCore_LIBRARY_TYPE SHARED)
    128 else ()
    129     set(WebCore_LIBRARY_TYPE STATIC)
    130 endif ()
    131 
     124# By default, only the highest-level libraries, WebKitLegacy and WebKit, are
     125# shared, because properly building shared libraries that depend on each other
     126# can be tricky. Override these in Options*.cmake for your port as needed.
    132127set(WTF_LIBRARY_TYPE STATIC)
    133 set(JavaScriptCore_LIBRARY_TYPE SHARED)
     128set(JavaScriptCore_LIBRARY_TYPE STATIC)
    134129set(PAL_LIBRARY_TYPE STATIC)
     130set(WebCore_LIBRARY_TYPE STATIC)
    135131set(WebKitLegacy_LIBRARY_TYPE SHARED)
    136132set(WebKit_LIBRARY_TYPE SHARED)
  • trunk/ChangeLog

    r226212 r226266  
     12017-12-22  Michael Catanzaro  <mcatanzaro@igalia.com>
     2
     3        [GTK] Duplicated symbols in libjavascriptcoregtk and libwebkit2gtk can cause crashes in production builds
     4        https://bugs.webkit.org/show_bug.cgi?id=179914
     5
     6        Reviewed by Carlos Garcia Campos.
     7
     8        * CMakeLists.txt:
     9        * Source/cmake/OptionsGTK.cmake:
     10        * Source/cmake/OptionsJSCOnly.cmake:
     11        * Source/cmake/OptionsMac.cmake:
     12        * Source/cmake/OptionsWPE.cmake:
     13        * Source/cmake/OptionsWin.cmake:
     14        * Source/cmake/WebKitCompilerFlags.cmake:
     15        * Source/cmake/wpesymbols.filter: Removed.
     16
     172017-12-22  Michael Catanzaro  <mcatanzaro@igalia.com>
     18
     19        [GTK] Duplicated symbols in libjavascriptcoregtk and libwebkit2gtk can cause crashes in production builds
     20        https://bugs.webkit.org/show_bug.cgi?id=179914
     21
     22        Reviewed by Carlos Garcia Campos.
     23
     24        Let's build JSC as a static library, and link that static lib to *both* our shared
     25        libjavascriptcoregtk and libwebkit2gtk. Then we can fix this and also filter out all the
     26        private symbols that we're currently exposing in libjavascriptcoregtk, which wouldn't be
     27        possible otherwise. The cost of this is disk space. I think this trade-off is reasonable,
     28        because it's the best way I could think of that accomplishes all our goals: (a) install two
     29        shared libs, (b) export only public API symbols, (c) does not require any linker hacks.
     30
     31        Additionally, build with -fvisibility=hidden so that the compiler knows that many symbols
     32        will be stripped out. This should improve code generation. It's actually how WPE was
     33        previously compiled, but I removed this when I added the version script for WPE, because I
     34        thought it was redundant with the version script. It is not, and we should use both,
     35        according to Ulrich Drepper's "How to Write Shared Libraries." We will use
     36        -fvisibility=hidden on all ports; this should be fine, as long as export macros are used
     37        where needed. This is actually a totally separate change, but it makes sense to do it now if
     38        we consider this bug a catch-all "fix how we link WebKit" issue.
     39
     40        * CMakeLists.txt: Rejigger the default library types, and remove the SHARED_CORE option,
     41          which is not likely to work properly in ports that are not expecting it. These changes are
     42          only mildly-related and certainly not required, but it makes sense to clean them up now.
     43        * Source/cmake/OptionsGTK.cmake: Don't set the version script here.
     44        * Source/cmake/OptionsJSCOnly.cmake: Adjust to changes in default library types.
     45        * Source/cmake/OptionsMac.cmake: Adjust to changes in default library types. Override the
     46          library type variables only when required.
     47        * Source/cmake/OptionsWPE.cmake: Overriding the library type variables is no longer
     48          required. Also, don't set the version script here.
     49        * Source/cmake/OptionsWin.cmake: Adjust to changes in default library types. Override the
     50          library type variables only when required.
     51        * Source/cmake/WebKitCompilerFlags.cmake: Build with -fvisibility=hidden,
     52          -fvisibility-inlines-hidden, and -Wno-attributes.
     53        * Source/cmake/wpesymbols.filter: Removed.
     54
    1552017-12-20  Don Olmstead  <don.olmstead@sony.com>
    256
  • trunk/Source/JavaScriptCore/ChangeLog

    r226261 r226266  
     12017-12-22  Michael Catanzaro  <mcatanzaro@igalia.com>
     2
     3        [GTK] Duplicated symbols in libjavascriptcoregtk and libwebkit2gtk can cause crashes in production builds
     4        https://bugs.webkit.org/show_bug.cgi?id=179914
     5
     6        Reviewed by Carlos Garcia Campos.
     7
     8        Add a new JavaScriptCoreGTK build target, to build JSC as a shared library. Link the
     9        original JavaScriptCore build target, which is now a static library, to it. Use
     10        --whole-archive to prevent all the JavaScriptCore symbols from being dropped, since none are
     11        used directly by JavaScriptCoreGTK.
     12
     13        The installed libjavascriptcoregtk-4.0 now corresponds to the JavaScriptCoreGTK target,
     14        instead of the JavaScriptCore target. There is almost no difference on the installed system,
     15        except that we now use a version script when linking, to hide private symbols, since they're
     16        no longer needed by libwebkit2gtk-4.0.so.
     17
     18        Also, move the symbols map here.
     19
     20        * PlatformGTK.cmake:
     21        * javascriptcoregtk-symbols.map: Added.
     22
    1232017-12-22  Yusuke Suzuki  <utatane.tea@gmail.com>
    224
  • trunk/Source/JavaScriptCore/PlatformGTK.cmake

    r223621 r226266  
    1 set(JavaScriptCore_OUTPUT_NAME javascriptcoregtk-${WEBKITGTK_API_VERSION})
    2 
    31list(APPEND JavaScriptCore_UNIFIED_SOURCE_LIST_FILES
    42    "SourcesGTK.txt"
     
    5351    ${GLIB_INCLUDE_DIRS}
    5452)
     53
     54# Linking WebKit properly is extremely tricky. We need to build both a static library
     55# and a shared library for JSC. See https://bugs.webkit.org/show_bug.cgi?id=179914.
     56set(JavaScriptCoreGTK_LIBRARIES
     57    JavaScriptCore${DEBUG_SUFFIX}
     58)
     59ADD_WHOLE_ARCHIVE_TO_LIBRARIES(JavaScriptCoreGTK_LIBRARIES)
     60
     61add_library(JavaScriptCoreGTK SHARED "${CMAKE_BINARY_DIR}/cmakeconfig.h")
     62target_link_libraries(JavaScriptCoreGTK ${JavaScriptCoreGTK_LIBRARIES})
     63set_target_properties(JavaScriptCoreGTK PROPERTIES OUTPUT_NAME javascriptcoregtk-${WEBKITGTK_API_VERSION})
     64
     65WEBKIT_POPULATE_LIBRARY_VERSION(JAVASCRIPTCORE)
     66set_target_properties(JavaScriptCoreGTK PROPERTIES VERSION ${JAVASCRIPTCORE_VERSION} SOVERSION ${JAVASCRIPTCORE_VERSION_MAJOR})
     67
     68if (DEVELOPER_MODE)
     69    WEBKIT_ADD_TARGET_PROPERTIES(JavaScriptCoreGTK LINK_FLAGS "-Wl,--version-script,${CMAKE_CURRENT_SOURCE_DIR}/javascriptcoregtk-symbols.map")
     70endif ()
     71
     72install(TARGETS JavaScriptCoreGTK DESTINATION "${LIB_INSTALL_DIR}")
  • trunk/Source/WebCore/CMakeLists.txt

    r226245 r226266  
    19941994target_link_libraries(WebCore ${WebCore_LIBRARIES})
    19951995
    1996 if (SHARED_CORE)
     1996if (${WebCore_LIBRARY_TYPE} MATCHES "SHARED")
    19971997    set_target_properties(WebCore PROPERTIES VERSION ${PROJECT_VERSION} SOVERSION ${PROJECT_VERSION_MAJOR})
    19981998    install(TARGETS WebCore DESTINATION "${LIB_INSTALL_DIR}")
  • trunk/Source/WebCore/ChangeLog

    r226265 r226266  
     12017-12-22  Michael Catanzaro  <mcatanzaro@igalia.com>
     2
     3        [GTK] Duplicated symbols in libjavascriptcoregtk and libwebkit2gtk can cause crashes in production builds
     4        https://bugs.webkit.org/show_bug.cgi?id=179914
     5
     6        Reviewed by Carlos Garcia Campos.
     7
     8        * CMakeLists.txt: Test for WebCore_LIBRARY_TYPE rather than SHARED_CORE.
     9
    1102017-12-22  Zalan Bujtas  <zalan@apple.com>
    211
  • trunk/Source/WebKit/CMakeLists.txt

    r225977 r226266  
    939939endif ()
    940940
    941 if (WebKit_VERSION_SCRIPT)
    942     WEBKIT_ADD_TARGET_PROPERTIES(WebKit LINK_FLAGS "${WebKit_VERSION_SCRIPT}")
    943 endif ()
    944 
    945941add_executable(WebProcess ${WebProcess_SOURCES})
    946942ADD_WEBKIT_PREFIX_HEADER(WebProcess)
  • trunk/Source/WebKit/ChangeLog

    r226264 r226266  
     12017-12-22  Michael Catanzaro  <mcatanzaro@igalia.com>
     2
     3        [GTK] Duplicated symbols in libjavascriptcoregtk and libwebkit2gtk can cause crashes in production builds
     4        https://bugs.webkit.org/show_bug.cgi?id=179914
     5
     6        Reviewed by Carlos Garcia Campos.
     7
     8        Mark a few InjectedBundle symbols with default visibility, so they don't get hidden by
     9        -fvisibility=hidden. Also, remove Windows conditionals, since Windows is not supported by
     10        any GLib ports.
     11
     12        Also, move the symbols map to here, and share it between WPE and GTK.
     13
     14        * CMakeLists.txt:
     15        * PlatformGTK.cmake:
     16        * PlatformWPE.cmake:
     17        * WebProcess/InjectedBundle/API/glib/WebKitExtensionManager.h:
     18        * WebProcess/InjectedBundle/API/glib/WebKitInjectedBundleMain.cpp:
     19        * webkitglib-symbols.map: Renamed from Source/cmake/gtksymbols.filter.
     20
    1212017-12-21  Michael Catanzaro  <mcatanzaro@igalia.com>
    222
  • trunk/Source/WebKit/PlatformGTK.cmake

    r225773 r226266  
    2424add_definitions(-DLIBDIR="${LIB_INSTALL_DIR}")
    2525add_definitions(-DDATADIR="${CMAKE_INSTALL_FULL_DATADIR}")
     26
     27if (DEVELOPER_MODE AND NOT CMAKE_SYSTEM_NAME MATCHES "Darwin")
     28    WEBKIT_ADD_TARGET_PROPERTIES(WebKit LINK_FLAGS "-Wl,--version-script,${CMAKE_CURRENT_SOURCE_DIR}/webkitglib-symbols.map")
     29endif ()
    2630
    2731# Temporary workaround to allow the build to succeed.
  • trunk/Source/WebKit/PlatformWPE.cmake

    r225773 r226266  
    1616add_definitions(-DLIBEXECDIR="${LIBEXEC_INSTALL_DIR}")
    1717add_definitions(-DLOCALEDIR="${CMAKE_INSTALL_FULL_LOCALEDIR}")
     18
     19if (DEVELOPER_MODE AND NOT CMAKE_SYSTEM_NAME MATCHES "Darwin")
     20    WEBKIT_ADD_TARGET_PROPERTIES(WebKit LINK_FLAGS "-Wl,--version-script,${CMAKE_CURRENT_SOURCE_DIR}/webkitglib-symbols.map")
     21endif ()
    1822
    1923# Temporary workaround to allow the build to succeed.
  • trunk/Source/WebKit/WebProcess/InjectedBundle/API/glib/WebKitExtensionManager.h

    r218628 r226266  
    3939    WTF_MAKE_NONCOPYABLE(WebKitExtensionManager);
    4040public:
    41     static WebKitExtensionManager& singleton();
     41    __attribute__((visibility("default"))) static WebKitExtensionManager& singleton();
    4242
    43     void initialize(InjectedBundle*, API::Object*);
     43    __attribute__((visibility("default"))) void initialize(InjectedBundle*, API::Object*);
    4444
    4545private:
  • trunk/Source/WebKit/WebProcess/InjectedBundle/API/glib/WebKitInjectedBundleMain.cpp

    r218628 r226266  
    2727using namespace WebKit;
    2828
    29 #if defined(WIN32) || defined(_WIN32)
    30 extern "C" __declspec(dllexport)
    31 #else
    32 extern "C"
    33 #endif
     29extern "C" __attribute__((visibility("default")))
    3430void WKBundleInitialize(WKBundleRef bundle, WKTypeRef userData)
    3531{
  • trunk/Source/WebKit/webkitglib-symbols.map

    r226265 r226266  
    66  PluginProcessMainUnix;
    77  StorageProcessMainUnix;
    8   _ZN6WebKit22WebKitExtensionManager10initializeEPNS_14InjectedBundleEPN3API6ObjectE;
    9   _ZN6WebKit22WebKitExtensionManager9singletonEv;
     8  extern "C++" {
     9    "WebKit::WebKitExtensionManager::singleton()";
     10    "WebKit::WebKitExtensionManager::initialize(WebKit::InjectedBundle*, API::Object*)";
     11  };
    1012local:
    1113  webkit_media_player_debug;
  • trunk/Source/cmake/OptionsGTK.cmake

    r225841 r226266  
    107107    WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_MINIBROWSER PUBLIC OFF)
    108108    WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_API_TESTS PRIVATE OFF)
    109     if (NOT CMAKE_SYSTEM_NAME MATCHES "Darwin")
    110         set(WebKit_VERSION_SCRIPT "-Wl,--version-script,${CMAKE_MODULE_PATH}/gtksymbols.filter")
    111     endif ()
    112109endif ()
    113110
  • trunk/Source/cmake/OptionsJSCOnly.cmake

    r225105 r226266  
    4949# FIXME: JSCOnly on WIN32 seems to only work with fully static build
    5050# https://bugs.webkit.org/show_bug.cgi?id=172862
    51 if (ENABLE_STATIC_JSC OR WIN32)
    52     set(JavaScriptCore_LIBRARY_TYPE STATIC)
     51if (NOT ENABLE_STATIC_JSC AND NOT WIN32)
     52    set(JavaScriptCore_LIBRARY_TYPE SHARED)
    5353endif ()
    5454
  • trunk/Source/cmake/OptionsMac.cmake

    r226189 r226266  
    5858set(ENABLE_WEBKIT ON)
    5959
     60set(JavaScriptCore_LIBRARY_TYPE SHARED)
    6061set(WebCore_LIBRARY_TYPE SHARED)
    6162set(WebCoreTestSupport_LIBRARY_TYPE SHARED)
    62 set(WebKit_LIBRARY_TYPE SHARED)
    6363
    6464add_definitions(-DU_DISABLE_RENAMING=1 -DU_SHOW_CPLUSPLUS_API=0)
  • trunk/Source/cmake/OptionsWPE.cmake

    r225888 r226266  
    5050
    5151WEBKIT_OPTION_END()
    52 
    53 set(JavaScriptCore_LIBRARY_TYPE STATIC)
    54 set(WebCore_LIBRARY_TYPE STATIC)
    5552
    5653find_package(Cairo 1.10.2 REQUIRED)
     
    114111set(DERIVED_SOURCES_WPE_API_DIR ${DERIVED_SOURCES_WEBKIT_DIR}/wpe)
    115112
    116 if (NOT DEVELOPER_MODE)
    117     set(WebKit_VERSION_SCRIPT "-Wl,--version-script,${CMAKE_MODULE_PATH}/wpesymbols.filter")
    118 endif ()
    119 
    120113include(GStreamerChecks)
  • trunk/Source/cmake/OptionsWin.cmake

    r226212 r226266  
    142142set(JavaScriptCore_LIBRARY_TYPE SHARED)
    143143set(WTF_LIBRARY_TYPE SHARED)
    144 set(PAL_LIBRARY_TYPE STATIC)
    145 set(WebKit_LIBRARY_TYPE SHARED)
    146 set(WebKitLegacy_LIBRARY_TYPE SHARED)
    147144
    148145find_package(ICU REQUIRED)
  • trunk/Source/cmake/WebKitCompilerFlags.cmake

    r222494 r226266  
    106106                                       -fno-rtti)
    107107
     108        if (UNIX AND NOT DEVELOPER_MODE)
     109            # These are used even for ports that use symbol maps so that the
     110            # compiler can take visibility into account for code optimization.
     111            WEBKIT_APPEND_GLOBAL_COMPILER_FLAGS(-fvisibility=hidden)
     112            WEBKIT_APPEND_GLOBAL_CXX_FLAGS(-fvisibility-inlines-hidden)
     113        endif ()
     114
    108115        if (WIN32)
    109116            WEBKIT_APPEND_GLOBAL_COMPILER_FLAGS(-mno-ms-bitfields)
     
    129136                                         -Wno-noexcept-type
    130137                                         -Wno-parentheses-equality)
     138
     139    # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80947
     140    if (${CMAKE_CXX_COMPILER_VERSION} VERSION_LESS "8.0" AND NOT CMAKE_CXX_COMPILER_ID MATCHES "Clang")
     141        WEBKIT_PREPEND_GLOBAL_CXX_FLAGS(-Wno-attributes)
     142    endif ()
    131143endif ()
    132144
Note: See TracChangeset for help on using the changeset viewer.