Bug 91397 - -Wstringop-overflow specified bound 18446744073709551615 exceeds maximum object size 9223372036854775807
Summary: -Wstringop-overflow specified bound 18446744073709551615 exceeds maximum obje...
Status: RESOLVED WORKSFORME
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 9.1.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks: Wstringop-overflow
  Show dependency treegraph
 
Reported: 2019-08-08 10:36 UTC by Steinar H. Gunderson
Modified: 2022-05-07 05:57 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2020-01-28 00:00:00


Attachments
Unreduced test case (426.29 KB, application/x-xz)
2019-08-08 10:37 UTC, Steinar H. Gunderson
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Steinar H. Gunderson 2019-08-08 10:36:06 UTC
Hi,

We saw this coming only with GCC 10 (probably different inlining decisions), but the reduced test case also triggers with 9, so I'm going to file it against 9.1.0.

This test case is reduced from MySQL 8.0 (osfile0.cc):

#include <string.h>
#include <stdlib.h>

char *allocate(unsigned long g) {
        if (g == 0) return (char *)malloc(0);
        for (;;)        
                ;
} 

char *i;
char *j(long k) {
        char *l(allocate(k + 1));
        memcpy(l, i, k);
        return l;
} 

atum17:~> g++-9 -O2 -Wstringop-overflow -c os0file.cc
os0file.cc: In function ‘char* j(long int)’:
os0file.cc:12:9: warning: ‘void* memcpy(void*, const void*, size_t)’ specified bound 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
   12 |   memcpy(l, i, k);
      |   ~~~~~~^~~~~~~~~

I don't even understand what the warning means (“specified bound” of what?), but it looks spurious to me. Perhaps some signed/unsigned confusion?

Since the reduction is somewhat nonsensical with the empty infinite loop in there, I'm also including an unreduced test case. With GCC 9.1.0 and -O2, it gives the same warning.
Comment 1 Steinar H. Gunderson 2019-08-08 10:37:14 UTC
Created attachment 46689 [details]
Unreduced test case
Comment 2 Marc Glisse 2019-08-08 11:30:41 UTC
        if (g == 0) return (char *)malloc(0);
        for (;;)        
                ;

so the only way this can return is if g is 0. This means that in j, k is -1, and you are calling memcpy with a huge argument. So at least in the reduced testcase, the warning makes some sense.
Comment 3 Steinar H. Gunderson 2019-08-08 11:35:29 UTC
Yes, the reduced one is awkward. Thus the unreduced one :-)
Comment 4 Marc Glisse 2019-08-08 12:10:43 UTC
I guess it happens in some dead path that gcc doesn't know is dead. At some point, you look at last_slash-path+1. Here there is a branch on whether this number is 0, and if it is 0, nonsense happens (writing 0 at address -1, this huge memcpy, etc). Maybe you know that last_slash is always >= path (so this weird code path is dead) and gcc doesn't?
Comment 5 Marc Glisse 2019-08-08 12:25:42 UTC
mem_strdupl calls allocate(len+1). If len+1 is 0, you proceed to write to s[len] i.e. 0[-1]. I think gcc would be happier if you handled this special case explicitly (you could error, trap, just assume it cannot happen (__builtin_unreachable), whatever).

This type of warning can easily give false positives if your code is written with invariants in mind that are not visible enough to the compiler.

If you had been writing to address 0, gcc would have detected that as a trap, but it doesn't do anything special for -1.
Comment 6 Steinar H. Gunderson 2019-08-08 12:42:03 UTC
So basically GCC is worried that I might be calling allocate() with -1 bytes, and gives a warning?

last_slash presumably has to be >= path, given that it comes out of strrchr(). But maybe GCC won't know that.
Comment 7 Marc Glisse 2019-08-08 13:44:20 UTC
(In reply to Steinar H. Gunderson from comment #6)
> So basically GCC is worried that I might be calling allocate() with -1
> bytes, and gives a warning?

Yes, although it might not always give the warning, depends on various heuristics.

> last_slash presumably has to be >= path, given that it comes out of
> strrchr().

It doesn't directly, there is a lot of last_slash-- with various conditions.
Comment 8 Steinar H. Gunderson 2019-08-08 13:58:25 UTC
But all of those conditions include last_slash > path.

I tried adding this just before the mem_strdupl() call:

  if (last_slash < path) {
    ib::fatal() << "Logic error.";
    __builtin_unreachable();
  }

and the warning still triggers.
Comment 9 Steinar H. Gunderson 2019-08-08 14:02:53 UTC
Putting this at the start of mem_strdupl() suppresses the warning:

  if (len + 1 == 0) __builtin_unreachable();

This seemingly also does:

  if (static_cast<long>(len) < 0) __builtin_unreachable();

So somehow, even though it knows that path >= last_slash (from before), it doesn't know that last_slash - path >= 0. I don't know how easy or hard this is to infer.
Comment 10 Martin Sebor 2019-08-09 01:16:41 UTC
GCC assumes that no object can be bigger than PTRDIFF_MAX - 1 bytes and warns when it detects code that expects otherwise.  A more general condition to add to mem_strdupl() to avoid the warning is:

  if (len >= PTRDIFF_MAX)
    __builtin_unreachable ();

GCC doesn't track pointer ranges as well as it does integers and it easily "loses" information about their relationships.  For instance, in the functions below, GCC folds the first test to false but it doesn't fold the second.

  void f (long a, long b)
  { 
    if (a < b)
      return;

    if (a - b < 0)   // folded to false
      __builtin_abort ();
  }

  void g (int *a, int *b)
  {
    if (a < b)
      return;

    if (a - b < 0)   // not folded
      __builtin_abort ();
  }
Comment 11 Martin Sebor 2020-04-22 21:00:00 UTC
The reduced test case in comment #0 still produces the same warning with GCC 10  but the attached test case no longer does.  I couldn't pinpoint the change that eliminated the warning between 9 and 10 because of all the C++ errors I get for the libstdc++ features that aren't supported by older versions of G++.

Since the small test case isn't representative of the larger one I'm going to resolve this as WORKSFORSOME.  If the problem persists please open a new bug with a new test case.