Bug 84095 - [8 Regression] false-positive -Wrestrict warnings for memcpy within array
Summary: [8 Regression] false-positive -Wrestrict warnings for memcpy within array
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 8.0.1
: P3 normal
Target Milestone: 8.0
Assignee: Martin Sebor
Keywords: diagnostic, patch
Depends on:
Reported: 2018-01-29 07:51 UTC by Arnd Bergmann
Modified: 2018-02-20 20:23 UTC (History)
3 users (show)

See Also:
Known to work:
Known to fail:
Last reconfirmed: 2018-01-29 00:00:00

linux/drivers/isdn/isdnloop/isdnloop.c, preprocessed, compressed (229.57 KB, application/gzip)
2018-01-30 17:58 UTC, Arnd Bergmann
Patch under test. (1.85 KB, patch)
2018-01-30 22:37 UTC, Martin Sebor
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Arnd Bergmann 2018-01-29 07:51:09 UTC
I see multiple new warnings for correct code in the Linux kernel for code that copies one array member into other members of the same array, reduced to:

$ cat > test.c << EOF
struct { int i; } a[8];

void f(void)
        int i;

        for (i=1; i <8; i++)
                __builtin_memcpy(&a[i], &a[0], sizeof(a[0]));

$ x86_64-linux-gcc-8.0.1 -c -Wall test.c 
test4.c: In function 'f':
test4.c:8:3: warning: '__builtin_memcpy' accessing 4 bytes at offsets 0 and 0 overlaps 4 bytes at offset 0 [-Wrestrict]
   __builtin_memcpy(&a[i], &a[0], sizeof(a[0]));
Comment 1 Jakub Jelinek 2018-01-29 10:55:18 UTC
This warning at least in its current shape doesn't belong into either -Wall or -W IMNSHO.

  if (TREE_CODE (expr) == ADDR_EXPR)
      poly_int64 off;
      tree op = TREE_OPERAND (expr, 0);

      /* Determine the base object or pointer of the reference
         and its constant offset from the beginning of the base.  */
      base = get_addr_base_and_unit_offset (op, &off);

      HOST_WIDE_INT const_off;
      if (base && off.is_constant (&const_off))
          offrange[0] += const_off;
          offrange[1] += const_off;

          /* Stash the reference for offset validation.  */
          ref = op;

          /* Also stash the constant offset for offset validation.  */
          if (TREE_CODE (op) == COMPONENT_REF)
            refoff = const_off;
          size = NULL_TREE;
          base = get_base_address (TREE_OPERAND (expr, 0));

is plain wrong.  get_addr_base_and_unit_offset is solely for computation of a constant offset (or these days poly_int64 offset).  Here you need to do get_inner_reference instead, and treat the poly_int64 bit offset (rather than byte offset) as the constant part and for the variable part try to use
value ranges and handle casts/extensions in the expression too properly.
I don't see how this warning can be enabled at all at -O0/-O1/-Og, unless you warn solely about cases where you can prove overalap, rather than warning just in case.  And the general case of the warning should be, if I don't understand something (such as the base = get_base_address (TREE_OPERAND (expr, 0)); above, I punt on the warning, rather than just giving false positives.  That is only a sure way to add the warning to the kill-list of projects that -Wno-* broken warnings, and for some users to stop using GCC.
Comment 2 Martin Sebor 2018-01-29 19:16:24 UTC
(In reply to Arnd Bergmann from comment #0)

Let me work on this.

I tested the warning with the kernel but don't recall coming across this false positive.  While I retry with the latest, how many of these do you see?
Comment 3 Arnd Bergmann 2018-01-29 19:18:48 UTC
(In reply to Martin Sebor from comment #2)
> (In reply to Arnd Bergmann from comment #0)
> Let me work on this.
> I tested the warning with the kernel but don't recall coming across this
> false positive.  While I retry with the latest, how many of these do you see?

They are fairly rare, I have seen four or five now with randconfig builds over the last two days. It's possible that there is none in the default config.
Comment 4 Martin Sebor 2018-01-29 20:22:14 UTC

Just for reference in case more of these should pop up, I see two -Wrestrict instances in my build (below).  The first one looks correct but the second one could be an instance of the false positive reported here.  If you come across others please open new bugs (it would be helpful if I could reproduce your build so if you can share your config file and/or whatever else I might need that would be great).

net/mac80211/cfg.c:2794:3: warning: ‘memcpy’ source argument is the same as destination [-Wrestrict]
   memcpy(pos, beacon->probe_resp, beacon->probe_resp_len);

drivers/iommu/amd_iommu.c: In function ‘get_alias’:
drivers/iommu/amd_iommu.c:310:4: warning: ‘memcpy’ accessing 32 bytes at offsets 0 and 0 overlaps 32 bytes at offset 0 [-Wrestrict]

PS The breakdown of all warnings I see is below (the Unique column gives the number of distinct origins of the warning, and the Files column the number of distinct originating files):

Diagnostic                        Count   Unique    Files
-Wattributes                       2139        1        1
-Wattribute-alias                   465        3        3
-Wpacked-not-aligned                 45       17        3
-Wstringop-truncation                39       23       10
-Wformat-truncation=                 21       21       15
-Wint-in-bool-context                10       10        2
-Wformat-overflow=                    3        3        2
-Wstringop-overflow=                  2        2        2
-Wrestrict                            2        2        2
-Warray-bounds                        2        2        2
-Wsizeof-pointer-memaccess            1        1        1
-Wnonnull                             1        1        1
Comment 5 Arnd Bergmann 2018-01-29 21:06:25 UTC
Here are some additional instances in the kernel. I'm currently trying to get a reliable build first and haven't got a log of all the messages, but there are a number of changes I did that are related, shutting up the -Wrestrict warning (some may be -Warray-bounds).

Here are some false-positives:

--- a/arch/x86/oprofile/nmi_int.c
+++ b/arch/x86/oprofile/nmi_int.c
@@ -282,9 +282,7 @@ static void mux_clone(int cpu)
        if (!has_mux())
-       memcpy(per_cpu(cpu_msrs, cpu).multiplex,
-              per_cpu(cpu_msrs, 0).multiplex,
-              sizeof(struct op_msr) * model->num_virt_counters);
+       per_cpu(cpu_msrs, cpu).multiplex = per_cpu(cpu_msrs, 0).multiplex;
@@ -463,6 +461,7 @@ static int nmi_setup(void)
                if (!cpu)
+#ifdef CONFIG_SMP
                memcpy(per_cpu(cpu_msrs, cpu).counters,
                       per_cpu(cpu_msrs, 0).counters,
                       sizeof(struct op_msr) * model->num_counters);
@@ -470,7 +469,7 @@ static int nmi_setup(void)
                memcpy(per_cpu(cpu_msrs, cpu).controls,
                       per_cpu(cpu_msrs, 0).controls,
                       sizeof(struct op_msr) * model->num_controls);
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
@@ -1643,7 +1643,7 @@ static int cfg80211_rtw_scan(struct wiphy *wiphy
        if (request->n_channels == 1) {
                for (i = 1;i<survey_times_for_one_ch;i++)
-                       memcpy(&ch[i], &ch[0], sizeof(struct rtw_ieee80211_channel));
+                       ch[i] = ch[0];
                _status = rtw_sitesurvey_cmd(padapter, ssid, RTW_SSID_SCAN_AMOUNT, ch, survey_times_for_one_ch);
        } else if (request->n_channels <= 4) {
                for (j =request->n_channels-1;j>= 0;j--)

This one was weird, I suspect my change is incorrect:

diff --git a/drivers/fmc/fmc-fakedev.c b/drivers/fmc/fmc-fakedev.c
index 941d0930969a..0d322380d952 100644
--- a/drivers/fmc/fmc-fakedev.c
+++ b/drivers/fmc/fmc-fakedev.c
@@ -305,7 +305,7 @@ static int ff_init(void)
        /* Replicate the default eeprom for the max number of mezzanines */
        for (i = 1; i < FF_MAX_MEZZANINES; i++)
-               memcpy(ff_eeimg[i], ff_eeimg[0], sizeof(ff_eeimg[0]));
+               memcpy(&ff_eeimg[i][0], &ff_eeimg[0][0], sizeof(ff_eeimg[0]));
        if (ff_nr_eeprom > ff_nr_dev)
                ff_nr_dev = ff_nr_eeprom;

This one seems to be a kernel bug:

--- a/kernel/debug/kdb/kdb_support.c
+++ b/kernel/debug/kdb/kdb_support.c
@@ -129,13 +129,13 @@ int kdbnearsym(unsigned long addr, kdb_symtab_t *symtab)
                if (i >= ARRAY_SIZE(kdb_name_table)) {
-                       memcpy(kdb_name_table, kdb_name_table+1,
+                       memmove(kdb_name_table, kdb_name_table+1,
                               sizeof(kdb_name_table[0]) *
                } else {
                        knt1 = kdb_name_table[i];
-                       memcpy(kdb_name_table+i, kdb_name_table+i+1,
+                       memmove(kdb_name_table+i, kdb_name_table+i+1,
                               sizeof(kdb_name_table[0]) *
Comment 6 Arnd Bergmann 2018-01-30 10:14:32 UTC
I got one file that produces a rather cryptic warning related to this:

In file included from /git/arm-soc/arch/x86/include/asm/page_32.h:35,
                 from /git/arm-soc/arch/x86/include/asm/page.h:14,
                 from /git/arm-soc/arch/x86/include/asm/thread_info.h:12,
                 from /git/arm-soc/include/linux/thread_info.h:38,
                 from /git/arm-soc/arch/x86/include/asm/preempt.h:7,
                 from /git/arm-soc/include/linux/preempt.h:81,
                 from /git/arm-soc/include/linux/spinlock.h:51,
                 from /git/arm-soc/include/linux/seqlock.h:36,
                 from /git/arm-soc/include/linux/time.h:6,
                 from /git/arm-soc/include/linux/stat.h:22,
                 from /git/arm-soc/include/linux/module.h:10,
                 from /git/arm-soc/drivers/isdn/isdnloop/isdnloop.c:12:
In function 'strcpy',
    inlined from 'isdnloop_parse_cmd' at /git/arm-soc/drivers/isdn/isdnloop/isdnloop.c:900:3:
/git/arm-soc/include/linux/string.h:437:10: error: '__builtin_strcpy' accessing 0 or more bytes at offsets [36, 25] and 446 may overlap up to 0 bytes at offset [9223372036854775807, -9223372036854775808] [-Werror=restrict]
   return __builtin_strcpy(p, q);

Not sure if gcc should try to avoid that warning or print something more helpful in that case. The isdnloop code itself is cryptic enough that I'm not surprised to see gcc get confused as well, and using strncpy() or strncpy() instead of strcpy() would avoid the warning and improve the source code.

Possibly gcc should not warn about anything involving 'up to 0 bytes' though ;-)
Comment 7 Martin Sebor 2018-01-30 16:19:17 UTC
(In reply to Arnd Bergmann from comment #6)
> /git/arm-soc/include/linux/string.h:437:10: error: '__builtin_strcpy'
> accessing 0 or more bytes at offsets [36, 25] and 446 may overlap up to 0
> bytes at offset [9223372036854775807, -9223372036854775808]

This does suggest a bug in checker, similar to one that was fixed for memcpy not too long ago.  The offset ranges should be in increasing order (i.e., [25, 36], not the other way around).  Internally, they are represented as 128-bit numbers but when they're printed they are converted to 64-bit because GCC doesn't have support for printing anything bigger.  The wrong order implies that the first offset crosses the PTRDIFF_MAX boundary (i.e., it's probably a union of two offsets: [36, PTRDIFF_MAX] and [PTRDIFF_MIN, 25]).  It's possible that the strcpy checker has the same bug as the memcpy one.  They are different because the memcpy checker warns only for definite overlaps and the one for strcpy and other string functions even for possible overlaps.

I haven't yet been able to reproduce this in a small test case.  I'll keep working on but it would help expedite the fix if you could attach a translation unit that reproduces it.  Thanks for your help!
Comment 8 Arnd Bergmann 2018-01-30 17:58:32 UTC
Created attachment 43295 [details]
linux/drivers/isdn/isdnloop/isdnloop.c, preprocessed, compressed

This is the preprocessed file that showed the funky -Wrestrict warning message:

$ x86_64-linux-gcc-8.0.1 -Wall -O2 isdnloop.i -c -m32 -Wno-pointer-sign -Wno-unused
In file included from /git/arm-soc/arch/x86/include/asm/page_32.h:35,
                 from /git/arm-soc/arch/x86/include/asm/page.h:14,
                 from /git/arm-soc/arch/x86/include/asm/thread_info.h:12,
                 from /git/arm-soc/include/linux/thread_info.h:38,
                 from /git/arm-soc/arch/x86/include/asm/preempt.h:7,
                 from /git/arm-soc/include/linux/preempt.h:81,
                 from /git/arm-soc/include/linux/spinlock.h:51,
                 from /git/arm-soc/include/linux/seqlock.h:36,
                 from /git/arm-soc/include/linux/time.h:6,
                 from /git/arm-soc/include/linux/stat.h:22,
                 from /git/arm-soc/include/linux/module.h:10,
                 from /git/arm-soc/drivers/isdn/isdnloop/isdnloop.c:12:
In function 'strcpy',
    inlined from 'isdnloop_parse_cmd' at /git/arm-soc/drivers/isdn/isdnloop/isdnloop.c:900:3,
    inlined from 'isdnloop_writecmd' at /git/arm-soc/drivers/isdn/isdnloop/isdnloop.c:989:5:
/git/arm-soc/include/linux/string.h:437:10: warning: '__builtin_strcpy' accessing 0 or more bytes at offsets [36, 25] and 446 may overlap up to 0 bytes at offset [9223372036854775807, -9223372036854775808] [-Wrestrict]
   return __builtin_strcpy(p, q);

I did not try to reduce the test case.
Comment 9 Arnd Bergmann 2018-01-30 21:39:51 UTC
I found another false-positive -Wrestrict warning, did a manual reduction. Let me know if I should better open separate bugs for each test case, or you prefer to have them all here.

$ aarch64-linux-gcc-8.0.1 -Wall -O2 -c sit.i 
sit.i: In function 'sit_init_net':
sit.i:29:2: warning: 'memcpy' source argument is the same as destination [-Wrestrict]
  memcpy(&t->ip6rd, &t0->ip6rd, sizeof(t->ip6rd));

void *memcpy(void *, const void *, unsigned long );
struct ip_tunnel_6rd_parm {
        int relay_prefix;
        int prefixlen;
        int relay_prefixlen;
struct netdevice {
        void *priv;
struct ip_tunnel {
        struct netdevice *dev;
        struct ip_tunnel_6rd_parm ip6rd;
struct sit_net {
        struct netdevice *fb_tunnel_dev;
void ipip6_tunnel_clone_6rd(struct netdevice *dev, struct sit_net *sitn)
        struct ip_tunnel *t = dev->priv;
        if (t->dev == sitn->fb_tunnel_dev)
        struct ip_tunnel *t0 = sitn->fb_tunnel_dev->priv;
        memcpy(&t->ip6rd, &t0->ip6rd, sizeof(t->ip6rd));
int sit_init_net(struct sit_net *sitn, struct netdevice *fb_tunnel_dev) 
        sitn->fb_tunnel_dev = fb_tunnel_dev;
        ipip6_tunnel_clone_6rd(sitn->fb_tunnel_dev, sitn);
        return 0;
Comment 10 Martin Sebor 2018-01-30 22:37:30 UTC
Created attachment 43299 [details]
Patch under test.

I think that false positive is already being tracked in bug 83456.  Let me add the test case to it.

I'm testing the attached patch (it's an aggregate of the one posted here and the one posted for bug 83698 which is still waiting to be approved).

Until I've verified the patch please add new test cases here and I'll triage them.
Comment 11 Martin Sebor 2018-01-30 23:21:49 UTC
Patch for the bug in comment #0: https://gcc.gnu.org/ml/gcc-patches/2018-01/msg02396.html
Comment 12 Martin Sebor 2018-01-31 02:35:20 UTC
(In reply to Arnd Bergmann from comment #8)

I have manually reproduced a similar false positive in the small test case below it's not quite the same issue but I have a fix for both.  They're both caused by similar oversights as the one in comment #0, except with slightly different root causes.  In the translation unit from comment #8 the checker cannot reliably distinguish between offsets into different members of the same struct and offsets into the same member.  In the case below the checker doesn't look deep enough through the array of arrays to see that p->a[i] need not be the same as p->a[0].

I expect to be done testing a fix for both tomorrow.  If you have other test case/translation units please do add them here.  This has been very helpful -- thanks again!

$ cat z.c && gcc -O2 -S -Wall z.c
struct S {
  char a[2][4];

void f (struct S *p, int i)
  __builtin_strcpy (p->a[0], "012");
  __builtin_strcpy (p->a[i] + 1, p->a[0]);
z.c: In function ‘f’:
z.c:8:3: warning: ‘__builtin_strcpy’ accessing 4 bytes at offsets 1 and 0 overlaps 3 bytes at offset 1 [-Wrestrict]
   __builtin_strcpy (p->a[i] + 1, p->a[0]);
Comment 13 Martin Sebor 2018-02-01 23:49:35 UTC
Updated patch with fixes for the false positives discussed in comment #12:
Comment 14 Arnd Bergmann 2018-02-06 13:24:03 UTC
I applied the patches and seem to still get a warning for this:

$ x86_64-linux-gcc-8.0.1 -Wall -O2 -c nmi_int.c
nmi_int.c: In function 'nmi_setup':
nmi_int.c:43:3: warning: 'memcpy' source argument is the same as destination [-Wrestrict]
   memcpy(per_cpu(cpu_msrs, cpu).counters,
          per_cpu(cpu_msrs, 0).counters,
          sizeof(int) * model->num_counters);

typedef unsigned long __kernel_size_t;
extern void * memcpy(void *,const void *,__kernel_size_t);
struct op_msrs {
        int *counters;
#define for_each_possible_cpu(cpu) for_each_cpu((cpu), cpu_possible_mask)
#define for_each_cpu(cpu, mask)  for ((cpu) = 0; (cpu) < 1; (cpu)++,(void)mask)
extern struct cpumask __cpu_possible_mask;
#define cpu_possible_mask ((const struct cpumask *)&__cpu_possible_mask)
#define DEFINE_PER_CPU(type, name) __typeof__(type) name
#define per_cpu_ptr(ptr, cpu)   ({ (void)(cpu); ptr; })
#define raw_cpu_ptr(ptr)        per_cpu_ptr(ptr, 0)
#define per_cpu(var, cpu)       (*per_cpu_ptr(&(var), cpu))
extern void *pcpu_base_addr;
extern const unsigned long *pcpu_unit_offsets;
struct op_x86_model_spec {
        unsigned int    num_counters;
static struct op_x86_model_spec *model;
static DEFINE_PER_CPU(struct op_msrs, cpu_msrs);
int nmi_setup(void)
        int err = 0;
        int cpu;
        for_each_possible_cpu(cpu) {
                if (!cpu)
                memcpy(per_cpu(cpu_msrs, cpu).counters,
                       per_cpu(cpu_msrs, 0).counters,
                       sizeof(int) * model->num_counters);
        return err;

In this code, we do copy from a variable onto itself, but only in a dead branch, here because the for_each_possible_cpu() and per_cpu() macros degrade to trivial wrappers on an non-SMP build.
Comment 15 Arnd Bergmann 2018-02-06 13:44:48 UTC
(In reply to Arnd Bergmann from comment #14)
> I applied the patches and seem to still get a warning for this

I also just got the one from comment #9 again and found that the reduced test case is still affected (and not claimed to be fixed by any of the patches, so that's my fault for not checking).
Comment 16 Martin Sebor 2018-02-06 17:55:26 UTC
Thanks for the test case in comment #14.  I've reproduced the warning and will look into it.

The false positive from comment #9 isn't fixed yet.  I'm assuming it's due to the same root cause as bug 83456 that I'm tracking separately.  There's no way to avoid that one other than by disabling the warning in a whole set of cases.  I'm trying to decide if trading the false positives for the unavoidable false negatives make sense.
Comment 17 Martin Sebor 2018-02-06 18:35:26 UTC
I've reduced the test case from comment #14 to the one below.  I'm inclined to think the warning is justified.  As you say, the code clearly does use the same pointer for both the source and the destination, so the code is strictly not valid.  GCC ultimately eliminates the memcpy call and the whole loop, but not before it notices the argument is the same.  The warning is issued in the same places as the one for bug 83456 so if a decision is made to fix that bug this warning will also disappear.

$ cat t.c && gcc -O2 -S -Wall t.c
extern void* memcpy (void*, const void*, __SIZE_TYPE__);

char a[4];

void f (unsigned n)
  for (int i = 0; i < 1; i++)
      if (!i)

      memcpy (a, a, n);
t.c: In function ‘f’:
t.c:12:7: warning: ‘memcpy’ source argument is the same as destination [-Wrestrict]
       memcpy (a, a, n);
Comment 18 Martin Sebor 2018-02-20 20:22:33 UTC
Author: msebor
Date: Tue Feb 20 20:22:01 2018
New Revision: 257860

URL: https://gcc.gnu.org/viewcvs?rev=257860&root=gcc&view=rev
PR middle-end/84095 - false-positive -Wrestrict warnings for memcpy within array


	PR middle-end/84095
	* gimple-ssa-warn-restrict.c (builtin_memref::extend_offset_range): New.
	(builtin_memref::set_base_and_offset): Same.  Handle inner references.
	(builtin_memref::builtin_memref): Factor out parts into
	set_base_and_offset and call it.


	PR middle-end/84095
	* c-c++-common/Warray-bounds-3.c: Adjust text of expected warnings.
	* c-c++-common/Wrestrict.c: Same.
	* gcc.dg/Wrestrict-6.c: Same.
	* gcc.dg/Warray-bounds-27.c: New test.
	* gcc.dg/Wrestrict-8.c: New test.
	* gcc.dg/Wrestrict-9.c: New test.
	* gcc.dg/pr84095.c: New test.

Comment 19 Martin Sebor 2018-02-20 20:23:44 UTC
Fixed in r257860.  (The remaining false positives are being tracked in bug 83456.)