[Bug middle-end/97336] False positive -Wstring-compare warning for strncmp()

2020-10-09 Thread msebor at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97336

--- Comment #3 from Martin Sebor  ---
For better or worse, in the internal representation there's no way to
differentiate a call that was synthesized during loop unrolling (or any other
similar transformation) from the original call in the source code, or even tell
whether or not loop unrolling or any other such transformation took place.

It also helps to keep in mind that the documented purpose of warnings in GCC
"is to report constructions that are not inherently erroneous but that are
risky or suggest there may have been an error."  If the test case in comment #0
is representative of real code then even if it doesn't indicate a bug I would
suggest to view the warning as an indication that the code can be improved. 
(This is often true for all of these flow-based warnings.) 

In the test case in comment #3 the body of the true branch of the if statement
is eliminated early on because the condition (i == 5 || buf[5] == ' ') is known
to be false.

[Bug middle-end/97336] False positive -Wstring-compare warning for strncmp()

2020-10-09 Thread franke at computer dot org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97336

--- Comment #2 from Christian Franke  ---
Sorry, but I disagree that this report is INVALID.

Unlike for example -Wstringop-overflow, warnings like -Wstring-compare should
IMO only occur if the warning condition holds for *all* function calls
generated by loop unrolling.

BTW, if the partial loop unrolling is done manually, the warning should occur,
but does not:

int f(const char * p, int n)
{
  char buf[10] = {0, };
  int i = 0;
  if (n <= 0) {
if (!__builtin_strncmp(buf, "12345", 5) // <== no warning
&& (i == 5 || buf[5] == ' ')) // <== warning if removed
  return 1;
  }
  else {
do {
  buf[i] = p[i]; i++;
} while (i < (int)sizeof(buf)-1 && i < n);
if (!__builtin_strncmp(buf, "12345", 5)
&& (i == 5 || buf[5] == ' '))
  return 1;
  }
  return 0;
}

With the above code, the warning occurs if the condition "&& (i == 5 || buf[5]
== ' ')" is removed. If this is done in the original testcase, the warning does
no longer occur.

[Bug middle-end/97336] False positive -Wstring-compare warning for strncmp()

2020-10-08 Thread msebor at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97336

Martin Sebor  changed:

   What|Removed |Added

 Resolution|--- |INVALID
 CC||msebor at gcc dot gnu.org
 Status|UNCONFIRMED |RESOLVED

--- Comment #1 from Martin Sebor  ---
The warning works as intended: unless n > 4, the strncmp call will return
nonzero because the length of buf is less than 5.  GCC partially unrolls the
loop and the first iteration of it is what triggers the warning.  It disappears
when the buffer is cleared with memset only because GCC (as a limitation) loses
track of the length of the string in buf after that.  Handling the case when n
is zero or less (e.g., via if (n <= 0) return 0;) does as well.

The IL that leads up to the warning can be seen in the output of the
-fdump-tree-dom4 option (below).

f (const char * p, int n)
{
  sizetype ivtmp.6;
  int i;
  char buf[10];
  char _3;
  _Bool _4;
  _Bool _5;
  _Bool _6;
  int _7;
  int _10;
  char prephitmp_20;
  int _21;
  unsigned int _25;
  char pretmp_33;

   [local count: 118111600]:
  buf = ""; <<< strlen(buf) == 0
  if (n_15(D) > 0)
goto ; [89.00%]
  else
goto ; [11.00%]

  ...
   [local count: 12992276]:   <<< n == 0
  _21 = __builtin_strncmp (, "12345", 5);   <<< warning here
  if (_21 == 0) <<< folded to false
goto ; [50.00%]
  else
goto ; [50.00%]

   [local count: 6496138]:
  # prephitmp_20 = PHI <0(4)>

   [local count: 95940523]:
  goto ; [100.00%]

   [local count: 105119324]:
  _7 = __builtin_strncmp (, "12345", 5);<<< second strncmp call
  if (_7 == 0)
goto ; [50.00%]
  else
goto ; [50.00%]

   [local count: 52559662]:
  if (i_19 == 5)
goto ; [23.93%]
  else
goto ; [76.07%]

   [local count: 40175661]:
  pretmp_33 = buf[5];
  if (pretmp_33 == 32)
goto ; [25.01%]
  else
goto ; [74.99%]

   [local count: 118111600]:
  # _10 = PHI <1(9), 0(6), 1(8)>
  buf ={v} {CLOBBER};
  return _10;

}


Rewriting the code in a way that avoids the loop (e.g., by using memcpy
instead) also avoids the warning.