[Bug middle-end/99578] gcc-11 -Warray-bounds or -Wstringop-overread warning when accessing a pointer from integer literal

["arnd at linaro dot org" <gcc-bugzilla@gcc.gnu.org>]

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

--- Comment #5 from Arnd Bergmann <arnd at linaro dot org> ---
(In reply to Martin Sebor from comment #4)
> Most warnings designed to detect invalid accesses (not just
> -Wstringop-overread but also -Wstringop-overflow and
> -Wformat-overflow/-truncation, -Wrestrict, and some forms of -Warray-bounds)
> use the same underlying code to determine the identity of the accessed
> object, so they all should trigger if they see a constant address.

Right, makes sense. 

> But I tested the warning with the kernel when I implemented it months ago
> and don't think I saw any instances of it (though I don't see sharpsl_param
> in any of my logs).  I still don't.  How many do you see?
> 
> Here's the list of -Wstringop- warnings in my fresh build but I'm never sure
> I use the right target.  Is allyesconfig the right one?

For brief testing I usually test 'allmodconfig', which has slightly better
coverage and is much faster to build than 'allyesconfig'. However, most of
my testing is on random configurations, with a lot of patches on top to
address all the known warnings. The sharpsl_param only shows up in
unusual builds since this is a legacy Arm platform that needs a custom
kernel configuration and is incompatible with normal armv5 builds.

Some of these tend to only show up with certain combinations of the various
sanitizers and inlining decisions such as -O2 vs -Os.

> $ grep Wstringop-over /src/linux-stable/gcc-master.log 
> arch/x86/mm/pgtable.c:437:13: warning: ‘preallocate_pmds.constprop’
> accessing 8 bytes in a region of size 0 [-Wstringop-overflow=]
> arch/x86/mm/pgtable.c:440:13: warning: ‘preallocate_pmds.constprop’
> accessing 8 bytes in a region of size 0 [-Wstringop-overflow=]
> arch/x86/mm/pgtable.c:462:9: warning: ‘free_pmds.constprop’ accessing 8
> bytes in a region of size 0 [-Wstringop-overflow=]
> arch/x86/mm/pgtable.c:455:9: warning: ‘pgd_prepopulate_user_pmd’ accessing 8
> bytes in a region of size 0 [-Wstringop-overflow=]
> arch/x86/mm/pgtable.c:464:9: warning: ‘free_pmds.constprop’ accessing 8
> bytes in a region of size 0 [-Wstringop-overflow=]

I don't see these at the moment, maybe the kernel already got fixed for them.

> mm/mempolicy.c:3001:26: warning: writing 1 byte into a region of size 0
> [-Wstringop-overflow=]

Nor this one.

> drivers/gpu/drm/i915/intel_pm.c:3093:9: warning: ‘intel_read_wm_latency’
> accessing 16 bytes in a region of size 10 [-Wstringop-overflow=]
> drivers/gpu/drm/i915/intel_pm.c:3057:9: warning: ‘intel_print_wm_latency’
> reading 16 bytes from a region of size 10 [-Wstringop-overread]

This looks like a reasonable warning in principle, though I think the code
is still correct. I have a patch for this.

> drivers/gpu/drm/i915/display/intel_dp.c:4556:22: warning:
> ‘drm_dp_channel_eq_ok’ reading 6 bytes from a region of size 4
> [-Wstringop-overread]

Different bug, similar verdict: I have a patch to work around it,
it seems reasonable to warn about it, but I think the code is correct.

Here is one that got added in gcc-11 I just couldn't figure out:

https://godbolt.org/z/sjjGc9

On this one I understand why gcc warns (pointer is compared to known
constant address), but the code is correct and I don't know how to rework the
code other than using #pragma to turn off the warning:

In file included from arch/x86/boot/compressed/misc.c:18:
In function ‘parse_elf’,
    inlined from ‘extract_kernel’ at arch/x86/boot/compressed/misc.c:442:2:
arch/x86/boot/compressed/../string.h:15:23: error: ‘__builtin_memcpy’ reading
64 bytes from a region of size 0 [-Werror=stringop-overread]
   15 | #define memcpy(d,s,l) __builtin_memcpy(d,s,l)
      |                       ^~~~~~~~~~~~~~~~~~~~~~~
arch/x86/boot/compressed/misc.c:283:9: note: in expansion of macro ‘memcpy’
  283 |         memcpy(&ehdr, output, sizeof(ehdr));
      |         ^~~~~~


This one is correct code, but has a simple workaround that does not
make the code any uglier:

    security/commoncap.c: In function ‘cap_inode_getsecurity’:
    security/commoncap.c:440:33: error: ‘memcpy’ reading 16 bytes from a region
of size 0 [-Werror=stringop-overread]
      440 |                                 memcpy(&nscap->data, &cap->data,
sizeof(__le32) * 2 * VFS_CAP_U32);
          |                                 
-       if (ret < 0)
+       if (ret < 0 || !tmpbuf)


I also got this one (with -Warray-bounds, but seems related) that looks like a
real bug in the kernel:

    net/core/skbuff.c: In function ‘skb_find_text’:
    net/core/skbuff.c:3498:26: error: array subscript ‘struct skb_seq_state[0]’
is partly outside array bounds of ‘struct ts_state[1]’ [-Werror=array-bounds]

I have a patch, but it needs to be discussed first.

> The full breakdown with the warnings forcefully disabled in the top-level
> Makefile re-enabled is below:
> 
> Diagnostic                        Count   Unique    Files
> -Wmissing-prototypes                759      248      114
> -Wunused-const-variable=            391      233       31
> -Wformat-truncation=                311      297      229
> -Wmaybe-uninitialized               158      133      103
> -Wunused-but-set-variable           143      137       88
> -Warray-bounds                       94       32       12
> -Wzero-length-bounds                 69       66       16
> -Wsuggest-attribute=format           60       26       21
> -Wnested-externs                     41        1        1
> -Woverride-init                      36       22       15
> -Wrestrict                           23       14       10
> -Wformat-overflow=                   20       19       15
> -Wempty-body                         15       15        8
> -Wstringop-overflow=                 12        7        3
> -Wmisleading-indentation             11        2        2
> -Wcast-function-type                 11        2        2
> -Wstringop-overread                  10       10        2
> -Wenum-conversion                    10       10        5
> -Warray-parameter=                    8        8        6
> -Wpacked-not-aligned                  5        3        2
> -Wold-style-declaration               3        3        2
> -Wignored-qualifiers                  1        1        1
> -Wconflicts-sr                        1        1        1
> -Wconflicts-rr                        1        1        1

Right, I see roughly the same, and I have patches for a lot of these.
Lee Jones has sent several thousand patches over the past year to get
to the point where we can almost turn most of these back on again.

I particularly want to finally enable -Wmissing-prototypes, it
seems we are getting fairly close to that (it used to be thousands
of warnings).

Linus Torvalds decided that Wmaybe-uninitialized was no longer worth
enabling after the different inlining choices in (iirc) gcc-10 made the
ratio of false-positive much worse than it was. I had spent a lot of time
on addressing issues on that, but have pretty much given up on it as well.
Fortunately, clang seems to be doing a reasonable job at finding the
actual bugs here with -Wsometimes-uninitialized, so we still catch most
of the important ones. Also, more kernels are now built with the
-fplugin-arg-structleak_plugin-byref-all option that initializes all
escaping local variables to zero, which effectively disables the
Wmaybe-uninitialized output anyway but at least makes the behavior more
deterministic.

I have enabled -Wextra for my local builds now, which mostly works.
The interesting options I still leave disabled are -Wpsabi,
-Wshift-negative-value, -Waddress-of-packed-member, -Wframe-address,
-Wstringop-truncation, -Wempty-body, -Wunused-but-set-variable, and
-Wmissing-prototypes.

For reference, here is the list of unique warnings I made a year ago:

5915026 -Wredundant-decls
4828168 -Wpacked
2241214 -Wswitch-enum
1266195 -Wsign-compare
1171275 -Winline
1141263 -Wlarger-than=
1075660 -Wmissing-field-initializers
 854764 -Wcast-align
 621102 -Wswitch-default
 619302 -Wshadow
 550802 -Wformat=
 305056 -Wunused-macros
 285910 -Wpointer-sign
 184929 -Wsuggest-attribute=malloc
  79124 -Wsuggest-attribute=pure
  68675 -Wshift-overflow=
  43685 -Wunused-const-variable=
  40664 -Wimplicit-fallthrough=
  37048 -Waggregate-return
  35789 -Wunsuffixed-float-constants
  35718 -Wsuggest-attribute=cold
  28348 -Wsuggest-attribute=const
  25315 -Wtype-limits
  21354 -Woverride-init
  19518 -Wmissing-prototypes
  15989 -Wuninitialized
  11801 -Wduplicated-branches
   7969 -Wunused-but-set-variable
   7826 -Woverlength-strings
   6697 -Wstack-protector
   6576 -Wformat-truncation=
   5171 -Wfloat-conversion
   4332 -Wduplicated-cond
   2784 -Wformat-nonliteral
   1972 -Wdouble-promotion
   1500 -Wempty-body
   1013 -Wformat-security
    997 -Wsuggest-attribute=noreturn
    984 -Wformat-overflow=
    743 -Wvector-operation-performance
    711 -Wcast-function-type
    671 -Wstringop-truncation
    647 -Wstack-usage=
    623 -Wsuggest-attribute=format
    393 -Wmaybe-uninitialized
    319 -Wignored-qualifiers
    307 -Wframe-larger-than=
    304 -Wold-style-declaration
    232 -Wlogical-op
    190 -Wjump-misses-init
    157 -Wframe-address
    145 -Wfloat-equal
     57 -Wundef
     26 -Wstrict-overflow
     16 -Wvla-larger-than=
     14 -Wold-style-definition
      6 -Wunused-but-set-parameter