Bug 239338 – False positive warning when building RefCounter test with GCC 12

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Michael Catanzaro]

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;
      |         ^~~~~~~~~~~

[Michael Catanzaro]

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.

[Radar WebKit Bug Importer]

<rdar://problem/93204514>

[Michael Catanzaro]

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.)

[Michael Catanzaro]

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.

[Michael Catanzaro]

Pull request: https://github.com/WebKit/WebKit/pull/605

[EWS]

Committed r294157 (250527@main): <https://commits.webkit.org/250527@main>

Reviewed commits have been landed. Closing PR #605 and removing active labels.