Bug 224782 – -Warray-bounds warning in AirAllocateRegistersByGraphColoring.cpp with GCC 11

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Michael Catanzaro]

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.

[Michael Catanzaro]

Created attachment 426471 [details]
Patch

[Darin Adler]

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 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]

Created attachment 426608 [details]
Patch for landing

[EWS]

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]

rdar://76926261