Bug 239338 - False positive warning when building RefCounter test with GCC 12
Summary: False positive warning when building RefCounter test with GCC 12
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: PC Linux
: P2 Normal
Assignee: Michael Catanzaro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-04-14 08:55 PDT by Michael Catanzaro
Modified: 2022-05-13 07:54 PDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2022-04-14 08:55:31 PDT
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;
      |         ^~~~~~~~~~~
Comment 1 Michael Catanzaro 2022-05-12 15:12:37 PDT
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.
Comment 2 Radar WebKit Bug Importer 2022-05-12 15:12:46 PDT
<rdar://problem/93204514>
Comment 3 Michael Catanzaro 2022-05-12 15:20:19 PDT
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.)
Comment 4 Michael Catanzaro 2022-05-12 16:05:18 PDT
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.
Comment 5 Michael Catanzaro 2022-05-12 17:34:35 PDT
Pull request: https://github.com/WebKit/WebKit/pull/605
Comment 6 EWS 2022-05-13 07:54:52 PDT
Committed r294157 (250527@main): <https://commits.webkit.org/250527@main>

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