Bug 239299

Summary: GCC 12 -Wdangling-pointer warning spam from AbstractSlotVisitorInlines.h
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: JavaScriptCoreAssignee: Michael Catanzaro <mcatanzaro>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, mcatanzaro, msaboff, saam, tzagallo, webkit-bug-importer, zan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch none

Michael Catanzaro
Reported 2022-04-13 13:06:52 PDT
GCC 12 has added a -Wdangling-pointer warning, which surprisingly WebKit survives quite well. It only trips once in JavaScriptCore at AbstractSlotVisitorInlines.h:77. Sadly, that's a header file, so it creates a huge warning spam as the warning gets printed again for every translation unit that includes AbstractSlotVisitorInlines.h. The full warning is: In file included from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/JavaScriptCore/PrivateHeaders/JavaScriptCore/SlotVisitorInlines.h:28, from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/JavaScriptCore/PrivateHeaders/JavaScriptCore/JSCellInlines.h:45, from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/JavaScriptCore/PrivateHeaders/JavaScriptCore/JSCJSValueInlines.h:35, from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/JavaScriptCore/PrivateHeaders/JavaScriptCore/StructureInlines.h:30, from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/JavaScriptCore/PrivateHeaders/JavaScriptCore/ArrayStorageInlines.h:29, from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/JavaScriptCore/PrivateHeaders/JavaScriptCore/ButterflyInlines.h:28, from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/JavaScriptCore/PrivateHeaders/JavaScriptCore/JSObjectInlines.h:28, from /home/mcatanzaro/Projects/WebKit/Source/WebCore/bindings/js/JSDOMGlobalObject.h:32, from /home/mcatanzaro/Projects/WebKit/Source/WebCore/bindings/js/JSDOMWrapper.h:24, from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WebCore/DerivedSources/JSDeprecatedCSSOMRect.h:24, from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WebCore/DerivedSources/JSDeprecatedCSSOMRect.cpp:22, from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WebCore/DerivedSources/unified-sources/UnifiedSource-3a52ce78-34.cpp:1: In constructor ‘JSC::AbstractSlotVisitor::ReferrerContext::ReferrerContext(JSC::AbstractSlotVisitor&, JSC::AbstractSlotVisitor::ReferrerToken)’, inlined from ‘static void JSC::JSCell::visitOutputConstraints(JSC::JSCell*, JSC::AbstractSlotVisitor&)’ at /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/JavaScriptCore/PrivateHeaders/JavaScriptCore/JSCellInlines.h:158:1: /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/JavaScriptCore/PrivateHeaders/JavaScriptCore/AbstractSlotVisitorInlines.h:77:25: warning: storing the address of local variable ‘context’ in ‘*visitor.JSC::AbstractSlotVisitor::m_context’ [-Wdangling-pointer=] 77 | m_visitor.m_context = this; | ~~~~~~~~~~~~~~~~~~~~^~~~~~ In file included from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/JavaScriptCore/PrivateHeaders/JavaScriptCore/JSCell.h:35, from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/JavaScriptCore/PrivateHeaders/JavaScriptCore/JSArray.h:26, from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/JavaScriptCore/PrivateHeaders/JavaScriptCore/ArrayAllocationProfile.h:29, from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/JavaScriptCore/PrivateHeaders/JavaScriptCore/JSGlobalObject.h:24, from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/JavaScriptCore/PrivateHeaders/JavaScriptCore/InternalFunctionAllocationProfile.h:28, from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/JavaScriptCore/PrivateHeaders/JavaScriptCore/FunctionRareData.h:28, from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/JavaScriptCore/PrivateHeaders/JavaScriptCore/JSFunction.h:26, from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WebCore/DerivedSources/JSDOMBindingInternalsBuiltins.h:35, from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WebCore/DerivedSources/WebCoreJSBuiltinInternals.h:38, from /home/mcatanzaro/Projects/WebKit/Source/WebCore/bindings/js/JSDOMGlobalObject.h:29: /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/JavaScriptCore/PrivateHeaders/JavaScriptCore/JSCellInlines.h: In static member function ‘static void JSC::JSCell::visitOutputConstraints(JSC::JSCell*, JSC::AbstractSlotVisitor&)’: /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/JavaScriptCore/PrivateHeaders/JavaScriptCore/SlotVisitorMacros.h:104:46: note: ‘context’ declared here 104 | AbstractSlotVisitor::ReferrerContext context(visitor, cell); \ | ^~~~~~~ /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/JavaScriptCore/PrivateHeaders/JavaScriptCore/SlotVisitorMacros.h:104:46: note: in definition of macro ‘DEFINE_VISIT_OUTPUT_CONSTRAINTS_WITH_MODIFIER’ 104 | AbstractSlotVisitor::ReferrerContext context(visitor, cell); \ | ^~~~~~~ /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/JavaScriptCore/PrivateHeaders/JavaScriptCore/SlotVisitorMacros.h:104:46: note: ‘visitor’ declared here 104 | AbstractSlotVisitor::ReferrerContext context(visitor, cell); \ | ^~~~~~~ /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/JavaScriptCore/PrivateHeaders/JavaScriptCore/SlotVisitorMacros.h:104:46: note: in definition of macro ‘DEFINE_VISIT_OUTPUT_CONSTRAINTS_WITH_MODIFIER’ 104 | AbstractSlotVisitor::ReferrerContext context(visitor, cell); \ | ^~~~~~~ I'll attach a patch to suppress the warning -- since it makes it difficult to see any other warnings -- but review from JSC devs would be appreciated, because frankly I have no clue whether this is a disaster or (more likely?) just a false positive. I guess the purported dangling pointer is the AbstractSlotVisitor::ReferrerContext declared in DEFINE_VISIT_OUTPUT_CONSTRAINTS_WITH_MODIFIER and used in JSCellInlines.h, but I don't see where it gets stored as part of the AbstractSlotVisitor::ReferrerContext object and certainly don't understand why that would be done.
Attachments
Patch (1.63 KB, patch)
2022-04-13 13:08 PDT, Michael Catanzaro 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Michael Catanzaro
Comment 1 2022-04-13 13:08:17 PDT
Created attachment 457563 [details] Patch
Mark Lam
Comment 2 2022-04-13 14:37:45 PDT
Comment on attachment 457563 [details] Patch r=me
Michael Catanzaro
Comment 3 2022-04-13 15:41:09 PDT
(In reply to Mark Lam from comment #2) > r=me OK, I'll take that to mean you've determined the warning is a false positive. I'm not too surprised.
Mark Lam
Comment 4 2022-04-13 15:52:46 PDT
(In reply to Michael Catanzaro from comment #3) > (In reply to Mark Lam from comment #2) > > r=me > > OK, I'll take that to mean you've determined the warning is a false > positive. I'm not too surprised. Yes, it's set and used while the AbstractSlotVisitor::ReferrerContext is on stack, and cleared out on destruction: inline AbstractSlotVisitor::ReferrerContext::ReferrerContext(AbstractSlotVisitor& visitor, ReferrerToken referrer) : m_visitor(visitor) , m_referrer(referrer) { m_previous = m_visitor.m_context; ... m_visitor.m_context = this; } inline AbstractSlotVisitor::ReferrerContext::~ReferrerContext() { m_visitor.m_context = m_previous; } The warning is wrong.
EWS
Comment 5 2022-04-13 16:15:19 PDT
Committed r292841 (249614@main): <https://commits.webkit.org/249614@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 457563 [details].
Radar WebKit Bug Importer
Comment 6 2022-04-13 16:16:20 PDT
<rdar://problem/91721538>
Zan Dobersek
Comment 7 2022-04-13 23:38:45 PDT
Now every supported GCC version below 12 is complaining: [1897/2120] Building CXX object Source/WebKit/CMakeFiles/WebKit.dir/WebProcess/WebPage/WebPage.cpp.o In file included from /build/webkit/build-unstable/build-webkit/WTF/Headers/wtf/Platform.h:31, from /work/webkit/git/Source/WebKit/WebKit2Prefix.h:31, from <command-line>: /build/webkit/build-unstable/build-webkit/JavaScriptCore/PrivateHeaders/JavaScriptCore/AbstractSlotVisitorInlines.h: In constructor ‘JSC::AbstractSlotVisitor::ReferrerContext::ReferrerContext(JSC::AbstractSlotVisitor&, JSC::AbstractSlotVisitor::ReferrerToken)’: /build/webkit/build-unstable/build-webkit/WTF/Headers/wtf/Compiler.h:451:5: warning: unknown option after ‘#pragma GCC diagnostic’ kind [-Wpragmas] 451 | _Pragma(_COMPILER_STRINGIZE(compiler diagnostic ignored warning)) | ^~~~~~~ /build/webkit/build-unstable/build-webkit/WTF/Headers/wtf/Compiler.h:438:34: note: in expansion of macro ‘IGNORE_WARNINGS_BEGIN_IMPL_1’ 438 | #define _COMPILER_CONCAT_I(a, b) a ## b | ^ /build/webkit/build-unstable/build-webkit/WTF/Headers/wtf/Compiler.h:439:32: note: in expansion of macro ‘_COMPILER_CONCAT_I’ 439 | #define _COMPILER_CONCAT(a, b) _COMPILER_CONCAT_I(a, b) | ^~~~~~~~~~~~~~~~~~ /build/webkit/build-unstable/build-webkit/WTF/Headers/wtf/Compiler.h:448:5: note: in expansion of macro ‘_COMPILER_CONCAT’ 448 | _COMPILER_CONCAT(IGNORE_WARNINGS_BEGIN_IMPL_, cond)(compiler, warning) | ^~~~~~~~~~~~~~~~ /build/webkit/build-unstable/build-webkit/WTF/Headers/wtf/Compiler.h:462:56: note: in expansion of macro ‘IGNORE_WARNINGS_BEGIN_COND’ 462 | #define _IGNORE_WARNINGS_BEGIN_IMPL(compiler, warning) IGNORE_WARNINGS_BEGIN_COND(1, compiler, warning) | ^~~~~~~~~~~~~~~~~~~~~~~~~~ /build/webkit/build-unstable/build-webkit/WTF/Headers/wtf/Compiler.h:466:5: note: in expansion of macro ‘_IGNORE_WARNINGS_BEGIN_IMPL’ 466 | _IGNORE_WARNINGS_BEGIN_IMPL(compiler, _COMPILER_WARNING_NAME(warning)) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ /build/webkit/build-unstable/build-webkit/WTF/Headers/wtf/Compiler.h:472:44: note: in expansion of macro ‘IGNORE_WARNINGS_BEGIN_IMPL’ 472 | #define IGNORE_GCC_WARNINGS_BEGIN(warning) IGNORE_WARNINGS_BEGIN_IMPL(GCC, warning) | ^~~~~~~~~~~~~~~~~~~~~~~~~~ /build/webkit/build-unstable/build-webkit/JavaScriptCore/PrivateHeaders/JavaScriptCore/AbstractSlotVisitorInlines.h:77:1: note: in expansion of macro ‘IGNORE_GCC_WARNINGS_BEGIN’ 77 | IGNORE_GCC_WARNINGS_BEGIN("dangling-pointer") | ^~~~~~~~~~~~~~~~~~~~~~~~~ /build/webkit/build-unstable/build-webkit/WTF/Headers/wtf/Compiler.h:451:5: note: did you mean ‘-Wdangling-else’? 451 | _Pragma(_COMPILER_STRINGIZE(compiler diagnostic ignored warning)) | ^~~~~~~ /build/webkit/build-unstable/build-webkit/WTF/Headers/wtf/Compiler.h:438:34: note: in expansion of macro ‘IGNORE_WARNINGS_BEGIN_IMPL_1’ 438 | #define _COMPILER_CONCAT_I(a, b) a ## b | ^ /build/webkit/build-unstable/build-webkit/WTF/Headers/wtf/Compiler.h:439:32: note: in expansion of macro ‘_COMPILER_CONCAT_I’ 439 | #define _COMPILER_CONCAT(a, b) _COMPILER_CONCAT_I(a, b) | ^~~~~~~~~~~~~~~~~~ /build/webkit/build-unstable/build-webkit/WTF/Headers/wtf/Compiler.h:448:5: note: in expansion of macro ‘_COMPILER_CONCAT’ 448 | _COMPILER_CONCAT(IGNORE_WARNINGS_BEGIN_IMPL_, cond)(compiler, warning) | ^~~~~~~~~~~~~~~~ /build/webkit/build-unstable/build-webkit/WTF/Headers/wtf/Compiler.h:462:56: note: in expansion of macro ‘IGNORE_WARNINGS_BEGIN_COND’ 462 | #define _IGNORE_WARNINGS_BEGIN_IMPL(compiler, warning) IGNORE_WARNINGS_BEGIN_COND(1, compiler, warning) | ^~~~~~~~~~~~~~~~~~~~~~~~~~ /build/webkit/build-unstable/build-webkit/WTF/Headers/wtf/Compiler.h:466:5: note: in expansion of macro ‘_IGNORE_WARNINGS_BEGIN_IMPL’ 466 | _IGNORE_WARNINGS_BEGIN_IMPL(compiler, _COMPILER_WARNING_NAME(warning)) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ /build/webkit/build-unstable/build-webkit/WTF/Headers/wtf/Compiler.h:472:44: note: in expansion of macro ‘IGNORE_WARNINGS_BEGIN_IMPL’ 472 | #define IGNORE_GCC_WARNINGS_BEGIN(warning) IGNORE_WARNINGS_BEGIN_IMPL(GCC, warning) | ^~~~~~~~~~~~~~~~~~~~~~~~~~ /build/webkit/build-unstable/build-webkit/JavaScriptCore/PrivateHeaders/JavaScriptCore/AbstractSlotVisitorInlines.h:77:1: note: in expansion of macro ‘IGNORE_GCC_WARNINGS_BEGIN’ 77 | IGNORE_GCC_WARNINGS_BEGIN("dangling-pointer") | ^~~~~~~~~~~~~~~~~~~~~~~~~
Michael Catanzaro
Comment 8 2022-04-14 07:12:10 PDT
Well that is frustrating. I might have to test compiler version manually here.
Michael Catanzaro
Comment 9 2022-04-14 09:46:44 PDT
(In reply to Michael Catanzaro from comment #8) > Well that is frustrating. I might have to test compiler version manually > here. I found another way... let's use bug #239344
Note You need to log in before you can comment on or make changes to this bug.