[Bug c/105679] New: erroneous -Warray-bounds warning with sanitizer

[Bug c/105679] New: erroneous -Warray-bounds warning with sanitizer

[kees at outflux dot net]

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

            Bug ID: 105679
           Summary: erroneous -Warray-bounds warning with sanitizer
           Product: gcc
           Version: 12.1.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: kees at outflux dot net
  Target Milestone: ---

Created attachment 53010
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53010&action=edit
test case minimized as much as possible

Some combination of things (likely triggered by -fsanitize=shift and an array
index being used with "<<" operator) is causing erroneous warnings about array
bounds accesses:

$ gcc -O2 -Warray-bounds -fno-strict-aliasing -fno-strict-overflow
-fsanitize=shift  -fsanitize-coverage=trace-pc -c -o /dev/null test.c
test.c: In function 'work.isra':
test.c:31:32: warning: array subscript 32 is above array bounds of 'struct
object[2]' [-Warray-bounds]
   31 |         entry = instance->array[irq];
      |                 ~~~~~~~~~~~~~~~^~~~~
test.c:19:23: note: while referencing 'array'
   19 |         struct object array[ELEMENTS];
      |                       ^~~~~

[Bug c/105679] erroneous -Warray-bounds warning with sanitizer

[kees at outflux dot net]

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

--- Comment #1 from Kees Cook <kees at outflux dot net> ---
The Linux kernel has encountered at least two of these (seen as specifically
"array subscript 32", though the root cause may be causing many others:

../drivers/net/wireless/ath/ath9k/mac.c:373:22: warning: array subscript 32 is
above array bounds of 'struct ath9k_tx_queue_info[10]' [-Warray-bounds]
../arch/x86/kvm/ioapic.c:213:33: warning: array subscript 32 is above array
bounds of 'union kvm_ioapic_redirect_entry[24]' [-Warray-bounds]

[Bug tree-optimization/105679] erroneous -Warray-bounds warning with sanitizer

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
This is jump threading due to the fsanitize=shift option.
With the irq argument to work changed to int from unsigned, the work function
is inlined. 

And then the check:
        if (irq < 0 || irq >= ELEMENTS)
                return;

works.

So I don't know how to solve this really.

[Bug tree-optimization/105679] [12/13 Regression] extra -Warray-bounds warning added with -fsanitize=shift due to jump threading

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |56456
   Target Milestone|---                         |12.2


Referenced Bugs:

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

[Bug tree-optimization/105679] [12/13 Regression] extra -Warray-bounds warning added with -fsanitize=shift due to jump threading

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-07-29
             Status|UNCONFIRMED                 |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
Checking profitability of path (backwards):  bb:5 (2 insns) bb:4 (7 insns) bb:3
  Control statement insns: 2
  Overall: 7 insns
  [1] Registering jump thread: (3, 4) incoming edge;  (4, 5) normal (5, 6)
nocopy;
path: 3->4->5->6 SUCCESS

but from the following you can see BB3 is known to probably never execute.

;;   basic block 2, loop depth 0, count 1073741823 (estimated locally), maybe
hot
;;    prev block 9, next block 3, flags: (NEW, REACHABLE, VISITED)
;;    pred:       9 [always]  count:1073741823 (estimated locally)
(FALLTHRU,EXECUTABLE)
  if (irq_1(D) > 31)
    goto <bb 3>; [0.00%]
  else
    goto <bb 4>; [100.00%]

;;   basic block 3, loop depth 0, count 0 (precise), probably never executed
;;    prev block 2, next block 4, flags: (NEW, REACHABLE, VISITED)
;;    pred:       2 [never]  count:0 (precise) (TRUE_VALUE,EXECUTABLE)
  _2 = (unsigned long) irq_1(D);
  __builtin___ubsan_handle_shift_out_of_bounds (&*.Lubsan_data0, 1, _2);
;;    succ:       4 [never]  count:0 (precise) (FALLTHRU,EXECUTABLE)

;;   basic block 4, loop depth 0, count 1073741823 (estimated locally), maybe
hot
;;    prev block 3, next block 5, flags: (NEW, REACHABLE, VISITED)
;;    pred:       2 [always]  count:1073741823 (estimated locally)
(FALSE_VALUE,EXECUTABLE)
;;                3 [never]  count:0 (precise) (FALLTHRU,EXECUTABLE)
  _3 = 1 << irq_1(D);
  mask_4 = (u32) _3;
  entry = instance_5(D)->array[irq_1(D)];
  capture (mask_4);
  if (level_6(D) != 0)
    goto <bb 7>; [34.00%]
  else
    goto <bb 5>; [66.00%]
;;    succ:       7 [34.0% (guessed)]  count:365072224 (estimated locally)
(TRUE_VALUE,EXECUTABLE)
;;                5 [66.0% (guessed)]  count:708669599 (estimated locally)
(FALSE_VALUE,EXECUTABLE)

;;   basic block 5, loop depth 0, count 708669600 (estimated locally), maybe
hot
;;    prev block 4, next block 6, flags: (NEW, REACHABLE, VISITED)
;;    pred:       4 [66.0% (guessed)]  count:708669599 (estimated locally)
(FALSE_VALUE,EXECUTABLE)
  if (irq_1(D) == 1)
    goto <bb 7>; [20.97%]
  else
    goto <bb 6>; [79.03%]
;;    succ:       7 [21.0% (guessed)]  count:148608017 (estimated locally)
(TRUE_VALUE,EXECUTABLE)
;;                6 [79.0% (guessed)]  count:560061583 (estimated locally)
(FALSE_VALUE,EXECUTABLE)

[Bug tree-optimization/105679] [12/13 Regression] extra -Warray-bounds warning added with -fsanitize=shift due to jump threading

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:49ba4fdeb648c149fa7d964ba812084262c3d06f

commit r13-1891-g49ba4fdeb648c149fa7d964ba812084262c3d06f
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Jul 29 10:40:34 2022 +0200

    tree-optimization/105679 - disable backward threading of unlikely entry

    The following makes the backward threader reject threads whose entry
    edge is probably never executed according to the profile.  That in
    particular, for the testcase, avoids threading the irq == 1 check
    on the path where irq > 31, thereby avoiding spurious -Warray-bounds
    diagnostics

      if (irq_1(D) > 31)
        goto <bb 3>; [0.00%]
      else
        goto <bb 4>; [100.00%]

    ;;   basic block 3, loop depth 0, count 0 (precise), probably never
executed
      _2 = (unsigned long) irq_1(D);
      __builtin___ubsan_handle_shift_out_of_bounds (&*.Lubsan_data0, 1, _2);

      _3 = 1 << irq_1(D);
      mask_4 = (u32) _3;
      entry = instance_5(D)->array[irq_1(D)];
      capture (mask_4);
      if (level_6(D) != 0)
        goto <bb 7>; [34.00%]
      else
        goto <bb 5>; [66.00%]

    ;;   basic block 5, loop depth 0, count 708669600 (estimated locally),
maybe hot  if (irq_1(D) == 1)
        goto <bb 7>; [20.97%]
      else
        goto <bb 6>; [79.03%]

            PR tree-optimization/105679
            * tree-ssa-threadbackward.cc
            (back_threader_profitability::profitable_path_p): Avoid threading
            when the entry edge is probably never executed.

[Bug tree-optimization/105679] [12 Regression] extra -Warray-bounds warning added with -fsanitize=shift due to jump threading

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |13.0
            Summary|[12/13 Regression] extra    |[12 Regression] extra
                   |-Warray-bounds warning      |-Warray-bounds warning
                   |added with -fsanitize=shift |added with -fsanitize=shift
                   |due to jump threading       |due to jump threading

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
Should be fixed on trunk.  Can you check on the original unreduced testcase?

[Bug tree-optimization/105679] [12 Regression] extra -Warray-bounds warning added with -fsanitize=shift due to jump threading

[kees at outflux dot net]

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

--- Comment #6 from Kees Cook <kees at outflux dot net> ---
(In reply to Richard Biener from comment #5)
> Should be fixed on trunk.  Can you check on the original unreduced testcase?

Thanks! I've done test builds and can confirm these two false positives have
been eliminated:

arch/x86/kvm/emulate.c:251:27: warning: array subscript 32 is above array
bounds of 'long unsigned int[17]' [-Warray-bounds]
arch/x86/kvm/ioapic.c:213:33: warning: array subscript 32 is above array bounds
of 'union kvm_ioapic_redirect_entry[24]' [-Warray-bounds]

Is this backportable to gcc 12?

[Bug tree-optimization/105679] [12 Regression] extra -Warray-bounds warning added with -fsanitize=shift due to jump threading

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Kees Cook from comment #6)
> (In reply to Richard Biener from comment #5)
> > Should be fixed on trunk.  Can you check on the original unreduced testcase?
> 
> Thanks! I've done test builds and can confirm these two false positives have
> been eliminated:
> 
> arch/x86/kvm/emulate.c:251:27: warning: array subscript 32 is above array
> bounds of 'long unsigned int[17]' [-Warray-bounds]
> arch/x86/kvm/ioapic.c:213:33: warning: array subscript 32 is above array
> bounds of 'union kvm_ioapic_redirect_entry[24]' [-Warray-bounds]
> 
> Is this backportable to gcc 12?

Yes, I think so - I'll do after a week of no ill-fated fallout.

[Bug tree-optimization/105679] [12 Regression] extra -Warray-bounds warning added with -fsanitize=shift due to jump threading

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105679
Bug 105679 depends on bug 106495, which changed state.

Bug 106495 Summary: [13 Regression] Build fails gcc/tree-ssa-threadbackward.cc:22: gcc/vec.h:890:19: error: array subscript 4294967294 is above array bounds of 'basic_block_def* [1]' [-Werror=array-bounds]
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106495

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

[Bug tree-optimization/105679] [12 Regression] extra -Warray-bounds warning added with -fsanitize=shift due to jump threading

[kees at outflux dot net]

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

Kees Cook <kees at outflux dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |qing.zhao at oracle dot com

--- Comment #9 from Kees Cook <kees at outflux dot net> ---
Does anyone have some time to do this backport for GCC 12?

[Bug tree-optimization/105679] [12 Regression] extra -Warray-bounds warning added with -fsanitize=shift due to jump threading

[rguenth at gcc dot gnu.org]

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

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Kees Cook from comment #9)
> Does anyone have some time to do this backport for GCC 12?

I sofar refrained from doing this because of the large amount of fallout and
followup changes and I think those are not warranted on the GCC 12 branch.

[Bug tree-optimization/105679] [12 Regression] extra -Warray-bounds warning added with -fsanitize=shift due to jump threading

[kees at outflux dot net]

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

--- Comment #11 from Kees Cook <kees at outflux dot net> ---
(In reply to Richard Biener from comment #10)
> I sofar refrained from doing this because of the large amount of fallout and
> followup changes and I think those are not warranted on the GCC 12 branch.

Totally understandable! Thanks for considering it; I didn't know if it was
"easy" or not, so I thought I'd ask. :)

[Bug tree-optimization/105679] [12 Regression] extra -Warray-bounds warning added with -fsanitize=shift due to jump threading

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|12.3                        |12.4

--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 12.3 is being released, retargeting bugs to GCC 12.4.