Bug 224782

Summary: -Warray-bounds warning in AirAllocateRegistersByGraphColoring.cpp with GCC 11
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: JavaScriptCoreAssignee: Michael Catanzaro <mcatanzaro>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, ews-watchlist, keith_miller, lingcherd_ho, mark.lam, mcatanzaro, msaboff, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch
none
Patch for landing none

Michael Catanzaro
Reported 2021-04-19 13:35:58 PDT
With GCC 11, we have a nice -Warray-bounds warning spam coming from AirAllocateRegistersByGraphColoring.cpp: [254/1653] Building CXX object Source/JavaScriptCore/CMak...edSources/unified-sources/UnifiedSource-23a5fd0e-10.cpp.o In file included from JavaScriptCore/DerivedSources/unified-sources/UnifiedSource-23a5fd0e-10.cpp:7: ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp: In member function ‘void JSC::B3::Air::{anonymous}::GraphColoringRegisterAllocation::allocateOnBank() [with JSC::B3::Bank bank = JSC::B3::GP]’: ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1559:29: warning: array subscript ‘JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::GP, JSC::B3::Air::{anonymous}::IRC, JSC::B3::Air::{anonymous}::InterferenceEdge<short unsigned int, unsigned int> >[0]’ is partly outside array bounds of ‘JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::GP, JSC::B3::Air::{anonymous}::Briggs, JSC::B3::Air::{anonymous}::InterferenceEdge<unsigned int, long unsigned int> > [1]’ [-Warray-bounds] 1559 | m_coloredTmp.resize(m_lastPrecoloredRegisterIndex + 1); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1895:91: note: while referencing ‘allocator’ 1895 | ColoringAllocator<bank, Briggs, InterferenceEdge<uint32_t, uint64_t>> allocator(m_code, m_tmpWidth, m_useCounts, unspillableTmps); | ^~~~~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1560:35: warning: array subscript ‘JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::GP, JSC::B3::Air::{anonymous}::IRC, JSC::B3::Air::{anonymous}::InterferenceEdge<short unsigned int, unsigned int> >[0]’ is partly outside array bounds of ‘JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::GP, JSC::B3::Air::{anonymous}::Briggs, JSC::B3::Air::{anonymous}::InterferenceEdge<unsigned int, long unsigned int> > [1]’ [-Warray-bounds] 1560 | for (unsigned i = 1; i <= m_lastPrecoloredRegisterIndex; ++i) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1895:91: note: while referencing ‘allocator’ 1895 | ColoringAllocator<bank, Briggs, InterferenceEdge<uint32_t, uint64_t>> allocator(m_code, m_tmpWidth, m_useCounts, unspillableTmps); | ^~~~~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1559:29: warning: array subscript ‘JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::GP, JSC::B3::Air::{anonymous}::IRC, JSC::B3::Air::{anonymous}::InterferenceEdge<short unsigned int, unsigned int> >[0]’ is partly outside array bounds of ‘JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::GP, JSC::B3::Air::{anonymous}::Briggs, JSC::B3::Air::{anonymous}::InterferenceEdge<short unsigned int, unsigned int> > [1]’ [-Warray-bounds] 1559 | m_coloredTmp.resize(m_lastPrecoloredRegisterIndex + 1); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1892:91: note: while referencing ‘allocator’ 1892 | ColoringAllocator<bank, Briggs, InterferenceEdge<uint16_t, uint32_t>> allocator(m_code, m_tmpWidth, m_useCounts, unspillableTmps); | ^~~~~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1560:35: warning: array subscript ‘JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::GP, JSC::B3::Air::{anonymous}::IRC, JSC::B3::Air::{anonymous}::InterferenceEdge<short unsigned int, unsigned int> >[0]’ is partly outside array bounds of ‘JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::GP, JSC::B3::Air::{anonymous}::Briggs, JSC::B3::Air::{anonymous}::InterferenceEdge<short unsigned int, unsigned int> > [1]’ [-Warray-bounds] 1560 | for (unsigned i = 1; i <= m_lastPrecoloredRegisterIndex; ++i) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1892:91: note: while referencing ‘allocator’ 1892 | ColoringAllocator<bank, Briggs, InterferenceEdge<uint16_t, uint32_t>> allocator(m_code, m_tmpWidth, m_useCounts, unspillableTmps); | ^~~~~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp: In member function ‘void JSC::B3::Air::{anonymous}::GraphColoringRegisterAllocation::allocateOnBank() [with JSC::B3::Bank bank = JSC::B3::FP]’: ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1559:29: warning: array subscript ‘JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::FP, JSC::B3::Air::{anonymous}::IRC, JSC::B3::Air::{anonymous}::InterferenceEdge<short unsigned int, unsigned int> >[0]’ is partly outside array bounds of ‘JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::FP, JSC::B3::Air::{anonymous}::Briggs, JSC::B3::Air::{anonymous}::InterferenceEdge<unsigned int, long unsigned int> > [1]’ [-Warray-bounds] 1559 | m_coloredTmp.resize(m_lastPrecoloredRegisterIndex + 1); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1895:91: note: while referencing ‘allocator’ 1895 | ColoringAllocator<bank, Briggs, InterferenceEdge<uint32_t, uint64_t>> allocator(m_code, m_tmpWidth, m_useCounts, unspillableTmps); | ^~~~~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1560:35: warning: array subscript ‘JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::FP, JSC::B3::Air::{anonymous}::IRC, JSC::B3::Air::{anonymous}::InterferenceEdge<short unsigned int, unsigned int> >[0]’ is partly outside array bounds of ‘JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::FP, JSC::B3::Air::{anonymous}::Briggs, JSC::B3::Air::{anonymous}::InterferenceEdge<unsigned int, long unsigned int> > [1]’ [-Warray-bounds] 1560 | for (unsigned i = 1; i <= m_lastPrecoloredRegisterIndex; ++i) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1895:91: note: while referencing ‘allocator’ 1895 | ColoringAllocator<bank, Briggs, InterferenceEdge<uint32_t, uint64_t>> allocator(m_code, m_tmpWidth, m_useCounts, unspillableTmps); | ^~~~~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1544:21: warning: array subscript ‘const JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::FP, JSC::B3::Air::{anonymous}::IRC, JSC::B3::Air::{anonymous}::InterferenceEdge<short unsigned int, unsigned int> >[0]’ is partly outside array bounds of ‘JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::FP, JSC::B3::Air::{anonymous}::Briggs, JSC::B3::Air::{anonymous}::InterferenceEdge<unsigned int, long unsigned int> > [1]’ [-Warray-bounds] 1544 | dataLog(m_code); | ^~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1895:91: note: while referencing ‘allocator’ 1895 | ColoringAllocator<bank, Briggs, InterferenceEdge<uint32_t, uint64_t>> allocator(m_code, m_tmpWidth, m_useCounts, unspillableTmps); | ^~~~~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1544:21: warning: array subscript ‘const JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::FP, JSC::B3::Air::{anonymous}::IRC, JSC::B3::Air::{anonymous}::InterferenceEdge<short unsigned int, unsigned int> >[0]’ is partly outside array bounds of ‘JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::FP, JSC::B3::Air::{anonymous}::Briggs, JSC::B3::Air::{anonymous}::InterferenceEdge<unsigned int, long unsigned int> > [1]’ [-Warray-bounds] 1544 | dataLog(m_code); | ^~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1895:91: note: while referencing ‘allocator’ 1895 | ColoringAllocator<bank, Briggs, InterferenceEdge<uint32_t, uint64_t>> allocator(m_code, m_tmpWidth, m_useCounts, unspillableTmps); | ^~~~~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1544:21: warning: array subscript ‘const JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::FP, JSC::B3::Air::{anonymous}::IRC, JSC::B3::Air::{anonymous}::InterferenceEdge<short unsigned int, unsigned int> >[0]’ is partly outside array bounds of ‘JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::FP, JSC::B3::Air::{anonymous}::Briggs, JSC::B3::Air::{anonymous}::InterferenceEdge<unsigned int, long unsigned int> > [1]’ [-Warray-bounds] 1544 | dataLog(m_code); | ^~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1895:91: note: while referencing ‘allocator’ 1895 | ColoringAllocator<bank, Briggs, InterferenceEdge<uint32_t, uint64_t>> allocator(m_code, m_tmpWidth, m_useCounts, unspillableTmps); | ^~~~~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1559:29: warning: array subscript ‘JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::FP, JSC::B3::Air::{anonymous}::IRC, JSC::B3::Air::{anonymous}::InterferenceEdge<short unsigned int, unsigned int> >[0]’ is partly outside array bounds of ‘JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::FP, JSC::B3::Air::{anonymous}::Briggs, JSC::B3::Air::{anonymous}::InterferenceEdge<short unsigned int, unsigned int> > [1]’ [-Warray-bounds] 1559 | m_coloredTmp.resize(m_lastPrecoloredRegisterIndex + 1); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1892:91: note: while referencing ‘allocator’ 1892 | ColoringAllocator<bank, Briggs, InterferenceEdge<uint16_t, uint32_t>> allocator(m_code, m_tmpWidth, m_useCounts, unspillableTmps); | ^~~~~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1560:35: warning: array subscript ‘JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::FP, JSC::B3::Air::{anonymous}::IRC, JSC::B3::Air::{anonymous}::InterferenceEdge<short unsigned int, unsigned int> >[0]’ is partly outside array bounds of ‘JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::FP, JSC::B3::Air::{anonymous}::Briggs, JSC::B3::Air::{anonymous}::InterferenceEdge<short unsigned int, unsigned int> > [1]’ [-Warray-bounds] 1560 | for (unsigned i = 1; i <= m_lastPrecoloredRegisterIndex; ++i) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1892:91: note: while referencing ‘allocator’ 1892 | ColoringAllocator<bank, Briggs, InterferenceEdge<uint16_t, uint32_t>> allocator(m_code, m_tmpWidth, m_useCounts, unspillableTmps); | ^~~~~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1544:21: warning: array subscript ‘const JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::FP, JSC::B3::Air::{anonymous}::IRC, JSC::B3::Air::{anonymous}::InterferenceEdge<short unsigned int, unsigned int> >[0]’ is partly outside array bounds of ‘JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::FP, JSC::B3::Air::{anonymous}::Briggs, JSC::B3::Air::{anonymous}::InterferenceEdge<short unsigned int, unsigned int> > [1]’ [-Warray-bounds] 1544 | dataLog(m_code); | ^~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1892:91: note: while referencing ‘allocator’ 1892 | ColoringAllocator<bank, Briggs, InterferenceEdge<uint16_t, uint32_t>> allocator(m_code, m_tmpWidth, m_useCounts, unspillableTmps); | ^~~~~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1544:21: warning: array subscript ‘const JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::FP, JSC::B3::Air::{anonymous}::IRC, JSC::B3::Air::{anonymous}::InterferenceEdge<short unsigned int, unsigned int> >[0]’ is partly outside array bounds of ‘JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::FP, JSC::B3::Air::{anonymous}::Briggs, JSC::B3::Air::{anonymous}::InterferenceEdge<short unsigned int, unsigned int> > [1]’ [-Warray-bounds] 1544 | dataLog(m_code); | ^~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1892:91: note: while referencing ‘allocator’ 1892 | ColoringAllocator<bank, Briggs, InterferenceEdge<uint16_t, uint32_t>> allocator(m_code, m_tmpWidth, m_useCounts, unspillableTmps); | ^~~~~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1544:21: warning: array subscript ‘const JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::FP, JSC::B3::Air::{anonymous}::IRC, JSC::B3::Air::{anonymous}::InterferenceEdge<short unsigned int, unsigned int> >[0]’ is partly outside array bounds of ‘JSC::B3::Air::{anonymous}::ColoringAllocator<JSC::B3::FP, JSC::B3::Air::{anonymous}::Briggs, JSC::B3::Air::{anonymous}::InterferenceEdge<short unsigned int, unsigned int> > [1]’ [-Warray-bounds] 1544 | dataLog(m_code); | ^~~~~~ ../../Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1892:91: note: while referencing ‘allocator’ 1892 | ColoringAllocator<bank, Briggs, InterferenceEdge<uint16_t, uint32_t>> allocator(m_code, m_tmpWidth, m_useCounts, unspillableTmps); | ^~~~~~~~~ The warnings can be silenced like this: diff --git a/Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp b/Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp index e7a20e90ab28..82067c5b3c2b 100644 --- a/Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp +++ b/Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp @@ -1541,7 +1541,9 @@ public: if (!reg) { dataLog("FATAL: No color for ", tmp, "\n"); dataLog("Code:\n"); +IGNORE_WARNINGS_BEGIN("array-bounds") dataLog(m_code); +IGNORE_WARNINGS_END RELEASE_ASSERT_NOT_REACHED(); } return reg; @@ -1556,12 +1558,14 @@ protected: void initializePrecoloredTmp() { +IGNORE_WARNINGS_BEGIN("array-bounds") m_coloredTmp.resize(m_lastPrecoloredRegisterIndex + 1); for (unsigned i = 1; i <= m_lastPrecoloredRegisterIndex; ++i) { Tmp tmp = TmpMapper::tmpFromAbsoluteIndex(i); ASSERT(tmp.isReg()); m_coloredTmp[i] = tmp.reg(); } +IGNORE_WARNINGS_END } bool mayBeCoalesced(Arg left, Arg right) I'm baffled because the first case does not appear to contain any array access, while the second case appears to be clearly safe given that m_coloredTmp is resized to m_lastPrecoloredRegisterIndex + 1 and the code does not go any higher than this. Looks like a false-positive to me, so my proposal is to just commit the IGNORE_WARNINGS macros unless somebody else wants to investigate further.
Attachments
Patch (1.89 KB, patch)
2021-04-19 13:37 PDT, Michael Catanzaro 		no flags
DetailsFormatted DiffDiff
Patch for landing (2.08 KB, patch)
2021-04-20 16:11 PDT, Michael Catanzaro 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Michael Catanzaro
Comment 1 2021-04-19 13:37:12 PDT
Created attachment 426471 [details] Patch
Darin Adler
Comment 2 2021-04-19 15:01:41 PDT
Comment on attachment 426471 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426471&action=review > Source/JavaScriptCore/ChangeLog:8 > + These warnings don't make any sense to me. Suppress them. If the warnings don’t make sense, we may need a comment at each of these two sites. > Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1546 > +IGNORE_WARNINGS_BEGIN("array-bounds") > dataLog(m_code); > +IGNORE_WARNINGS_END How did you settle on using IGNORE_WARNINGS_BEGIN vs. IGNORE_GCC_WARNINGS_BEGIN?
Michael Catanzaro
Comment 3 2021-04-19 16:03:56 PDT
Comment on attachment 426471 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426471&action=review >> Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp:1546 >> +IGNORE_WARNINGS_END > > How did you settle on using IGNORE_WARNINGS_BEGIN vs. IGNORE_GCC_WARNINGS_BEGIN? I didn't even consider it. I'll switch it to IGNORE_GCC_WARNINGS_BEGIN since clang isn't complaining here and this seems more likely to be a GCC bug than anything Clang is likely to warn about. And I'll add comments pointing to this issue.
Michael Catanzaro
Comment 4 2021-04-20 16:11:13 PDT
Created attachment 426608 [details] Patch for landing
EWS
Comment 5 2021-04-20 16:57:50 PDT
Committed r276329 (236809@main): <https://commits.webkit.org/236809@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 426608 [details].
Ling Ho
Comment 6 2021-04-23 02:39:06 PDT
rdar://76926261
Note You need to log in before you can comment on or make changes to this bug.