Bug 99578 - [11/12 Regression] gcc-11 -Warray-bounds or -Wstringop-overread warning when accessing a pointer from integer literal
Summary: [11/12 Regression] gcc-11 -Warray-bounds or -Wstringop-overread warning when ...
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 11.0
: P3 normal
Target Milestone: 11.3
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
: 100190 100309 100680 100834 102037 (view as bug list)
Depends on:
Blocks: Warray-bounds
  Show dependency treegraph
 
Reported: 2021-03-13 14:23 UTC by Arnd Bergmann
Modified: 2021-09-14 18:46 UTC (History)
10 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2021-03-15 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Arnd Bergmann 2021-03-13 14:23:37 UTC
This snippet from the Linux kernel reads a data structure from an architecturally defined location in memory into a local copy:

struct sharpsl_param_info {
  unsigned int comadj_keyword;
};
extern struct sharpsl_param_info sharpsl_param;
typedef unsigned long __kernel_size_t;
extern void * memcpy(void *, const void *, __kernel_size_t);
void sharpsl_save_param(void)
{
 memcpy(&sharpsl_param, (void *)(0xe8ffc000), sizeof(struct sharpsl_param_info));
}

With gcc-11, this now triggers a -Wstringop-overread warning on x86:

arch/arm/common/sharpsl_param.i: In function ‘sharpsl_save_param’:
arch/arm/common/sharpsl_param.i:11:2: warning: ‘memcpy’ reading 4 bytes from a region of size 0 [-Wstringop-overread]
   11 |  memcpy(&sharpsl_param, (void *)(0xe8ffc000), sizeof(struct sharpsl_param_info));


I tried to reproduce this on godbolt.org, which apparently has a slightly different snapshot version and instead produces -Warray-bounds warning for the same input: https://godbolt.org/z/ve6h6b

I could not find a way to avoid this warning, other than turning off the entire warning option globally or with a pragma. Accessing a pointer from a literal integer value is not too unusual in the kernel and should not cause a warning.
Comment 1 Martin Sebor 2021-03-13 20:40:41 UTC
The warning is by design.  Its purpose is to detect invalid accesses at non-zero offsets to null pointers, like in the memset call below:

  struct S { int a, b[4]; };

  void f (struct S *p)
  {
    if (p) return;
    memset (p->b, 0, 4 * sizeof p->b);
  }

For now, I recommend suppressing the warning either by #pragma GCC diagnostic or by making the pointer volatile.  In the future, providing an attribute to annotate constant addresses with (or extending the io() attribute supported by some targets to all targets) might be another way to avoid it.
Comment 2 Arnd Bergmann 2021-03-13 21:40:20 UTC
Ok, I see. Thanks for the explanation!

I found a couple other instances (so far all false positive) and will see if any of them have a sane workaround. I'll probably just turn off both flags globally for the kernel otherwise.
Comment 3 Arnd Bergmann 2021-03-13 22:38:23 UTC
After some more analysis, I found that the -Wstringop-overread warning only happens here (and presumably in all the other cases I found) because I disabled -Warray-bounds for gcc-11.

I'm still looking at -Warray-bounds to see what has changed here. There were some interesting findings from that one, but the number of added warnings made it hard to keep track of what is going on. It appears that the -Wstringop-overread warnings mostly a subset of those.
Comment 4 Martin Sebor 2021-03-14 00:57:28 UTC
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.

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?

$ 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=]
mm/mempolicy.c:3001:26: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
mm/mempolicy.c:3001:26: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
mm/mempolicy.c:3001:26: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
mm/mempolicy.c:3001:26: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
mm/mempolicy.c:3001:26: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
mm/mempolicy.c:3001:26: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
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]
drivers/gpu/drm/i915/intel_pm.c:3058:9: warning: ‘intel_print_wm_latency’ reading 16 bytes from a region of size 10 [-Wstringop-overread]
drivers/gpu/drm/i915/intel_pm.c:3059:9: warning: ‘intel_print_wm_latency’ reading 16 bytes from a region of size 10 [-Wstringop-overread]
drivers/gpu/drm/i915/intel_pm.c:3086:9: warning: ‘intel_print_wm_latency’ reading 16 bytes from a region of size 10 [-Wstringop-overread]
drivers/gpu/drm/i915/intel_pm.c:3087:9: warning: ‘intel_print_wm_latency’ reading 16 bytes from a region of size 10 [-Wstringop-overread]
drivers/gpu/drm/i915/intel_pm.c:3088:9: warning: ‘intel_print_wm_latency’ reading 16 bytes from a region of size 10 [-Wstringop-overread]
drivers/gpu/drm/i915/intel_pm.c:3103:9: warning: ‘intel_print_wm_latency’ reading 16 bytes from a region of size 10 [-Wstringop-overread]
drivers/gpu/drm/i915/intel_pm.c:3104:9: warning: ‘intel_print_wm_latency’ reading 16 bytes from a region of size 10 [-Wstringop-overread]
drivers/gpu/drm/i915/intel_pm.c:3105:9: warning: ‘intel_print_wm_latency’ reading 16 bytes from a region of size 10 [-Wstringop-overread]
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]

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
Comment 5 Arnd Bergmann 2021-03-14 11:54:57 UTC
(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
Comment 6 Arnd Bergmann 2021-03-14 21:25:51 UTC
I figured out the qnx4 warning in the end: https://godbolt.org/z/hvqjr3

struct qnx4_inode_entry {
    char di_status;
    union {
        struct {
            char di_fname[16];
            char di_pad[32];
        };

        struct {
            char dl_fname[48];
        };
    };
};

int qnx4_readdir(struct qnx4_inode_entry *de)
{
    if (!de->di_fname[0])
        return 0;
    if (de->di_status)
        return __builtin_strnlen(de->di_fname, sizeof(de->di_fname));
    else
        return __builtin_strnlen(de->dl_fname, sizeof(de->dl_fname));
    return 0;
}

This produces

<source>:22:16: warning: '__builtin_strnlen' specified bound 48 exceeds source size 16 [-Wstringop-overread]

because the first access on the object seems to decide which layout is assumed.
Changing (!de->di_fname[0]) to (!de->dl_fname[0]) shuts up the warning since that is a longer field. This is probably enough as a workaround, if you can confirm that the behavior of the compiler is also intentional for this input.
Comment 7 Richard Biener 2021-03-15 08:38:35 UTC
Note heuristically 0xe8ffc000 isn't likely such an offset from a NULL pointer object because the object would be quite large.

The diagnostic could maybe also clarify that it assumes 0xe8ffc000 is an offsetted NULL pointer.
Comment 8 Martin Sebor 2021-03-15 19:57:33 UTC
(In reply to Arnd Bergmann from comment #6)
> I figured out the qnx4 warning in the end: https://godbolt.org/z/hvqjr3

The false positive is a known problem caused by redundancy elimination (the FRE/PRE passes) substituting one member for another when they both refer to the same address.  It often happens with union members that share the same starting offset but can also happen with struct members in code that refers to both the end of one and the beginning of the next member with no intervening padding.

There's no particular reason why it picks one or the other member (maybe it picks whichever it sees first in some order, I don't know), but the warning only triggers when it substitutes the smaller of the two members in a larger access.  In the strnlen test case w/o sanitization it happens to pick the bigger member.  This can be seen in the output of -fdump-tree-pre-details:

Inserted pretmp_12 = &de_9(D)->D.2393.D.2392.dl_fname;
 in predecessor 3 (0004)
Replaced &de_9(D)->D.2393.D.2392.dl_fname with pretmp_12 in all uses of _5 = &de_9(D)->D.2393.D.2392.dl_fname;
Replaced &de_9(D)->D.2393.D.2389.di_fname with pretmp_12 in all uses of _3 = &de_9(D)->D.2393.D.2389.di_fname;
Removing dead stmt _3 = &de_9(D)->D.2393.D.2389.di_fname;
Removing dead stmt _5 = &de_9(D)->D.2393.D.2392.dl_fname;

The instrumentation introduced by the sanitizers changes the IL in ways that affect the choices made by subsequent transformations.  In this case, the sanitizer first inserts a call to __builtin___tsan_read1() to record the access to di_fname[0].  The sanitizers run after PRE but before some FRE iterations.  The resulting IL from the thread sanitizer looks like this:

  <bb 2> [local count: 1073741824]:
  _13 = __builtin_return_address (0);
  __builtin___tsan_func_entry (_13);
  _5 = &de_9(D)->D.2390.D.2386.di_fname[0];
  __builtin___tsan_read1 (_5);                    <<< record access to di_fname[0]
  _1 = de_9(D)->D.2390.D.2386.di_fname[0];
  if (_1 == 0)
    goto <bb 7>; [34.00%]
  else
    goto <bb 3>; [66.00%]

  <bb 7> [local count: 365072224]:
  goto <bb 6>; [100.00%]

  <bb 3> [local count: 708669601]:
  _3 = &de_9(D)->di_status;
  __builtin___tsan_read1 (_3);
  _2 = de_9(D)->di_status;
  pretmp_12 = &de_9(D)->D.2390.D.2389.dl_fname;   <<< temporary added by PRE
  if (_2 != 0)
    goto <bb 4>; [50.00%]
  else
    goto <bb 5>; [50.00%]

  <bb 4> [local count: 354334800]:
  _4 = __builtin_strnlen (pretmp_12, 16);
  _11 = (int) _4;
  goto <bb 6>; [100.00%]

  <bb 5> [local count: 354334800]:
  _6 = __builtin_strnlen (pretmp_12, 48);
  _10 = (int) _6;

  <bb 6> [local count: 1073741824]:
  # _7 = PHI <0(7), _11(4), _10(5)>
  __builtin___tsan_func_exit ();
  return _7;

}

The IL above is then changed by FRE which replaces dl_fname with di_fname:

Value numbering stmt = pretmp_12 = &de_9(D)->D.2390.D.2389.dl_fname;
Setting value number of pretmp_12 to _5 (changed)
_5 is available for _5
Replaced &de_9(D)->D.2390.D.2389.dl_fname with _5 in all uses of pretmp_12 = &de_9(D)->D.2390.D.2389.dl_fname;

The warning is then issued by the strlen pass that runs after FRE5 and that sees the IL below:

  <bb 2> [local count: 1073741824]:
  _13 = __builtin_return_address (0);
  __builtin___tsan_func_entry (_13);
  _5 = &de_9(D)->D.2393.D.2389.di_fname[0];   <<< inserted by sanitizer
  __builtin___tsan_read1 (_5);                <<< record access to di_fname[0] 
  _1 = de_9(D)->D.2393.D.2389.di_fname[0];    <<< inserted by FRE5
  if (_1 == 0)
    goto <bb 6>; [34.00%]
  else
    goto <bb 3>; [66.00%]

  <bb 3> [local count: 708669601]:
  _3 = &de_9(D)->di_status;
  __builtin___tsan_read1 (_3);
  _2 = de_9(D)->di_status;
  if (_2 != 0)
    goto <bb 4>; [50.00%]
  else
    goto <bb 5>; [50.00%]

  <bb 4> [local count: 354334800]:
  _4 = strnlen (_5, 16);
  _11 = (int) _4;
  goto <bb 6>; [100.00%]

  <bb 5> [local count: 354334800]:
  _6 = strnlen (_5, 48);                      <<< -Wstringop-overread
  _10 = (int) _6;

  <bb 6> [local count: 1073741824]:
  # _7 = PHI <0(2), _11(4), _10(5)>
  __builtin___tsan_func_exit ();
  return _7;

}

I'm hoping to change PRE/FRE in GCC 12 to replace the member substitution with one involving &object + offsetof (type, memer).  That will avoid the false positives in these cases with no adverse impact on optimization.
Comment 9 Martin Sebor 2021-03-15 20:24:09 UTC
(In reply to Richard Biener from comment #7)
> Note heuristically 0xe8ffc000 isn't likely such an offset from a NULL
> pointer object because the object would be quite large.
> 
> The diagnostic could maybe also clarify that it assumes 0xe8ffc000 is an
> offsetted NULL pointer.

I can do that in stage 1 when I convert the warning to use the access_ref class (that exposes this information).

A better solution we discussed with Jeff is to issue -Wnonnull when a member access through a null pointer is first detected.  Using something like __builtin_warning for that would help avoid false positives when this happens early on (in the test case in comment #1 that's in EVRP).
Comment 10 Andrew Pinski 2021-04-21 19:34:40 UTC
*** Bug 100190 has been marked as a duplicate of this bug. ***
Comment 11 Martin Sebor 2021-04-28 16:11:36 UTC
*** Bug 100309 has been marked as a duplicate of this bug. ***
Comment 12 Andi Kleen 2021-05-01 15:08:53 UTC
It looks to me separate bugs are mixed together here.

For example I looked at the preallocate_pmd warning again and I don't think there is any union there. Also I noticed that when I replace the *foo[N] with **foo it disappears. So I think that is something different.

So there seem to be instances where such warnings happen without union members. Perhaps that one (and perhaps some others) need to be reanalyzed.

I also looked at the intel_pm.c and I think that one is a real kernel bug, where the field accessed is really too small. I'll submit a patch for that.
Comment 13 Martin Sebor 2021-05-19 15:07:00 UTC
*** Bug 100680 has been marked as a duplicate of this bug. ***
Comment 14 Andrew Cooper 2021-05-19 18:01:02 UTC
I too have had what appears to be this bug, raised against a microkernel project.

The logic is a test case involving interactions at the x86-64 lower canonical boundary, and reads like so:

    ...
    uint64_t *ptr = (void *)0x00007ffffffffff8ul;

    memcpy(ptr, "\x90\x90\xf\xbxen\x90", 8);
    ...

This yields:

    include/xtf/libc.h:36:37: error: ‘__builtin_memcpy’ offset [0, 7] is out of the bounds [0, 0] [-Werror=array-bounds]
       36 | #define memcpy(d, s, n)             __builtin_memcpy(d, s, n)
          |                                     ^~~~~~~~~~~~~~~~~~~~~~~~~
    main.c:81:5: note: in expansion of macro ‘memcpy’
       81 |     memcpy(ptr, "\x90\x90\xf\xbxen\x90", 8);
          |     ^~~~~~

It is worth pointing out that it is common for kernels to have some virtual addresses derived from compile-time constants, notably the fixmap and frametable.
Comment 15 Andrew Cooper 2021-05-19 19:19:12 UTC
(In reply to Martin Sebor from comment #1)
> For now, I recommend suppressing the warning either by #pragma GCC
> diagnostic or by making the pointer volatile.

Trying this with the code sample from comment 14 doesn't work.

Not only does the -Werror=array-bounds diagnostic remain, a second is picked up from:

    passing argument 1 of ‘__builtin_memcpy’ discards ‘volatile’ qualifier from pointer target type [-Werror=discarded-qualifiers]
Comment 16 Martin Sebor 2021-05-19 20:48:20 UTC
It's the pointer itself that needs to be volatile to keep GCC from determining its value.  This shows the difference:

$ cat pr99578-15.c && gcc -O2 -S -Wall pr99578-15.c
void f (void)
{ 
  void* ptr = (void *)0x00007ffffffffff8ul;
  __builtin_memcpy (ptr, "\x90\x90\xf\xbxen\x90", 8);   // warning
}

void g (void)
{
  void* volatile ptr = (void *)0x00007ffffffffff8ul;
  __builtin_memcpy (ptr, "\x90\x90\xf\xbxen\x90", 8);   // okay
}
pr99578-15.c: In function ‘f’:
pr99578-15.c:4:3: warning: ‘__builtin_memcpy’ offset [0, 7] is out of the bounds [0, 0] [-Warray-bounds]
    4 |   __builtin_memcpy (ptr, "\x90\x90\xf\xbxen\x90", 8);   // warning
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Alternatively, store the address a global (modifiable) variable.
Comment 17 Martin Sebor 2021-05-30 23:40:08 UTC
*** Bug 100834 has been marked as a duplicate of this bug. ***
Comment 18 Andrew Pinski 2021-08-24 16:03:59 UTC
*** Bug 102037 has been marked as a duplicate of this bug. ***