[Bug c++/100197] New: g++ emits spurious Wstring-compare warnings on strcmp()

[Bug c++/100197] New: g++ emits spurious Wstring-compare warnings on strcmp()

[jaf at meyersound dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100197

            Bug ID: 100197
           Summary: g++ emits spurious Wstring-compare warnings on
                    strcmp()
           Product: gcc
           Version: 10.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jaf at meyersound dot com
  Target Milestone: ---

Created attachment 50653
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50653&action=edit
preprocessed file from teststring.cpp

Tested with g++ (Ubuntu 10.20-13ubuntu1) 10.2.0 (as installed via "sudp apt-get
install g++") on an Ubuntu 20.10 64-bit VM running inside VMWare Fusion on a
2018 Mac Mini.

To reproduce, compile the attached .cpp file, like this:

$ g++ -O2 -W -c teststring.cpp
In member function ‘bool String::operator==(const char*) const’,
    inlined from ‘int main(int, char**)’ at teststring.cpp:52:21:
teststring.cpp:27:21: warning: ‘int strcmp(const char*, const char*)’ of a
string of length 10 and an array of size 8 evaluates to nonzero
[-Wstring-compare]
   27 |       return (strcmp(myStr, rhs) == 0);
      |               ~~~~~~^~~~~~~~~~~~
teststring.cpp: In function ‘int main(int, char**)’:
teststring.cpp:52:16: note: in this expression
   52 |    if ((rand())&&(s == "1234567890")) printf("Mwahey!\n");
      |        ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~

Note that the emitted warnings are (AFAICT) spurious, because in the example
program the two strings that get passed to strcmp() are both "1234567890".  The
8-byte array (_smallBuffer[8]) that the warning mentions is not actually used.

It seems like maybe the compiler is assuming that the
IsArrayDynamicallyAllocated() method in the example will always return false,
which is not a valid assumption.

The fault only happens when I specifed -O2 or -O3, it doesn't happen if I leave
optimization disabled or specify -O1.

[Bug tree-optimization/100197] g++ emits spurious Wstring-compare warnings on strcmp()

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100197

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |10.2.0, 11.0
             Status|UNCONFIRMED                 |NEW
           Keywords|                            |alias, missed-optimization
   Last reconfirmed|                            |2021-04-22
     Ever confirmed|0                           |1
          Component|c++                         |tree-optimization

--- Comment #1 from Martin Sebor <msebor at gcc dot gnu.org> ---
The warning only runs at -O2 (and with -Wextra).  It works as designed.  It's
issued for the IL below.  The String::_smallBuffer is defined as:

   char _smallBuffer[8];

The reason for the warning is missing folding/constant prtopagation due to
overly conservative alias analysis: GCC assumes that the call to printf() might
modify the String object because it's passed the this pointer (the address of
&s).  That prevents the constant store to _buffLen from being propagated.  But
the only standard directive that can clobber an object is %n, so the assumption
could be avoided by scanning the format string for %n (and either matching it
to the argument or simply assuming it clobbers any argument).  This is the
missed-optimization part.

The same false positive would still happen with any other function whose
semantics GCC doesn't have knowledge of.  To avoid it in those cases the
warning (but not optimizers) could assume that the argument might be clobbered
only when passed to a non-const pointer/reference.  This would be a good change
to make in all these flow-sensitive warnings (all of which are subject to the
same class of false positives as a result of the overly conservative
escape/alias analysis).  This is the diagnostic aspect of this bug.

int main (int D.4559, char * * D.4560)
{
  bool D.4665;
  const size_t sLen;
  struct String s;
  int _1;
  int _6;
  long unsigned int _8;
  char * _9;
  long unsigned int _10;
  char * _11;
  long unsigned int _20;
  char * iftmp.2_21;
  int _22;
  char * iftmp.2_24;
  void * _26;
  long unsigned int _27;
  const char * iftmp.3_28;

  <bb 2> [local count: 1073741824]:
  s ={v} {CLOBBER};
  s._length = 0;
  s._bufferLen = 128;                               <<< store
  _26 = malloc (128);
  s._bigBuffer = _26;
  __printf_chk (1, "EnsureBufSize on String %p:  _bigBuffer=%p
_bufferLen=%zu\n", &s, _26, 128);                                    <<<
assumed to clobber s._bufferLen
  _20 = MEM[(const struct String *)&s]._bufferLen;  <<< load
  if (_20 > 8)
    goto <bb 3>; [50.00%]
  else
    goto <bb 4>; [50.00%]

  <bb 3> [local count: 536870913]:
  iftmp.2_21 = s._bigBuffer;

  <bb 4> [local count: 1073741824]:
  # iftmp.2_24 = PHI <iftmp.2_21(3), &s._smallBuffer(2)>
  __builtin_memcpy (iftmp.2_24, "1234567890", 11);
  s._length = 10;
  _1 = rand ();
  if (_1 != 0)
    goto <bb 6>; [50.00%]
  else
    goto <bb 5>; [50.00%]

  <bb 5> [local count: 714038312]:
  _8 = s._bufferLen;
  if (_8 > 8)
    goto <bb 8>; [24.44%]
  else
    goto <bb 10>; [75.56%]

  <bb 6> [local count: 536870913]:
  _27 = s._bufferLen;                               <<< load
  if (_27 > 8)
    goto <bb 7>; [50.00%]
  else
    goto <bb 15>; [50.00%]

  <bb 7> [local count: 268435456]:
  iftmp.3_28 = s._bigBuffer;
  _22 = strcmp (iftmp.3_28, "1234567890");
  if (_22 == 0)
    goto <bb 9>; [33.00%]
  else
    goto <bb 8>; [67.00%]

  <bb 8> [local count: 354334801]:
  _9 = s._bigBuffer;
  free (_9);
  goto <bb 11>; [100.00%]

  <bb 9> [local count: 177167400]:
  __builtin_puts (&"Mwahey!"[0]);
  goto <bb 5>; [100.00%]

  <bb 10> [local count: 719407023]:

  <bb 11> [local count: 1073741824]:
  s ={v} {CLOBBER};
  s ={v} {CLOBBER};
  return 0;

  <bb 12> [count: 0]:
<L10>:
  _10 = s._bufferLen;
  if (_10 > 8)
    goto <bb 13>; [0.00%]
  else
    goto <bb 14>; [0.00%]

  <bb 13> [count: 0]:
  _11 = s._bigBuffer;
  free (_11);

  <bb 14> [count: 0]:
  s ={v} {CLOBBER};
  resx 2

  <bb 15> [local count: 268435457]:
  _6 = strcmp (&s._smallBuffer, "1234567890");      <<< -Wstring-compare
  if (_6 == 0)
    goto <bb 9>; [33.00%]
  else
    goto <bb 10>; [67.00%]

}

[Bug tree-optimization/100197] g++ emits spurious Wstring-compare warnings on strcmp()

[rajpal.gusain at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100197

--- Comment #2 from Rajpal Singh <rajpal.gusain at gmail dot com> ---
I also get similar error when one of the argument string is constant and I
believe it's related. I tried it with g++ 11.2

'int strncmp(const char*, const char*, size_t)' of strings of length 1 and 6
and bound of 6 evaluates to nonzero [-Werror=string-compare]

if ((std::strncmp(tmp, "define", 6) == 0) && (std::isspace(*(tmp+6)))) {

In no possible way, length of tmp can be 1 and error is definitely spurious and
it's only with -O3 and -Wall.

Moreover, Clang can compile it fine without any issue with same optimization
level.