239338 2022-04-14 08:55:31 -0700 False positive warning when building RefCounter test with GCC 12 2022-05-13 07:54:54 -0700 1 1 1 Unclassified WebKit Tools / Tests WebKit Nightly Build PC Linux RESOLVED FIXED InRadar P2 Normal --- 1 mcatanzaro mcatanzaro mcatanzaro webkit-bug-importer oldest_to_newest 1861515 0 mcatanzaro 2022-04-14 08:55:31 -0700 TODO: I should investigate this more closely [1736/6690] Building CXX object Tools/TestWebKitAPI/CMakeFiles/TestWTF.dir/Tests/WTF/RefCounter.cpp.o In file included from /home/mcatanzaro/Projects/WebKit/Tools/TestWebKitAPI/Tests/WTF/RefCounter.cpp:29: In member function ‘void WTF::RefCounter<T>::Count::deref() [with T = TestWebKitAPI::TestCounterType]’, inlined from ‘static void WTF::DefaultRefDerefTraits< <template-parameter-1-1> >::derefIfNotNull(T*) [with T = WTF::RefCounter<TestWebKitAPI::TestCounterType>::Count]’ at /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WTF/Headers/wtf/RefPtr.h:42:23, inlined from ‘WTF::RefPtr<T, PtrTraits, RefDerefTraits>& WTF::RefPtr<T, <template-parameter-1-2>, <template-parameter-1-3> >::operator=(std::nullptr_t) [with T = WTF::RefCounter<TestWebKitAPI::TestCounterType>::Count; _PtrTraits = WTF::RawPtrTraits<WTF::RefCounter<TestWebKitAPI::TestCounterType>::Count>; _RefDerefTraits = WTF::DefaultRefDerefTraits<WTF::RefCounter<TestWebKitAPI::TestCounterType>::Count>]’ at /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WTF/Headers/wtf/RefPtr.h:160:22, inlined from ‘virtual void TestWebKitAPI::WTF_RefCounter_Test::TestBody()’ at /home/mcatanzaro/Projects/WebKit/Tools/TestWebKitAPI/Tests/WTF/RefCounter.cpp:152:19: /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WTF/Headers/wtf/RefCounter.h:101:9: warning: pointer used after ‘void operator delete(void*, std::size_t)’ [-Wuse-after-free] 101 | if (m_refCounter && m_refCounter->m_valueDidChange) { | ^~~~~~~~~~~~ In member function ‘void WTF::RefCounter<T>::Count::refCounterWasDeleted() [with T = TestWebKitAPI::TestCounterType]’, inlined from ‘void WTF::RefCounter<T>::Count::deref() [with T = TestWebKitAPI::TestCounterType]’ at /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WTF/Headers/wtf/RefCounter.h:115:13, inlined from ‘static void WTF::DefaultRefDerefTraits< <template-parameter-1-1> >::derefIfNotNull(T*) [with T = WTF::RefCounter<TestWebKitAPI::TestCounterType>::Count]’ at /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WTF/Headers/wtf/RefPtr.h:42:23, inlined from ‘WTF::RefPtr<T, PtrTraits, RefDerefTraits>& WTF::RefPtr<T, <template-parameter-1-2>, <template-parameter-1-3> >::operator=(std::nullptr_t) [with T = WTF::RefCounter<TestWebKitAPI::TestCounterType>::Count; _PtrTraits = WTF::RawPtrTraits<WTF::RefCounter<TestWebKitAPI::TestCounterType>::Count>; _RefDerefTraits = WTF::DefaultRefDerefTraits<WTF::RefCounter<TestWebKitAPI::TestCounterType>::Count>]’ at /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WTF/Headers/wtf/RefPtr.h:160:22, inlined from ‘virtual void TestWebKitAPI::WTF_RefCounter_Test::TestBody()’ at /home/mcatanzaro/Projects/WebKit/Tools/TestWebKitAPI/Tests/WTF/RefCounter.cpp:150:19: /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WTF/Headers/wtf/RefCounter.h:128:9: note: call to ‘void operator delete(void*, std::size_t)’ here 128 | delete this; | ^~~~~~~~~~~ In member function ‘void WTF::RefCounter<T>::Count::deref() [with T = TestWebKitAPI::TestCounterType]’, inlined from ‘static void WTF::DefaultRefDerefTraits< <template-parameter-1-1> >::derefIfNotNull(T*) [with T = WTF::RefCounter<TestWebKitAPI::TestCounterType>::Count]’ at /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WTF/Headers/wtf/RefPtr.h:42:23, inlined from ‘WTF::RefPtr<T, PtrTraits, RefDerefTraits>& WTF::RefPtr<T, <template-parameter-1-2>, <template-parameter-1-3> >::operator=(std::nullptr_t) [with T = WTF::RefCounter<TestWebKitAPI::TestCounterType>::Count; _PtrTraits = WTF::RawPtrTraits<WTF::RefCounter<TestWebKitAPI::TestCounterType>::Count>; _RefDerefTraits = WTF::DefaultRefDerefTraits<WTF::RefCounter<TestWebKitAPI::TestCounterType>::Count>]’ at /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WTF/Headers/wtf/RefPtr.h:160:22, inlined from ‘virtual void TestWebKitAPI::WTF_RefCounter_Test::TestBody()’ at /home/mcatanzaro/Projects/WebKit/Tools/TestWebKitAPI/Tests/WTF/RefCounter.cpp:152:19: /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WTF/Headers/wtf/RefCounter.h:99:7: warning: pointer used after ‘void operator delete(void*, std::size_t)’ [-Wuse-after-free] 99 | --m_value; | ^~~~~~~ In member function ‘void WTF::RefCounter<T>::Count::refCounterWasDeleted() [with T = TestWebKitAPI::TestCounterType]’, inlined from ‘void WTF::RefCounter<T>::Count::deref() [with T = TestWebKitAPI::TestCounterType]’ at /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WTF/Headers/wtf/RefCounter.h:115:13, inlined from ‘static void WTF::DefaultRefDerefTraits< <template-parameter-1-1> >::derefIfNotNull(T*) [with T = WTF::RefCounter<TestWebKitAPI::TestCounterType>::Count]’ at /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WTF/Headers/wtf/RefPtr.h:42:23, inlined from ‘WTF::RefPtr<T, PtrTraits, RefDerefTraits>& WTF::RefPtr<T, <template-parameter-1-2>, <template-parameter-1-3> >::operator=(std::nullptr_t) [with T = WTF::RefCounter<TestWebKitAPI::TestCounterType>::Count; _PtrTraits = WTF::RawPtrTraits<WTF::RefCounter<TestWebKitAPI::TestCounterType>::Count>; _RefDerefTraits = WTF::DefaultRefDerefTraits<WTF::RefCounter<TestWebKitAPI::TestCounterType>::Count>]’ at /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WTF/Headers/wtf/RefPtr.h:160:22, inlined from ‘virtual void TestWebKitAPI::WTF_RefCounter_Test::TestBody()’ at /home/mcatanzaro/Projects/WebKit/Tools/TestWebKitAPI/Tests/WTF/RefCounter.cpp:150:19: /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WTF/Headers/wtf/RefCounter.h:128:9: note: call to ‘void operator delete(void*, std::size_t)’ here 128 | delete this; | ^~~~~~~~~~~ 1868562 1 mcatanzaro 2022-05-12 15:12:37 -0700 This warning is a false positive. m_refCounter is nullptr at this point. RefCounter<T>::~RefCounter calls m_count->refCounterWasDeleted(), which sets m_refCounter = nullptr. It's a poor man's WeakPtr, basically. I instrumented the code with some WTFLogAlways and it looks like everything is going fine, so I will suppress this warning. 1868563 2 webkit-bug-importer 2022-05-12 15:12:46 -0700 <rdar://problem/93204514> 1868564 3 mcatanzaro 2022-05-12 15:20:19 -0700 Actually wait, I only checked the first of the two warnings. As usual, I commented too soon. The second warning relates to m_value, not m_refCounter. The only way this could go wrong is if the Counter object itself is already invalid when its member function is called. I highly doubt that is happening, but will add more instrumentation to check this case as well, just to be completely sure. (If that does happen, then it would explain the first warning as well.) 1868570 4 mcatanzaro 2022-05-12 16:05:18 -0700 I think we're fine: // Testing (3g) - deref with callback from 1 -> 0 AFTER RefCounter has been destroyed. deref: this=0x20cdd10 ~Count this=0x20cdd10 // Testing (1b) - Construction without a callback. The use after free would happen if ~Count were somehow destroyed before Count::deref is called. The opposite is happening. Seems good. 1868593 5 mcatanzaro 2022-05-12 17:34:35 -0700 Pull request: https://github.com/WebKit/WebKit/pull/605 1868762 6 ews-feeder 2022-05-13 07:54:52 -0700 Committed r294157 (250527@main): <https://commits.webkit.org/250527@main> Reviewed commits have been landed. Closing PR #605 and removing active labels.