Changeset 274166 in webkit

Timestamp:

Mar 9, 2021, 12:11:42 PM (6 years ago)

Author:

commit-queue@webkit.org

Message:

.:
[GTK] Reenable -fvisibility=hidden
​https://bugs.webkit.org/show_bug.cgi?id=181916

Patch by Michael Catanzaro <Michael Catanzaro> on 2021-03-09
Reviewed by Don Olmstead.

In non-DEVELOPER_MODE builds, we rely on a linker version script to hide symbols that we
don't want to export. Building with hidden visibility might seem redundant with this, but
actually building with hidden visibility has advantages anyway. See
​https://gcc.gnu.org/wiki/Visibility.

Note that I'm not confident GTK port can safely use -fvisibility-inlines-hidden, since it's
split between two shared objects. Also, because GTK is split into two shared objects, GTK
needs to build bmalloc and WTF as CMake OBJECT libraries, which is effectively the same as
using -Wl,--whole-archive to prevent symbols from being prematurely stripped away.

P.S. Major credit to Don Olmstead, who did most of the work to make this possible, which has
already landed in previous patches.

• Source/cmake/OptionsGTK.cmake:

Source/WebCore:
[WPE][GTK] Reenable -fvisibility=hidden (and -fvisibility-inlines-hidden for WPE)
​https://bugs.webkit.org/show_bug.cgi?id=181916

Patch by Michael Catanzaro <Michael Catanzaro> on 2021-03-09
Reviewed by Don Olmstead.

We need to export the destructor of EventTarget.

• PlatformGTK.cmake:
• dom/EventTarget.cpp:
• dom/EventTarget.h:

Tools:
[GTK] Reenable -fvisibility=hidden
​https://bugs.webkit.org/show_bug.cgi?id=181916

Patch by Michael Catanzaro <Michael Catanzaro> on 2021-03-09
Reviewed by Don Olmstead.

• TestWebKitAPI/PlatformGTK.cmake:
• TestWebKitAPI/glib/TestExpectations.json:

Location:

trunk

Files:

• ChangeLog (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/PlatformGTK.cmake (modified) (1 diff)
• Source/WebCore/dom/EventTarget.cpp (modified) (1 diff)
• Source/WebCore/dom/EventTarget.h (modified) (1 diff)
• Source/cmake/OptionsGTK.cmake (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/PlatformGTK.cmake (modified) (1 diff)
• Tools/TestWebKitAPI/glib/TestExpectations.json (modified) (1 diff)

trunk/ChangeLog

@@ +1 @@
+2021-03-09  Michael Catanzaro  <mcatanzaro@gnome.org>
+
+        [GTK] Reenable -fvisibility=hidden
+        https://bugs.webkit.org/show_bug.cgi?id=181916
+
+        Reviewed by Don Olmstead.
+
+        In non-DEVELOPER_MODE builds, we rely on a linker version script to hide symbols that we
+        don't want to export. Building with hidden visibility might seem redundant with this, but
+        actually building with hidden visibility has advantages anyway. See
+        https://gcc.gnu.org/wiki/Visibility.
+
+        Note that I'm not confident GTK port can safely use -fvisibility-inlines-hidden, since it's
+        split between two shared objects. Also, because GTK is split into two shared objects, GTK
+        needs to build bmalloc and WTF as CMake OBJECT libraries, which is effectively the same as
+        using -Wl,--whole-archive to prevent symbols from being prematurely stripped away.
+
+        P.S. Major credit to Don Olmstead, who did most of the work to make this possible, which has
+        already landed in previous patches.
+
+        * Source/cmake/OptionsGTK.cmake:
+
 2021-03-05  Michael Catanzaro  <mcatanzaro@gnome.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-03-09  Michael Catanzaro  <mcatanzaro@gnome.org>
+
+        [WPE][GTK] Reenable -fvisibility=hidden (and -fvisibility-inlines-hidden for WPE)
+        https://bugs.webkit.org/show_bug.cgi?id=181916
+
+        Reviewed by Don Olmstead.
+
+        We need to export the destructor of EventTarget.
+
+        * PlatformGTK.cmake:
+        * dom/EventTarget.cpp:
+        * dom/EventTarget.h:
+
 2021-03-09  Antoine Quint  <graouts@webkit.org>
 

trunk/Source/WebCore/PlatformGTK.cmake

@@ -8 +8 @@
 
 set(WebCore_OUTPUT_NAME WebCoreGTK)
-
-# FIXME: https://bugs.webkit.org/show_bug.cgi?id=181916
-# Remove these lines when turning on hidden visibility
-list(APPEND WebCore_PRIVATE_LIBRARIES WebKit::WTF)
-if (NOT USE_SYSTEM_MALLOC)
-    list(APPEND WebCore_PRIVATE_LIBRARIES WebKit::bmalloc)
-endif ()
 
 list(APPEND WebCore_UNIFIED_SOURCE_LIST_FILES

trunk/Source/WebCore/dom/EventTarget.cpp

@@ -67 +67 @@
 }
 
+EventTarget::~EventTarget() = default;
+
 bool EventTarget::isNode() const
 {

trunk/Source/WebCore/dom/EventTarget.h

@@ -104 +104 @@
 
 protected:
-    virtual ~EventTarget() = default;
+    WEBCORE_EXPORT virtual ~EventTarget();
     
     virtual EventTargetData* eventTargetData() = 0;

trunk/Source/cmake/OptionsGTK.cmake

@@ -18 +18 @@
 
 CALCULATE_LIBRARY_VERSIONS_FROM_LIBTOOL_TRIPLE(JAVASCRIPTCORE 37 0 19)
+
+set(CMAKE_C_VISIBILITY_PRESET hidden)
+set(CMAKE_CXX_VISIBILITY_PRESET hidden)
+set(bmalloc_LIBRARY_TYPE OBJECT)
+set(WTF_LIBRARY_TYPE OBJECT)
 
 # These are shared variables, but we special case their definition so that we can use the

trunk/Tools/ChangeLog

@@ +1 @@
+2021-03-09  Michael Catanzaro  <mcatanzaro@gnome.org>
+
+        [GTK] Reenable -fvisibility=hidden
+        https://bugs.webkit.org/show_bug.cgi?id=181916
+
+        Reviewed by Don Olmstead.
+
+        * TestWebKitAPI/PlatformGTK.cmake:
+        * TestWebKitAPI/glib/TestExpectations.json:
+
 2021-03-09  Mark Lam  <mark.lam@apple.com>
 

trunk/Tools/TestWebKitAPI/PlatformGTK.cmake

@@ -28 +28 @@
     GTK::GTK
 )
-
-# FIXME: Remove when turning on hidden visibility https://bugs.webkit.org/show_bug.cgi?id=181916
-list(APPEND TestJavaScriptCore_LIBRARIES WTF)
 
 # TestWebCore

trunk/Tools/TestWebKitAPI/glib/TestExpectations.json

@@ -295 +295 @@
             "/jsc/vm": {
                 "expected": {"all": {"slow": true}}
+            },
+            "/jsc/weak-value": {
+                "expected": {"all": {"status": ["FAIL", "PASS"], "bug": "webkit.org/b/222972"}}
             }
         }