Bug 79929 - [7 Regression] Bogus Warning: '__builtin_memset': specified size 4294967291 exceeds maximum object size 2147483647
Summary: [7 Regression] Bogus Warning: '__builtin_memset': specified size 4294967291 e...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 7.0.1
: P4 normal
Target Milestone: 7.4
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks:
 
Reported: 2017-03-06 20:46 UTC by Harald Anlauf
Modified: 2018-05-04 12:40 UTC (History)
8 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2017-03-07 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Harald Anlauf 2017-03-06 20:46:54 UTC
Hi all,

for trunk rev.245718, GNU Fortran (GCC) 7.0.1 20170224 (experimental),
the code:

% cat gfcbug138.f90
subroutine gfcbug138 (yerrmsg)
  character(*) :: yerrmsg
  yerrmsg = ""
  yerrmsg = "bug: " // yerrmsg
end subroutine gfcbug138

produces

% gfc-trunk -Wall -c gfcbug138.f90 -O1
gfcbug138.f90:4:0:

   yerrmsg = "bug: " // yerrmsg
 
Warning: '__builtin_memset': specified size 4294967291 exceeds maximum object size 2147483647 [-Wstringop-overflow=]

at -O1, -O2, -O3, but not at -O0.
Comment 1 Harald Anlauf 2017-03-06 21:49:53 UTC
Possibly related to PR78758, which is already fixed.
Comment 2 Jeffrey A. Law 2017-03-07 16:06:52 UTC
Note, this triggers at O2 and above, but not at O1.

This really looks like bogus code coming out of the Fortran front-end.  I've got no clue why it's generating this code:

[ ... ]
  _32 = _yerrmsg_19(D) + 5;
  _33 = _yerrmsg_19(D);
[ ... ]

;;   basic block 6, loop depth 0, count 0, freq 0, maybe hot
;;    prev block 5, next block 7, flags: (NEW, REACHABLE)
;;    pred:       5 (TRUE_VALUE)
  _7 = (unsigned long) _33;
  _8 = (unsigned long) _32;
  _9 = MIN_EXPR <_7, _8>;
  __builtin_memmove (yerrmsg_25(D), pstr.0_29, _9);
  _10 = (unsigned long) _32;
  _11 = (unsigned long) _33;
  if (_10 < _11)
    goto <bb 7>; [0.00%]
  else
    goto <bb 8>; [0.00%]
;;    succ:       7 (TRUE_VALUE)
;;                8 (FALSE_VALUE)

;;   basic block 7, loop depth 0, count 0, freq 0, maybe hot
;;    prev block 6, next block 8, flags: (NEW, REACHABLE)
;;    pred:       6 (TRUE_VALUE)
  _12 = (unsigned long) _33;
  _13 = (unsigned long) _32;
  _14 = _12 - _13;
  _15 = (sizetype) _32;
  _16 = yerrmsg_25(D) + _15;
  __builtin_memset (_16, 32, _14);
;;    succ:       8 (FALLTHRU)

So the test at the end of bb6 is really an overflow test.  (x + 5 < x).  

The assignment to _14 is going to overflow.   We end up in this match.pd pattern:

 /* (T)P - (T)(P + A) -> -(T) A */
  (for add (plus pointer_plus)
   (simplify
    (minus (convert @0)
     (convert (add @@0 @1)))
    (if (element_precision (type) <= element_precision (TREE_TYPE (@1))
         /* For integer types, if A has a smaller type
            than T the result depends on the possible
            overflow in P + A.
            E.g. T=size_t, A=(unsigned)429497295, P>0.
            However, if an overflow in P + A would cause
            undefined behavior, we can assume that there
            is no overflow.  */
         || (INTEGRAL_TYPE_P (TREE_TYPE (@0))
             && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0)))
         /* For pointer types, if the conversion of A to the
            final type requires a sign- or zero-extension,
            then we have to punt - it is not defined which
            one is correct.  */
         || (POINTER_TYPE_P (TREE_TYPE (@0))
             && TREE_CODE (@1) == INTEGER_CST
             && tree_int_cst_sign_bit (@1) == 0))
     (negate (convert @1)))))

Which results in:

  __builtin_memset (_16, 32, 18446744073709551611);

Which we quite reasonably warn for.

AFAICT this looks like bogus code coming out of the Fortran front-end to me.  I don't see any way to mitigate in the optimization pipeline.
Comment 3 Harald Anlauf 2017-03-30 19:33:25 UTC
I've slightly reduced the example to the following:

% cat gfcbug138c.f90
subroutine gfcbug138 (yerrmsg)
  character(kind=1,len=*) :: yerrmsg
  yerrmsg = 1_"bug: " // yerrmsg
end subroutine gfcbug138


The dump tree now reads:


gfcbug138 (character(kind=1)[1:_yerrmsg] & restrict yerrmsg, integer(kind=4) _yerrmsg)
{
  bitsizetype D.3423;
  sizetype D.3424;

  D.3423 = (bitsizetype) (sizetype) NON_LVALUE_EXPR <_yerrmsg> * 8;
  D.3424 = (sizetype) NON_LVALUE_EXPR <_yerrmsg>;
  {
    character(kind=1)[1:] * pstr.0;
    void * restrict D.3419;
    integer(kind=4) D.3420;
    integer(kind=4) D.3421;
    integer(kind=4) D.3422;

    D.3419 = (void * restrict) __builtin_malloc (MAX_EXPR <(character(kind=4)) (_yerrmsg + 5), 1>);
    pstr.0 = (character(kind=1)[1:] *) D.3419;
    _gfortran_concat_string (_yerrmsg + 5, pstr.0, 5, &"bug: "[1]{lb: 1 sz: 1}, _yerrmsg, yerrmsg);
    D.3420 = _yerrmsg + 5;
    D.3421 = _yerrmsg + 5;
    D.3422 = _yerrmsg;
    if (D.3422 != 0)
      {
        __builtin_memmove ((void *) yerrmsg, (void *) pstr.0, MIN_EXPR <(character(kind=4)) D.3422, (character(kind=4)) D.3421>);
        if ((character(kind=4)) D.3421 < (character(kind=4)) D.3422)
          {
            __builtin_memset ((void *) yerrmsg + (sizetype) D.3421, 32, (character(kind=4)) D.3422 - (character(kind=4)) D.3421);
          }
      }
    __builtin_free ((void *) pstr.0);
  }
}


- Why are there still occurrences of character(kind=4)?  Shouldn't there be
  always character(kind=1)?

- The optimizer gets confused by a potential overflow of the length of the
concatenated string (_yerrmsg + 5), which is the only way the memset can be
invoked, with a length of -5, which is less than 0 (assuming ordinary wrapping
integer arithmetic)
Comment 4 Thomas Koenig 2017-04-12 17:00:43 UTC
Here's equivlalent C code:

$ cat bug.c
#include <stdlib.h>
#include <string.h>

char * foo(char *c, int len)
{
  char *p, *n;
  n = malloc(len + 5);
  p = c + 5;
  memmove (c, n, p-c);
  if (p < c)
    memset (n + 5, 32, c-p);
  return n;
}
$ gcc -Wall -O -c bug.c
bug.c: In function 'foo':
bug.c:11:5: warning: 'memset': specified size 18446744073709551611 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
     memset (n + 5, 32, c-p);

I don't think the code is buggy, but the warning is bogus.
Comment 5 Martin Sebor 2017-04-12 17:46:09 UTC
Thanks for the C test case.  In it, the warning is a false positive caused by GCC's failure to eliminate the excessive memset at -O1.  The call is emitted by the ccp1 pass at all optimization levels.  At -O2, it's also removed, looks like by the forward propagation pass that runs just after ccp1, but at -O1 it's never removed.
Comment 6 Dominique d'Humieres 2017-04-12 17:47:29 UTC
Likely caused by revision r243419. Note that I don't get the warning with the C code in comment 4.
Comment 7 Thomas Koenig 2017-04-12 20:30:58 UTC
Setting component to middle end, resetting priority.
Comment 8 Jakub Jelinek 2017-04-13 14:31:40 UTC
Well, with -O1 one asks only for cheap optimizations, so the question is if a warning that relies on more optimizations as this should be enabled at -O1.

We have:
  p_18 = c_17(D) + 5;
...
  if (p_18 < c_17(D))
    goto <bb 3>; [0.00%]
  else
    goto <bb 4>; [0.00%]

  <bb 3> [0.00%]:
  c.2_7 = (long int) c_17(D);
  p.3_8 = (long int) p_18;
  _9 = c.2_7 - p.3_8;
  _10 = (long unsigned int) _9;
  _11 = n_16 + 5;
  __builtin_memset (_11, 32, _10);

and cpp1 pass manages to fold the _10 into the huge value, but doesn't similarly fold the pointer comparison.  So either we should not do that at -O1, or should also be able to fold (A + cst) < A for pointer type A into cst < 0 (only if POINTER_TYPE_OVERFLOW_UNDEFINED ?).
Comment 9 Martin Sebor 2017-04-13 17:12:36 UTC
I actually think that for the test case in comment #4, the warning does highlight a problem in the code: since the test (p < c) can never be true the memset can never be called in a valid program.  At -O2 the problem is detected by -Wstrict-overflow.  Unfortunately, because -Wstrict-overflow is only issued when an optimization takes place that depends on the absence of an overflow, it can't be issued at -O1 when the condition isn't removed.  I still think that eliminating the memset call (or replacing it with a trap) when the size is known to be excessive is a viable strategy.  (For memset perhaps under an option to allow for the rare programs that make use of Physical Address Extension and clear memory regions larger than 2GB.)
Comment 10 Jeffrey A. Law 2017-04-14 20:44:58 UTC
At -O2 the memset is removed very early in the optimization pipeline.   Thus there's no warning at O2, but there is a warning at O1.  Reality is some warnings are going to give false positives when optimizations are throttled back and I wouldn't call that a regression for the C testcase.


What I am concerned about is in the original Fortran based test, we warned at -O2, but not at -O1.  Something has been lost in the translation to C.
Comment 11 Dominique d'Humieres 2017-04-15 14:32:52 UTC
On x86_64-apple-darwin16 I see the warning

Warning: '__builtin_memset': specified size 18446744073709551611 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]

for -Os and -O2 and above. I don't understand why this case is not detected with the -Wcharacter-truncation flag: the length of "bug: " // yerrmsg is the len(yerrmsg)+5, thus will be truncated when assigned to yerrmsg.

Apparently the problem comes from the fact that len(yerrmsg) is not known at compile time, but I don't understand why the warning use nonsensical numbers.
Comment 12 Markus Trippelsdorf 2017-04-15 14:46:04 UTC
(In reply to Dominique d'Humieres from comment #11)
> On x86_64-apple-darwin16 I see the warning
> 
> Warning: '__builtin_memset': specified size 18446744073709551611 exceeds
> maximum object size 9223372036854775807 [-Wstringop-overflow=]
> 
> for -Os and -O2 and above. I don't understand why this case is not detected
> with the -Wcharacter-truncation flag: the length of "bug: " // yerrmsg is
> the len(yerrmsg)+5, thus will be truncated when assigned to yerrmsg.
> 
> Apparently the problem comes from the fact that len(yerrmsg) is not known at
> compile time, but I don't understand why the warning use nonsensical numbers.

I agree that these huge decimal numbers look ridiculous. 
At the very least they should be printed as hex numbers.
Comment 13 Martin Sebor 2017-04-15 16:42:29 UTC
> Apparently the problem comes from the fact that len(yerrmsg) is not known at
> compile time, but I don't understand why the warning use nonsensical numbers.

The numbers that are printed are the values or their ranges that GCC computes for the arguments and what GCC uses as the maximum: 9223372036854775807 (or PTRDIFF_MAX) in LP64.  Memset takes a size_t argument so negative values passed to it are printed as very large positive numbers.  In both instances of the warning for the Fortran test case, 4294967291 (in ILP32) and 18446744073709551611 (LP64) is the same as -5.

On x86_64 where the warning triggers with -O2, the excessive memset is introduced during constant propagation (-fdump-tree-ccp1=/dev/stdout).  The .ccp1 dump shows this:

  _32 = _yerrmsg_19(D) + 5;
  ...
  _10 = (unsigned long) _32;
  _11 = (unsigned long) _yerrmsg_19(D);
  if (_10 < _11)                   <<<  can never be true
    goto <bb 7>; [0.00%]
  else
    goto <bb 8>; [0.00%]

  <bb 7> [0.00%]:
  _12 = (unsigned long) _yerrmsg_19(D);
  _13 = (unsigned long) _32;
  _15 = (sizetype) _32;
  _16 = yerrmsg_25(D) + _15;
  __builtin_memset (_16, 32, 18446744073709551611);

The result of yerrmsg + 5 (_10) can never be less than the yerrmsg pointer itself (_11) so the test (_10  < _11) above can never succeed.  The test shows up in the first dump so, as Jeff already noted in comment #2,  it's introduced by the Fortran front end.  I don't enough about Fortran to guess why the test is introduced but it should be clear that the problem is not that the warning is behaving incorrectly or printing nonsensical numbers but rather either that a test that cannot be true is inserted to begin with, or that it's not eliminated.

That said, it's possible that the warning could be made clearer if it used some other notation instead of printing very large positive decimal numbers.  I'm not sure that printing hex numbers instead is the right solution though.  The following doesn't seem much clearer to me:

Warning: ‘__builtin_memset’: specified size 0xfffffffffffffffb exceeds maximum object size 7fffffffffffffff [-Wstringop-overflow=]

In any event, the particular notation used by the warning is orthogonal to this problem.
Comment 14 Markus Trippelsdorf 2017-04-15 16:55:09 UTC
(In reply to Martin Sebor from comment #13)
> That said, it's possible that the warning could be made clearer if it used
> some other notation instead of printing very large positive decimal numbers.
> I'm not sure that printing hex numbers instead is the right solution though.
> The following doesn't seem much clearer to me:
> 
> Warning: ‘__builtin_memset’: specified size 0xfffffffffffffffb exceeds
> maximum object size 7fffffffffffffff [-Wstringop-overflow=]

It is not my intention to derail the discussion, but hex numbers are much clearer.
One immediately sees that 0x7FFFFFFFFFFFFFFF is ULLONG_MAX/2.
Comment 15 Martin Sebor 2017-04-15 19:47:11 UTC
I've raised bug 80437 for the suggested change to the notation of the warning(s).
Comment 16 Jeffrey A. Law 2017-04-19 16:25:36 UTC
So WRT c#4, that is not an equivalent testcase in C AFAICT.

The reduced fortran testcase fails at -O2, the C testcase does not.  The fact that the C testcase fails at -O1 is simply uninteresting.

If we go back to the Fortran testcases I don't see anything that would allow us to determine statically that the problematical block can not be reached.

Working with the reduced Fortran testcase we have this in the dumps:

  sizetype _1;
  integer(kind=4) _2;
  unsigned long _3;


  _1 = (sizetype) _yerrmsg_9(D);
[ ... ]
  _2 = _yerrmsg_9(D) + 5;
  _3 = (unsigned long) _2;
[ ... ]
  if (_1 > _3)
    goto <bb 6>; [36.64%]
  else
    goto <bb 7>; [63.36%]

We're dealing with unsigned types here, so overflow has well defined semantics.  Thus we can not statically determine the result of that conditional.

This really is a Fortran problem AFAICT.
Comment 17 janus 2017-04-27 11:33:05 UTC
Sidenote: Apparently disabling such warnings for Fortran code is not possible.

$ gfortran-8 c3.f90 -c -O2 -Wno-stringop-overflow

gives me:

f951: Warning: command line option ‘-Wstringop-overflow=0’ is valid for C/C++/ObjC/ObjC++ but not for Fortran


This is problematic because obviously one can get such warnings on Fortran code (even without -Wall because -Wstringop-overflow=2 seems to be the default).
Comment 18 Martin Sebor 2017-04-27 14:18:23 UTC
(In reply to janus from comment #17)

I've been meaning to open a bug for this.  It's now PR80545.
Comment 19 Jakub Jelinek 2017-05-02 15:58:51 UTC
GCC 7.1 has been released.
Comment 20 Alexander Ivchenko 2017-05-09 10:02:15 UTC
Not sure whether it is connected, but when I bootstrap with:
>../gcc_ref/configure  --with-system-zlib --with-demangler-in-ld --with-arch=corei7 --with-cpu=corei7 --with-fpmath=sse                     --enable-shared --enable-host-shared --enable-clocale=gnu --enable-cloog-backend=isl --enable-languages=c --enable-libmpx=yes --with-build-config=bootstrap-lto
>make

In function ‘rtvec_alloc’,
    inlined from ‘copy_rtx_for_iterators’ at ../../gcc_ref/gcc/read-rtl.c:448:32:
../../gcc_ref/gcc/rtl.c:155:10: error: ‘memset’: specified size 18446744073709551608 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
   memset (&rt->elem[0], 0, n * sizeof (rtx));
Comment 21 Martin Sebor 2017-05-09 14:35:02 UTC
The rtl.c error was discussed in the context of some other bug having to do with profiledbootstrap failure (I can't find the bug now).  If I recall, it's due to the same signed <-> unsigned conversion issue as a number of other warnings of this kind, i.e., rtvec_alloc taking a signed int argument that's being converted to size_t.  Besides configuring with the --disable-werror recommended for profiledbootstrap, adding a gcc_assert(n >= 0) fixed it.  Strangely, though, changing the function's argument to unsigned seemed to tickle some latent bug somewhere and caused GCC to crash during bootstrap.  I never investigated why.
Comment 22 Neil Carlson 2017-11-24 21:14:30 UTC
I'm seeing these warning messages with essentially the same test case as comment 0 using 8.0 (20171123) at -O1 and higher.

Same warning messages using 7.2.1 but only at -O2 and higher.

Even if these are bogus (I hope), they really need to be gotten rid of.
Comment 23 Marc Glisse 2017-11-24 22:40:14 UTC
(In reply to Harald Anlauf from comment #3)
> subroutine gfcbug138 (yerrmsg)
>   character(kind=1,len=*) :: yerrmsg
>   yerrmsg = 1_"bug: " // yerrmsg
> end subroutine gfcbug138
[...]
> gfcbug138 (character(kind=1)[1:_yerrmsg] & restrict yerrmsg, integer(kind=4)
> _yerrmsg)

I don't know anything about fortran and this isn't the best way to fix this bug, but I was wondering if, for this kind of ABI (passing an array with a pointer and a length IIUC), fortran guarantees that the length is non-negative? If that's the case, could the front-end convey that information to the middle-end using set_range_info or some other mechanism?
Comment 24 Dominique d'Humieres 2017-11-24 23:32:13 UTC
The following variant does not give the warning

subroutine gfcbug138 (yerrmsg)
  character(kind=1,len=*) :: yerrmsg
  character(kind=1,len=len(yerrmsg)+5) :: tmp
  tmp = 1_"bug: " // yerrmsg
  yerrmsg = tmp
end subroutine gfcbug138
Comment 25 Marc Glisse 2017-11-25 00:02:01 UTC
(In reply to Dominique d'Humieres from comment #24)
> The following variant does not give the warning

That's because the code has become obfuscated enough that we don't have the simplification l-(l+5) anymore (we see l-max(0,l+5) instead) and the argument to the dead memset call is not seen as a constant. But the dead code is still there, even if we don't warn.
Comment 26 Dominique d'Humieres 2017-11-25 00:09:21 UTC
Another variant without warning:

subroutine gfcbug138 (yerrmsg)
  character(kind=1,len=*) :: yerrmsg
  yerrmsg = 1_"bug: " // yerrmsg(1:len(yerrmsg)-5)
end subroutine gfcbug138
Comment 27 Richard Biener 2018-01-25 08:25:15 UTC
GCC 7.3 is being released, adjusting target milestone.
Comment 28 janus 2018-03-15 10:27:27 UTC
With a current trunk (gcc version 8.0.1 20180315 / trunk revision 258550), I don't see the warnings on comment 0 and comment 3 any more.

I do see them with gcc version 7.2.0 (Ubuntu 7.2.0-1ubuntu1~16.04), but I haven't checked the latest gcc-7-branch.
Comment 29 Thomas Koenig 2018-03-18 09:21:15 UTC
Author: tkoenig
Date: Sun Mar 18 09:20:37 2018
New Revision: 258630

URL: https://gcc.gnu.org/viewcvs?rev=258630&root=gcc&view=rev
Log:
2018-03-18  Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR fortran/79929
	* gfortran.dg/warn_concat.f90: New test.


Added:
    trunk/gcc/testsuite/gfortran.dg/warn_concat.f90
Modified:
    trunk/gcc/testsuite/ChangeLog
Comment 30 Dominique d'Humieres 2018-03-18 11:13:11 UTC
Fixed by revision r256284?
Comment 31 Thomas Koenig 2018-03-18 15:55:03 UTC
(In reply to Dominique d'Humieres from comment #30)
> Fixed by revision r256284?

r256284 is OK, r256283 fails.

We cannot backport r256284 because this is the ABI change for
long string lengths.

So, this is a candidate for "RESOLVED FIXED" unless somebody
comes up with an independent way to fix this (but frankly,
I wouldn't try to do it).
Comment 32 Thomas Koenig 2018-03-18 17:22:30 UTC
So, fixed in gcc-8, backport too invasive / not needed for gcc-7.
Comment 33 andysem 2018-04-28 23:22:31 UTC
The r256284 seems to fix the warning for Fortran and I'm seeing the warning in C/C++ on gcc 7.2. Is the C/C++ compiler also fixed?