Title: [274216] trunk
Revision
274216
Author
[email protected]
Date
2021-03-10 09:26:04 -0800 (Wed, 10 Mar 2021)

Log Message

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

Patch by Michael Catanzaro <[email protected]> on 2021-03-10
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/_javascript_Core:

We need to export the destructor of IsoHeapCellType.

* heap/IsoHeapCellType.cpp:
* heap/IsoHeapCellType.h:

Source/WebCore:

We need to export the destructor of EventTarget and the delete operator of
EventTargetWithInlineData. They are used in WebKitTestRunner.

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

Source/WTF:

We need to export WTF::filenameForDisplay.

* wtf/FileSystem.h:

Tools:

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

Modified Paths

Diff

Modified: trunk/ChangeLog (274215 => 274216)


--- trunk/ChangeLog	2021-03-10 16:18:06 UTC (rev 274215)
+++ trunk/ChangeLog	2021-03-10 17:26:04 UTC (rev 274216)
@@ -1,3 +1,25 @@
+2021-03-10  Michael Catanzaro  <[email protected]>
+
+        [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-10  Commit Queue  <[email protected]>
 
         Unreviewed, reverting r274166.

Modified: trunk/Source/_javascript_Core/ChangeLog (274215 => 274216)


--- trunk/Source/_javascript_Core/ChangeLog	2021-03-10 16:18:06 UTC (rev 274215)
+++ trunk/Source/_javascript_Core/ChangeLog	2021-03-10 17:26:04 UTC (rev 274216)
@@ -1,3 +1,15 @@
+2021-03-10  Michael Catanzaro  <[email protected]>
+
+        [GTK] Reenable -fvisibility=hidden
+        https://bugs.webkit.org/show_bug.cgi?id=181916
+
+        Reviewed by Don Olmstead.
+
+        We need to export the destructor of IsoHeapCellType.
+
+        * heap/IsoHeapCellType.cpp:
+        * heap/IsoHeapCellType.h:
+
 2021-03-09  Don Olmstead  <[email protected]>
 
         GLib JSC API headers should only include other GLib JSC API headers

Modified: trunk/Source/_javascript_Core/heap/IsoHeapCellType.cpp (274215 => 274216)


--- trunk/Source/_javascript_Core/heap/IsoHeapCellType.cpp	2021-03-10 16:18:06 UTC (rev 274215)
+++ trunk/Source/_javascript_Core/heap/IsoHeapCellType.cpp	2021-03-10 17:26:04 UTC (rev 274216)
@@ -37,6 +37,8 @@
 {
 }
 
+IsoHeapCellType::~IsoHeapCellType() = default;
+
 void IsoHeapCellType::finishSweep(MarkedBlock::Handle& handle, FreeList* freeList)
 {
     handle.finishSweepKnowingHeapCellType(freeList, *this);

Modified: trunk/Source/_javascript_Core/heap/IsoHeapCellType.h (274215 => 274216)


--- trunk/Source/_javascript_Core/heap/IsoHeapCellType.h	2021-03-10 16:18:06 UTC (rev 274215)
+++ trunk/Source/_javascript_Core/heap/IsoHeapCellType.h	2021-03-10 17:26:04 UTC (rev 274216)
@@ -34,6 +34,8 @@
 public:
     using DestroyFunctionPtr = void (*)(JSCell*);
 
+    JS_EXPORT_PRIVATE ~IsoHeapCellType();
+
     JS_EXPORT_PRIVATE IsoHeapCellType(DestructionMode, DestroyFunctionPtr);
 
     template<typename CellType>

Modified: trunk/Source/WTF/ChangeLog (274215 => 274216)


--- trunk/Source/WTF/ChangeLog	2021-03-10 16:18:06 UTC (rev 274215)
+++ trunk/Source/WTF/ChangeLog	2021-03-10 17:26:04 UTC (rev 274216)
@@ -1,3 +1,14 @@
+2021-03-10  Michael Catanzaro  <[email protected]>
+
+        [GTK] Reenable -fvisibility=hidden
+        https://bugs.webkit.org/show_bug.cgi?id=181916
+
+        Reviewed by Don Olmstead.
+
+        We need to export WTF::filenameForDisplay.
+
+        * wtf/FileSystem.h:
+
 2021-03-09  Chris Dumez  <[email protected]>
 
         Use RELEASE_ASSERT() for Deque bounds checks

Modified: trunk/Source/WTF/wtf/FileSystem.h (274215 => 274216)


--- trunk/Source/WTF/wtf/FileSystem.h	2021-03-10 16:18:06 UTC (rev 274215)
+++ trunk/Source/WTF/wtf/FileSystem.h	2021-03-10 17:26:04 UTC (rev 274216)
@@ -183,7 +183,7 @@
 #endif
 
 #if USE(GLIB)
-String filenameForDisplay(const String&);
+WTF_EXPORT_PRIVATE String filenameForDisplay(const String&);
 #endif
 
 #if OS(WINDOWS)

Modified: trunk/Source/WebCore/ChangeLog (274215 => 274216)


--- trunk/Source/WebCore/ChangeLog	2021-03-10 16:18:06 UTC (rev 274215)
+++ trunk/Source/WebCore/ChangeLog	2021-03-10 17:26:04 UTC (rev 274216)
@@ -1,3 +1,17 @@
+2021-03-10  Michael Catanzaro  <[email protected]>
+
+        [GTK] Reenable -fvisibility=hidden
+        https://bugs.webkit.org/show_bug.cgi?id=181916
+
+        Reviewed by Don Olmstead.
+
+        We need to export the destructor of EventTarget and the delete operator of 
+        EventTargetWithInlineData. They are used in WebKitTestRunner.
+
+        * PlatformGTK.cmake:
+        * dom/EventTarget.cpp:
+        * dom/EventTarget.h:
+
 2021-03-10  Wenson Hsieh  <[email protected]>
 
         Logic for updating the text selection when dragging selection handles should account for image overlays

Modified: trunk/Source/WebCore/PlatformGTK.cmake (274215 => 274216)


--- trunk/Source/WebCore/PlatformGTK.cmake	2021-03-10 16:18:06 UTC (rev 274215)
+++ trunk/Source/WebCore/PlatformGTK.cmake	2021-03-10 17:26:04 UTC (rev 274216)
@@ -8,13 +8,6 @@
 
 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
     "SourcesGTK.txt"
 

Modified: trunk/Source/WebCore/dom/EventTarget.cpp (274215 => 274216)


--- trunk/Source/WebCore/dom/EventTarget.cpp	2021-03-10 16:18:06 UTC (rev 274215)
+++ trunk/Source/WebCore/dom/EventTarget.cpp	2021-03-10 17:26:04 UTC (rev 274216)
@@ -66,6 +66,8 @@
     return EventTargetConcrete::create(context);
 }
 
+EventTarget::~EventTarget() = default;
+
 bool EventTarget::isNode() const
 {
     return false;

Modified: trunk/Source/WebCore/dom/EventTarget.h (274215 => 274216)


--- trunk/Source/WebCore/dom/EventTarget.h	2021-03-10 16:18:06 UTC (rev 274215)
+++ trunk/Source/WebCore/dom/EventTarget.h	2021-03-10 17:26:04 UTC (rev 274216)
@@ -103,7 +103,7 @@
     const EventTargetData* eventTargetData() const;
 
 protected:
-    virtual ~EventTarget() = default;
+    WEBCORE_EXPORT virtual ~EventTarget();
     
     virtual EventTargetData* eventTargetData() = 0;
     virtual EventTargetData* eventTargetDataConcurrently() = 0;
@@ -122,7 +122,7 @@
 };
 
 class EventTargetWithInlineData : public EventTarget {
-    WTF_MAKE_ISO_ALLOCATED(EventTargetWithInlineData);
+    WTF_MAKE_ISO_ALLOCATED_EXPORT(EventTargetWithInlineData, WEBCORE_EXPORT);
 protected:
     EventTargetData* eventTargetData() final { return &m_eventTargetData; }
     EventTargetData* eventTargetDataConcurrently() final { return &m_eventTargetData; }

Modified: trunk/Source/cmake/OptionsGTK.cmake (274215 => 274216)


--- trunk/Source/cmake/OptionsGTK.cmake	2021-03-10 16:18:06 UTC (rev 274215)
+++ trunk/Source/cmake/OptionsGTK.cmake	2021-03-10 17:26:04 UTC (rev 274216)
@@ -18,6 +18,11 @@
 
 CALCULATE_LIBRARY_VERSIONS_FROM_LIBTOOL_TRIPLE(_javascript_CORE 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
 # CMAKE_INSTALL_* variables that are populated by the GNUInstallDirs macro.
 set(LIB_INSTALL_DIR "${CMAKE_INSTALL_FULL_LIBDIR}" CACHE PATH "Absolute path to library installation directory")

Modified: trunk/Tools/ChangeLog (274215 => 274216)


--- trunk/Tools/ChangeLog	2021-03-10 16:18:06 UTC (rev 274215)
+++ trunk/Tools/ChangeLog	2021-03-10 17:26:04 UTC (rev 274216)
@@ -1,3 +1,13 @@
+2021-03-10  Michael Catanzaro  <[email protected]>
+
+        [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-10  Jonathan Bedard  <[email protected]>
 
         git-webkit should have an "info" command

Modified: trunk/Tools/TestWebKitAPI/PlatformGTK.cmake (274215 => 274216)


--- trunk/Tools/TestWebKitAPI/PlatformGTK.cmake	2021-03-10 16:18:06 UTC (rev 274215)
+++ trunk/Tools/TestWebKitAPI/PlatformGTK.cmake	2021-03-10 17:26:04 UTC (rev 274216)
@@ -28,9 +28,6 @@
     GTK::GTK
 )
 
-# FIXME: Remove when turning on hidden visibility https://bugs.webkit.org/show_bug.cgi?id=181916
-list(APPEND TestJavaScriptCore_LIBRARIES WTF)
-
 # TestWebCore
 list(APPEND TestWebCore_SOURCES
     ${test_main_SOURCES}

Modified: trunk/Tools/TestWebKitAPI/glib/TestExpectations.json (274215 => 274216)


--- trunk/Tools/TestWebKitAPI/glib/TestExpectations.json	2021-03-10 16:18:06 UTC (rev 274215)
+++ trunk/Tools/TestWebKitAPI/glib/TestExpectations.json	2021-03-10 17:26:04 UTC (rev 274216)
@@ -294,6 +294,9 @@
         "subtests": {
             "/jsc/vm": {
                 "expected": {"all": {"slow": true}}
+            },
+            "/jsc/weak-value": {
+                "expected": {"all": {"status": ["FAIL", "PASS"], "bug": "webkit.org/b/222972"}}
             }
         }
     },
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to