[Bug tree-optimization/105420] New: Bogus -Warray-bounds with non-compile time-constant variable

[Bug tree-optimization/105420] New: Bogus -Warray-bounds with non-compile time-constant variable

[byteslice at airmail dot cc]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105420

            Bug ID: 105420
           Summary: Bogus -Warray-bounds with non-compile time-constant
                    variable
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: byteslice at airmail dot cc
  Target Milestone: ---

Created attachment 52894
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52894&action=edit
Reduced example

On gcc 12.0.1 20220413 (Fedora 36 Beta), with c++ -O1 -fexpensive-optimizations
-ftree-vrp -Werror=array-bounds, the attachment fails to compile, with the
following message:

<source>: In function 'void Initialize(int)':
<source>:9:53: error: array subscript -1 is below array bounds of 'int [8]'
[-Werror=array-bounds]
    9 |   int phys_core = VirtualToPhysicalCoreMap[virt_core];
      |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
<source>:2:5: note: while referencing 'VirtualToPhysicalCoreMap'
    2 | int VirtualToPhysicalCoreMap[8];
      |     ^~~~~~~~~~~~~~~~~~~~~~~~
cc1plus: some warnings being treated as errors
Compiler returned: 1

virt_core does not have a compile time-constant evaluation, so this warning is
bogus. The bogus warning does not appear in older versions of GCC.

Removing either -fexpensive-optimizations or -ftree-vrp allows compilation to
succeed.

[Bug tree-optimization/105420] Bogus -Warray-bounds with non-compile time-constant variable

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105420

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
           Keywords|                            |diagnostic
             Blocks|                            |56456
   Last reconfirmed|                            |2022-04-29
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC threads the Initialize_type check and effectively sees

  if (Initialize_type)
    {
      if (virt_core >= 0)
        return;
      int phys_core = VirtualToPhysicalCoreMap[virt_core]; // virt_core is
negative
      if (phys_core)
        external();
    }
  else
    {
       int phys_core = VirtualToPhysicalCoreMap[virt_core];
       if (phys_core && Initialize_type == Kernel)
         external();
    }

it should probably warn that virt_core is negative, not that it is literally -1
here.

I agree the diagnostic can be improved, but telling the user the execution
path considered is probably difficult (at best we might somehow be able to
emit a maybe warning).

Still there's an obvious defect in your program that's worth fixing, possibly
by refactoring of this code.


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56456
[Bug 56456] [meta-bug] bogus/missing -Warray-bounds

[Bug tree-optimization/105420] Bogus -Warray-bounds with non-compile time-constant variable

[byteslice at airmail dot cc]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105420

--- Comment #2 from Liam White <byteslice at airmail dot cc> ---
Created attachment 52906
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52906&action=edit
Preprocessed source

The first attachment is automatically and manually reduced from the following
source:

ResultCode KThread::Initialize(KThreadFunction func, uintptr_t arg, VAddr
user_stack_top, s32 prio,
                               s32 virt_core, KProcess* owner, ThreadType type)
{
    // Assert parameters are valid.
    ASSERT((type == ThreadType::Main) || (type == ThreadType::Dummy) ||
           (Svc::HighestThreadPriority <= prio && prio <=
Svc::LowestThreadPriority));
    ASSERT((owner != nullptr) || (type != ThreadType::User));
    ASSERT(0 <= virt_core && virt_core <
static_cast<s32>(Common::BitSize<u64>()));

    // Convert the virtual core to a physical core.
    const s32 phys_core = Core::Hardware::VirtualToPhysicalCoreMap[virt_core];
    ASSERT(0 <= phys_core && phys_core <
static_cast<s32>(Core::Hardware::NUM_CPU_CORES));

    // ...
}

And it produces this compiler error (compile this preprocessed source with -O2
-Werror=array-bounds -std=gnu++20):

In static member function ‘static constexpr _Tp& std::__array_traits<_Tp,
_Nm>::_S_ref(const _Tp (&)[_Nm], std::size_t) [with _Tp = int; long unsigned
int _Nm = 64]’,
    inlined from ‘constexpr const std::array<_Tp, _Nm>::value_type&
std::array<_Tp, _Nm>::operator[](size_type) const [with _Tp = int; long
unsigned int _Nm = 64]’ at k_thread.cpp:59748:25,
    inlined from ‘ResultCode
Kernel::KThread::Initialize(Kernel::KThreadFunction, uintptr_t, VAddr, s32,
s32, Kernel::KProcess*, Kernel::ThreadType)’ at k_thread.cpp:102280:77:
k_thread.cpp:59638:36: error: array subscript 64 is above array bounds of
‘std::__array_traits<int, 64>::_Type’ {aka ‘const int [64]’}
[-Werror=array-bounds]
59638 |       { return const_cast<_Tp&>(__t[__n]); }
      |                                 ~~~^
k_thread.cpp: In member function ‘ResultCode
Kernel::KThread::Initialize(Kernel::KThreadFunction, uintptr_t, VAddr, s32,
s32, Kernel::KProcess*, Kernel::ThreadType)’:
k_thread.cpp:77557:51: note: while referencing
‘Core::Hardware::VirtualToPhysicalCoreMap’
77557 | constexpr std::array<s32, Common::BitSize<u64>()>
VirtualToPhysicalCoreMap{
      |                                                  
^~~~~~~~~~~~~~~~~~~~~~~~
cc1plus: some warnings being treated as errors

I generally believe that this is a bug in the compiler as the array is
similarly never accessed with the constant value 64.