Bug 96447 - False positive -Wstringop-overflow with -O3 due to loop unrolling
Summary: False positive -Wstringop-overflow with -O3 due to loop unrolling
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 11.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic, missed-optimization
Depends on:
Blocks: Wstringop-overflow
  Show dependency treegraph
 
Reported: 2020-08-03 20:51 UTC by Daniel Scharrer
Modified: 2022-10-23 00:34 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2020-08-03 00:00:00


Attachments
test.c (231 bytes, text/plain)
2020-08-03 20:51 UTC, Daniel Scharrer
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Scharrer 2020-08-03 20:51:59 UTC
Created attachment 48990 [details]
test.c

The attached reduced test case results in the following false positive warning at -03 (but not -O2) without any additional compiler flags with both GCC 10.2.0 and GCC 11.0.0 (compiled from git today):

test.c: In function ‘load’:
test.c:23:25: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
   23 |                 data[i] = get8u(s);
      |                 ~~~~~~~~^~~~~~~~~~
test.c:21:23: note: at offset 4 to object ‘data’ with size 4 declared here
   21 |         unsigned char data[4];
      |                       ^~~~
Comment 1 Martin Sebor 2020-08-03 22:13:50 UTC
The warning works as designed.  The false positive is caused by the assignment below in the unrolled loop with the index in RANGE [4, 5].  GCC doesn't understand that the constraint on bpp implies that i < 4, likely because the range of bpp is discontiguous.

;;   basic block 19, loop depth 0, count 0 (precise), probably never executed
;;    prev block 18, next block 20, flags: (NEW, REACHABLE, VISITED)
;;    pred:       18 [80.0% (guessed)]  count:0 (precise) (TRUE_VALUE,EXECUTABLE)
  # .MEM_3 = VDEF <.MEM_50>
  dataD.1941[i_49] = 0;
  # RANGE [4, 5] NONZERO 5
  i_2 = i_49 + 1;
  # RANGE [32, 32] NONZERO 32
  _1 = _6 + 8;
  if (bpp_25 > 32)
    goto <bb 20>; [80.00%]
  else
    goto <bb 16>; [20.00%]
;;    succ:       20 [80.0% (guessed)]  count:0 (precise) (TRUE_VALUE,EXECUTABLE)
;;                16 [20.0% (guessed)]  count:0 (precise) (FALSE_VALUE,EXECUTABLE)

;;   basic block 20, loop depth 0, count 0 (precise), probably never executed
;;    prev block 19, next block 21, flags: (NEW, REACHABLE, VISITED)
;;    pred:       19 [80.0% (guessed)]  count:0 (precise) (TRUE_VALUE,EXECUTABLE)
  # .MEM_45 = VDEF <.MEM_3>
  dataD.1941[i_2] = 0;   <<< warning


The warning can be avoided by constraining the range of bpp to [1, 4] instead (by shifting it to the right by 3 bits) and changing the loop test to i < bpp.  That also leads to GCC emitting better code.

int load(input * s)
{
    unsigned char bpp = get8u (s);

    if(bpp != 8 && bpp != 16 && bpp != 24 && bpp != 32) {
        return 0;
    }

    bpp >>= 3;

    unsigned char data[4];
    for(int i = 0; i < bpp; ++i) {
        data[i] = get8u (s);
    }
Comment 2 Daniel Scharrer 2020-08-04 17:38:19 UTC
For warnings enabled by something like -fanalyzer this might be reasonable but for a warning I enabled by default (not even requiring -Wall) the bar should ideally be a bit higher.