Bug 82103 - spurious stringop-overflow warning
Summary: spurious stringop-overflow warning
Status: RESOLVED INVALID
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 8.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks:
 
Reported: 2017-09-04 23:07 UTC by Arnd Bergmann
Modified: 2017-12-05 19:11 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2017-09-29 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Arnd Bergmann 2017-09-04 23:07:14 UTC
On a current snapshot (tested with r251683), I get a warning that seems unhelpful and probably should not be there.

This happened in exactly one file from the Linux kernel so far, on any 32-bit target:

$ arm-linux-gnueabi-gcc-8.0.0 -Werror -O2 -march=armv7-a -c -Wall hns_ethtool-b.c

hns_ethtool-b.c: In function 'g':
hns_ethtool-b.c:9:7: error: 'memset' specified size 4294967295 exceeds maximum object size 2147483647 [-Werror=stringop-overflow=]
       memset(__p, v, __n);           \
       ^~~~~~~~~~~~~~~~~~~
hns_ethtool-b.c:18:2: note: in expansion of macro 'memset'
  memset(&data[frame_size / 2], 0, frame_size / 2 - 1);
  ^~~~~~

---
extern void f(void);
extern void *memset();

#define memset(p, v, n)              \
  ({                                 \
    void *__p = p;                   \
    unsigned int __n = n;            \
    if (__n != 0)                    \
      memset(__p, v, __n);           \
    (__p);			     \
  })

void g(char *data, unsigned int frame_size, _Bool c)
{
	memset(data, 5, frame_size);
	if (c)
		f();
	memset(&data[frame_size / 2], 0, frame_size / 2 - 1);
	memset(&data[frame_size / 2 + 0], 4, frame_size / 2 - 11);
}
Comment 1 Eric Gallager 2017-09-29 02:30:25 UTC
(In reply to Arnd Bergmann from comment #0)
> On a current snapshot (tested with r251683), I get a warning that seems
> unhelpful and probably should not be there.
> 
> This happened in exactly one file from the Linux kernel so far, on any
> 32-bit target

Confirmed that I also get the warning; it also happens on i386 (32-bit, as you said)
Comment 2 Martin Sebor 2017-12-01 22:09:39 UTC
Oddly, this only happens with the statement expression, not with direct calls to memset.

The invalid memset first shows up in the .phicprop2 dump which shows the output below.  This seems like another instance (similar to pr83239) where the value of the size operand to one these built-ins could be checked and, if it's invalid/out-of-bounds, the call replaced with a trap.

;; Function g (g, funcdef_no=0, decl_uid=1839, cgraph_uid=0, symbol_order=0)

  ...
  Replacing '__n_38' with constant '4294967295'
    Original statement:memset (_16, 0, __n_38);
    Updated statement:memset (_16, 0, 4294967295);
  ...
g (char * data, unsigned int frame_size, _Bool c)
{
  unsigned int __n;
  unsigned int __n;
  unsigned int _1;
  char * _6;
  char * _16;
  char * _22;
  unsigned int _25;
  unsigned int _28;

  <bb 2> [local count: 1073741825]:
  if (frame_size_8(D) != 0)
    goto <bb 3>; [33.00%]
  else
    goto <bb 10>; [67.00%]

  <bb 3> [local count: 354334802]:
  memset (data_10(D), 5, frame_size_8(D));
  if (c_12(D) != 0)
    goto <bb 4>; [0.00%]
  else
    goto <bb 5>; [100.00%]

  <bb 4> [local count: 354334802]:
  f ();

  <bb 5> [local count: 719407025]:
  _1 = frame_size_8(D) >> 1;
  __n_14 = _1 + 4294967295;
  _6 = data_10(D) + _1;
  _25 = _1 + 4294967285;
  if (__n_14 != 0)
    goto <bb 7>; [0.00%]
  else
    goto <bb 6>; [100.00%]

  <bb 6> [local count: 719407025]:
  # _22 = PHI <_6(5), _16(9), _6(7)>
  # _28 = PHI <_25(5), 4294967285(9), _25(7)>
  memset (_22, 4, _28);
  goto <bb 8>; [100.00%]

  <bb 7> [local count: 719407025]:
  memset (_6, 0, __n_14);
  if (_25 != 0)
    goto <bb 6>; [0.00%]
  else
    goto <bb 8>; [100.00%]

  <bb 8> [local count: 1073741825]:
  return;

  <bb 9> [local count: 354334800]:
  _16 = data_10(D);
  memset (_16, 0, 4294967295);
  goto <bb 6>; [100.00%]

  <bb 10> [local count: 719407025]:
  if (c_12(D) != 0)
    goto <bb 4>; [50.75%]
  else
    goto <bb 9>; [49.25%]

}
Comment 3 Marc Glisse 2017-12-01 22:44:54 UTC
This warning is "less wrong" than the other related ones. If frame_size is 0, this does call memset(,,-1). And there is an explicit test for frame_size == 0 in the function, which makes it look like 0 is not such an absurd value for frame_size. If we are going to keep the "maybe" variant of this warning at all, warning for this function makes sense...
Comment 4 Jeffrey A. Law 2017-12-05 00:38:15 UTC
IMHO the warning is correct here.  The code clearly does very bad things when frame is zero.  In that case we pass -1 to the memset #define.  Which ultimately results in the insane memset arguments.

This is *precisely* the kinds of things we want to be warning about.
Comment 5 Arnd Bergmann 2017-12-05 08:40:25 UTC
In that case, shouldn't we also warn if the conditional function call in front of it wasn't there, or without the '__n != 0' check?
Comment 6 Jeffrey A. Law 2017-12-05 19:11:08 UTC
It should.  It may not though because one the n != 0 test is removed, the resulting range of N is probably VR_VARYING rather than ~[0,0] at the call to memset.

The former signifies we know nothing about the length and given how often that likely occurs in practice I suspect the warning code suppresses the warning in that case.

The latter says we know the range is everything except 0.  With that sliver of information the warnings kick in.

It's a bit of speculation on my part, but it's informed speculation.