[Webkit-unassigned] [Bug 239353] New: [GTK] -Wuse-after-free in WebKitDOMDocumentGtk.cpp with GCC 12
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Apr 14 14:02:08 PDT 2022
https://bugs.webkit.org/show_bug.cgi?id=239353
Bug ID: 239353
Summary: [GTK] -Wuse-after-free in WebKitDOMDocumentGtk.cpp
with GCC 12
Product: WebKit
Version: WebKit Nightly Build
Hardware: PC
OS: Linux
Status: NEW
Severity: Normal
Priority: P2
Component: WebKitGTK
Assignee: webkit-unassigned at lists.webkit.org
Reporter: mcatanzaro at gnome.org
CC: bugs-noreply at webkitgtk.org
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....
--
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20220414/a2a36318/attachment.htm>
More information about the webkit-unassigned
mailing list