239353 2022-04-14 14:02:08 -0700 [GTK] Warning in WebKitDOMDocumentGtk.cpp with GCC 12 2022-05-16 06:56:21 -0700 1 1 1 Unclassified WebKit WebKitGTK WebKit Nightly Build PC Linux RESOLVED FIXED P2 Normal --- 1 mcatanzaro mcatanzaro bugs-noreply mcatanzaro oldest_to_newest 1861622 0 mcatanzaro 2022-04-14 14:02:08 -0700 I think GCC maybe found a real bug here: In file included from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/DerivedSources/WebKit/unified-sources/UnifiedSource-54928a2b-16.cpp:5: /home/mcatanzaro/Projects/WebKit/Source/WebKit/WebProcess/InjectedBundle/API/gtk/DOM/WebKitDOMDocumentGtk.cpp: In function ‘WebKitDOMNodeIterator* webkit_dom_document_create_node_iterator(WebKitDOMDocument*, WebKitDOMNode*, gulong, WebKitDOMNodeFilter*, gboolean, GError**)’: /home/mcatanzaro/Projects/WebKit/Source/WebKit/WebProcess/InjectedBundle/API/gtk/DOM/WebKitDOMDocumentGtk.cpp:1107:23: warning: pointer may be used after ‘static void WebCore::NodeIterator::operator delete(void*)’ [-Wuse-after-free] 1107 | return WebKit::kit(gobjectResult.get()); | ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~ In file included from /usr/include/c++/12/memory:76, from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WTF/Headers/wtf/StdLibExtras.h:30, from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WTF/Headers/wtf/FastMalloc.h:26, from /home/mcatanzaro/Projects/WebKit/Source/WebKit/config.h:42, from /home/mcatanzaro/Projects/WebKit/Source/WebKit/WebProcess/InjectedBundle/API/gtk/DOM/WebKitDOMDOMTokenList.cpp:20, from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/DerivedSources/WebKit/unified-sources/UnifiedSource-54928a2b-16.cpp:1: In member function ‘void std::default_delete<_Tp>::operator()(_Tp*) const [with _Tp = WebCore::NodeIterator]’, inlined from ‘void std::default_delete<_Tp>::operator()(_Tp*) const [with _Tp = WebCore::NodeIterator]’ at /usr/include/c++/12/bits/unique_ptr.h:79:7, inlined from ‘void WTF::RefCounted<T, Deleter>::deref() const [with T = WebCore::NodeIterator; Deleter = std::default_delete<WebCore::NodeIterator>]’ at /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WTF/Headers/wtf/RefCounted.h:190:22, inlined from ‘WTF::Ref<T, <template-parameter-1-2> >::~Ref() [with T = WebCore::NodeIterator; Traits = WTF::RawPtrTraits<WebCore::NodeIterator>]’ at /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WTF/Headers/wtf/Ref.h:61:23, inlined from ‘WebKitDOMNodeIterator* webkit_dom_document_create_node_iterator(WebKitDOMDocument*, WebKitDOMNode*, gulong, WebKitDOMNodeFilter*, gboolean, GError**)’ at /home/mcatanzaro/Projects/WebKit/Source/WebKit/WebProcess/InjectedBundle/API/gtk/DOM/WebKitDOMDocumentGtk.cpp:1106:87: /usr/include/c++/12/bits/unique_ptr.h:85:9: note: call to ‘static void WebCore::NodeIterator::operator delete(void*)’ here 85 | delete __ptr; | ^~~~~~~~~~~~ /home/mcatanzaro/Projects/WebKit/Source/WebKit/WebProcess/InjectedBundle/API/gtk/DOM/WebKitDOMDocumentGtk.cpp: In function ‘WebKitDOMTreeWalker* webkit_dom_document_create_tree_walker(WebKitDOMDocument*, WebKitDOMNode*, gulong, WebKitDOMNodeFilter*, gboolean, GError**)’: /home/mcatanzaro/Projects/WebKit/Source/WebKit/WebProcess/InjectedBundle/API/gtk/DOM/WebKitDOMDocumentGtk.cpp:1121:23: warning: pointer may be used after ‘static void WebCore::TreeWalker::operator delete(void*)’ [-Wuse-after-free] 1121 | return WebKit::kit(gobjectResult.get()); | ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~ In member function ‘void std::default_delete<_Tp>::operator()(_Tp*) const [with _Tp = WebCore::TreeWalker]’, inlined from ‘void std::default_delete<_Tp>::operator()(_Tp*) const [with _Tp = WebCore::TreeWalker]’ at /usr/include/c++/12/bits/unique_ptr.h:79:7, inlined from ‘void WTF::RefCounted<T, Deleter>::deref() const [with T = WebCore::TreeWalker; Deleter = std::default_delete<WebCore::TreeWalker>]’ at /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WTF/Headers/wtf/RefCounted.h:190:22, inlined from ‘WTF::Ref<T, <template-parameter-1-2> >::~Ref() [with T = WebCore::TreeWalker; Traits = WTF::RawPtrTraits<WebCore::TreeWalker>]’ at /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WTF/Headers/wtf/Ref.h:61:23, inlined from ‘WebKitDOMTreeWalker* webkit_dom_document_create_tree_walker(WebKitDOMDocument*, WebKitDOMNode*, gulong, WebKitDOMNodeFilter*, gboolean, GError**)’ at /home/mcatanzaro/Projects/WebKit/Source/WebKit/WebProcess/InjectedBundle/API/gtk/DOM/WebKitDOMDocumentGtk.cpp:1120:83: /usr/include/c++/12/bits/unique_ptr.h:85:9: note: call to ‘static void WebCore::TreeWalker::operator delete(void*)’ here 85 | delete __ptr; | ^~~~~~~~~~~~ Unless I'm mistaken, the problem is here: RefPtr<WebCore::TreeWalker> gobjectResult = WTF::getPtr(item->createTreeWalker(*convertedRoot, whatToShow, WTF::getPtr(convertedFilter), expandEntityReferences)); return WebKit::kit(gobjectResult.get()); Behind so many confusing layers like WTF::getPtr and WebKit::kit, it's a little unclear what's happening with the refcount, but it sure looks like have only one ref and return a dangling pointer. Using leakRef() makes the warnings go away: diff --git a/Source/WebKit/WebProcess/InjectedBundle/API/gtk/DOM/WebKitDOMDocumentGtk.cpp b/Source/WebKit/WebProcess/InjectedBundle/API/gtk/DOM/WebKitDOMDocumentGtk.cpp index a9cdac9c0911..95f6e5932abe 100644 --- a/Source/WebKit/WebProcess/InjectedBundle/API/gtk/DOM/WebKitDOMDocumentGtk.cpp +++ b/Source/WebKit/WebProcess/InjectedBundle/API/gtk/DOM/WebKitDOMDocumentGtk.cpp @@ -1104,7 +1104,7 @@ WebKitDOMNodeIterator* webkit_dom_document_create_node_iterator(WebKitDOMDocumen WebCore::Node* convertedRoot = WebKit::core(root); RefPtr<WebCore::NodeFilter> convertedFilter = WebKit::core(item, filter); RefPtr<WebCore::NodeIterator> gobjectResult = WTF::getPtr(item->createNodeIterator(*convertedRoot, whatToShow, WTF::getPtr(convertedFilter), expandEntityReferences)); - return WebKit::kit(gobjectResult.get()); + return WebKit::kit(gobjectResult.leakRef()); } WebKitDOMTreeWalker* webkit_dom_document_create_tree_walker(WebKitDOMDocument* self, WebKitDOMNode* root, gulong whatToShow, WebKitDOMNodeFilter* filter, gboolean expandEntityReferences, GError** error) @@ -1118,7 +1118,7 @@ WebKitDOMTreeWalker* webkit_dom_document_create_tree_walker(WebKitDOMDocument* s WebCore::Node* convertedRoot = WebKit::core(root); RefPtr<WebCore::NodeFilter> convertedFilter = WebKit::core(item, filter); RefPtr<WebCore::TreeWalker> gobjectResult = WTF::getPtr(item->createTreeWalker(*convertedRoot, whatToShow, WTF::getPtr(convertedFilter), expandEntityReferences)); - return WebKit::kit(gobjectResult.get()); + return WebKit::kit(gobjectResult.leakRef()); } WebKitDOMCSSStyleDeclaration* webkit_dom_document_get_override_style(WebKitDOMDocument*, WebKitDOMElement*, const gchar*) Problem is, this was previously generated code, so it's a pattern that is very common throughout these bindings. I wonder why GCC is only flagging these two particular places.... 1868866 1 mcatanzaro 2022-05-13 14:39:38 -0700 (In reply to Michael Catanzaro from comment #0) > Behind so many confusing layers like WTF::getPtr and WebKit::kit, it's a > little unclear what's happening with the refcount, but it sure looks like > have only one ref and return a dangling pointer. What we're returning is actually a GObject from the DOMObjectCache, not the WebCore object. So the general pattern looks fine. 1868876 2 mcatanzaro 2022-05-13 15:01:13 -0700 I don't see anything wrong with the code. Maybe there could be a bug in DOMObjectCache, but if so, we're not going to find it from this useless warning. I'm just going to suppress it and move on. Spent too much effort already squinting at these deprecated functions. 1868883 3 mcatanzaro 2022-05-13 15:10:34 -0700 Pull request: https://github.com/WebKit/WebKit/pull/621 1869213 4 ews-feeder 2022-05-16 06:56:18 -0700 Committed r294232 (250590@main): <https://commits.webkit.org/250590@main> Reviewed commits have been landed. Closing PR #621 and removing active labels.