Description of problem: With GCC 11, we have a warning spam when building WebKit, coming from RefPtr.h. It looks like this: [1019/5391] Building CXX object Tools/TestWebKitAPI/CMakeFiles/TestWTF.dir/Tests/WTF/HashMap.cpp.o In file included from WTF/Headers/wtf/HashFunctions.h:26, from ../../Tools/TestWebKitAPI/Tests/WTF/DeletedAddressOfOperator.h:28, from ../../Tools/TestWebKitAPI/Tests/WTF/HashMap.cpp:30: In static member function ‘static void WTF::DefaultRefDerefTraits< <template-parameter-1-1> >::derefIfNotNull(T*) [with T = TestWebKitAPI::RefLogger]’, inlined from ‘WTF::RefPtr<T, <template-parameter-1-2>, <template-parameter-1-3> >::~RefPtr() [with T = TestWebKitAPI::RefLogger; _PtrTraits = WTF::RawPtrTraits<TestWebKitAPI::RefLogger>; _RefDerefTraits = WTF::DefaultRefDerefTraits<TestWebKitAPI::RefLogger>]’ at WTF/Headers/wtf/RefPtr.h:73:61, inlined from ‘virtual void TestWebKitAPI::WTF_HashMap_Ref_Value_Test::TestBody()’ at ../../Tools/TestWebKitAPI/Tests/WTF/HashMap.cpp:977:36: WTF/Headers/wtf/RefPtr.h:42:23: warning: ‘this’ pointer is null [-Wnonnull] 42 | ptr->deref(); | ~~~~~~~~~~^~ In file included from ../../Tools/TestWebKitAPI/Tests/WTF/HashMap.cpp:32: ../../Tools/TestWebKitAPI/Tests/WTF/RefLogger.h: In member function ‘virtual void TestWebKitAPI::WTF_HashMap_Ref_Value_Test::TestBody()’: ../../Tools/TestWebKitAPI/Tests/WTF/RefLogger.h:35:10: note: in a call to non-static member function ‘void TestWebKitAPI::RefLogger::deref()’ 35 | void deref(); | ^~~~~ I believe this is a GCC bug because we just checked for null on the previous line. template<typename T> struct DefaultRefDerefTraits { static ALWAYS_INLINE void refIfNotNull(T* ptr) { if (LIKELY(ptr != nullptr)) ptr->ref(); } static ALWAYS_INLINE void derefIfNotNull(T* ptr) { if (LIKELY(ptr != nullptr)) ptr->deref(); // <------------ -Wnonnull warning here } }; The preprocessed source (attached) looks like this: static inline __attribute__((__always_inline__)) void derefIfNotNull(T* ptr) { if (__builtin_expect(!!(ptr != nullptr), 1)) ptr->deref(); } Version-Release number of selected component (if applicable): gcc-11.0.1-0.3.fc34 How reproducible: Always Steps to Reproduce: 1. gcc HashMap.cpp.ii -O2 -Wnonnull -c Actual results: warning prints Expected results: no warning Additional info: I'm attaching HashMap.cpp, HashMap.cpp.ii, and also RefPtr.h. Note it only happens with -O2, doesn't happen at lower optimization levels. Also does not happen if I remove the use of LIKELY()/__builtin_expect().
Created attachment 1771439 [details] preprocessed source
Created attachment 1771450 [details] RefPtr.h
Created attachment 1771451 [details] HashMap.cpp
The warning works as designed, but it runs relatively early in the optimization pipeline in hopes of avoiding false positives, but as result, doesn't benefit from transformations that happen later on that might eliminate code subsequently determined to be unreachable. That can, in turn, trigger false positives. The warning doesn't try to compensate for this by analyzing the control flow and is simply issued for calls to member functions with constant null this pointers like the one in basic block 233 below (seen in the output of -fdump-tree-post_ipa_warn), without trying to figure out if the call may be unreachable. In this case, one of the optimization that runs after the warning has been issued eliminates the invalid call (the last dump the call is seen in is thread1 and it's eliminated in vrp1). This could and should be improved but it's too late for GCC 11. ;; Function TestWebKitAPI::WTF_HashMap_Ref_Value_Test::TestBody (_ZN13TestWebKitAPI26WTF_HashMap_Ref_Value_Test8TestBodyEv, funcdef_no=10522, decl_uid=259574, cgraph_uid=4096, symbol_order=5311) void TestWebKitAPI::WTF_HashMap_Ref_Value_Test::TestBody (struct WTF_HashMap_Ref_Value_Test * const this) { ... <bb 232> [local count: 92578498]: gtest_ar_ ={v} {CLOBBER}; gtest_ar_ ={v} {CLOBBER}; MEM[(struct RefLogger * &)&emptyTake] = 0B; _182 = (long int) _476; _169 = _182; if (_169 != 0) goto <bb 233>; [90.00%] else goto <bb 234>; [10.00%] <bb 233> [local count: 83320647]: TestWebKitAPI::RefLogger::deref (0B); <<< -Wnonnull If the warning is triggered by a test I would recommend suppressing it via #pragma GCC diagnostic. I need to reduce the translation unit to a smaller test case to get a better idea of how to work around it in general (if it's possible). Please note that -Wnonnull as well as all other flow-sensitive warnings are necessarily prone to false positives that we will never be able to completely eliminate.
I finally reduced this monstrous translation unit to a small enough test case and submitted the following report upstream: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100086.
To get back to the workaround idea, as you discovered removing __builtin_expect() changes the IL in a way that avoids the warning and even though it looks like it only makes a negligible difference to the emitted assembly on x86_64 I would not suggest it as a workaround. If the warning comes up only in a test, I would suggest to suppress it there. If it impacts other code as well, a strategically placed #pragma GCC diagnostic might help (but in GCC 11 the pragma isn't easy to use in a targeted way with inlining.)
The pragma worked for me. I spent some more time on this today and left a follow-up comment in https://bugs.webkit.org/show_bug.cgi?id=224452. TL;DR: I think the costs of the new behavior outweighs the benefit, because I don't see how I could ever track down what's causing the warnings. Maybe there really is some impressive way that the |this| pointer could wind up null in these cases, but if so, I would need to see a coverity-style walkthrough showing exactly how in order for the warning to be actionable. In contrast, traditional -Wnonnull warnings were quite simple, and false-positives were rare.
The core -Wnonnull implementation hasn't changed recently. What has changed in GCC is that the this pointer is implicitly considered nonnull. In the test case I submitted upstream I replaced the this pointer with an argument to a nonmember nonnull function (puts) to show that the same warning is emitted regardless of the tighter GCC 11 checking. So the underlying problem isn't new, and while GCC 11 will have more false positives, it should also find correspondingly more real bugs, and so have the same S/R ratio.
This message is a reminder that Fedora Linux 34 is nearing its end of life. Fedora will stop maintaining and issuing updates for Fedora Linux 34 on 2022-06-07. It is Fedora's policy to close all bug reports from releases that are no longer maintained. At that time this bug will be closed as EOL if it remains open with a 'version' of '34'. Package Maintainer: If you wish for this bug to remain open because you plan to fix it in a currently maintained version, change the 'version' to a later Fedora Linux version. Thank you for reporting this issue and we are sorry that we were not able to fix it before Fedora Linux 34 is end of life. If you would still like to see this bug fixed and are able to reproduce it against a later version of Fedora Linux, you are encouraged to change the 'version' to a later version prior to this bug being closed.
This bug appears to have been reported against 'rawhide' during the Fedora Linux 37 development cycle. Changing version to 37.
This message is a reminder that Fedora Linux 37 is nearing its end of life. Fedora will stop maintaining and issuing updates for Fedora Linux 37 on 2023-12-05. It is Fedora's policy to close all bug reports from releases that are no longer maintained. At that time this bug will be closed as EOL if it remains open with a 'version' of '37'. Package Maintainer: If you wish for this bug to remain open because you plan to fix it in a currently maintained version, change the 'version' to a later Fedora Linux version. Note that the version field may be hidden. Click the "Show advanced fields" button if you do not see it. Thank you for reporting this issue and we are sorry that we were not able to fix it before Fedora Linux 37 is end of life. If you would still like to see this bug fixed and are able to reproduce it against a later version of Fedora Linux, you are encouraged to change the 'version' to a later version prior to this bug being closed.
Fedora Linux 37 entered end-of-life (EOL) status on None. Fedora Linux 37 is no longer maintained, which means that it will not receive any further security or bug fix updates. As a result we are closing this bug. If you can reproduce this bug against a currently maintained version of Fedora Linux please feel free to reopen this bug against that version. Note that the version field may be hidden. Click the "Show advanced fields" button if you do not see the version field. If you are unable to reopen this bug, please file a new report against an active release. Thank you for reporting this bug and we are sorry it could not be fixed.