Bug 87672

Created attachment 44868 [details]
[Part 1] i386.ii

Created attachment 44869 [details]
[Part 2] i386.ii

Created attachment 44870 [details]
[Part 3] i386.ii

Created attachment 44871 [details]
[Part 4] i386.ii

Created attachment 44872 [details]
[Part 5] i386.ii

P.S.  In case it sheds any light, I just found out that after the build failure, if I manually re-run the stage 3 "./prev-gcc/xg++ ... .../i386/i386.c" command without "-D_FORTIFY_SOURCE=2", it doesn't error.  (I was curious if the stage 2 gcc was broken from being compiled with the option, and would fail on this file no matter what.)

Until dom3, everything is OK...

i386.ii.176t.dom3:
  <bb 15> [local count: 267871744]:
  __builtin___strcat_chk (&buf, _14, 40);
  output_asm_insn (&buf, operands_23(D));
  goto <bb 14>; [100.00%]

... but strlen does an invalid transformation,
which is does not even cause the ICE,
the last parameter should be 36, because
the strcpy_chk needs the size of the remaining
buffer after the first 4 bytes...

i386.ii.177t.strlen:
  <bb 15> [local count: 267871744]:
  _27 = &buf + 4;
  __builtin___strcpy_chk (_27, _14, 40);
  output_asm_insn (&buf, operands_23(D));
  goto <bb 14>; [100.00%]

i386.ii.178t.thread4:
  <bb 15> [local count: 267871744]:
  _27 = &buf + 4;
  __builtin___strcpy_chk (_27, _14, 40);
  output_asm_insn (&buf, operands_23(D));
  goto <bb 14>; [100.00%]

... and now in vrp2, the strcpy_chk is replaced
by something that looks not gimplified, which
causes the ICE:

i386.ii.179t.vrp2:
  <bb 15> [local count: 267871744]:
  _58 = (long unsigned int) ((ssizetype) _17 <= 7 ? 7 - (ssizetype) _17 : 0);
  _59 = _58 + 1;
  __builtin___memcpy_chk (&MEM[(void *)&buf + 4B], _14, _59, 40);
  output_asm_insn (&buf, operands_23(D));
  goto <bb 14>; [100.00%]

thus confirmed.

reduced test case:
$ cat pr87672.c 
char buf[40];
void test (int x)
{
  __builtin_strcpy(buf, "test");
  __builtin___strcat_chk(buf, "postfix" + x, sizeof(buf));
}
$ gcc -O2 -S -Wall pr87672.c 
pr87672.c: In function 'test':
pr87672.c:2:6: error: invalid operand in unary operation
    2 | void test (int x)
      |      ^~~~
pr87672.c:2:6: error: incorrect sharing of tree nodes
(ssizetype) _1
_8 = (long unsigned int) ((ssizetype) _1 <= 7 ? 7 - (ssizetype) _1 : 0);
during GIMPLE pass: vrp
pr87672.c:2:6: internal compiler error: verify_gimple failed
0xd58395 verify_gimple_in_cfg(function*, bool)
	../../gcc-trunk/gcc/tree-cfg.c:5422
0xc3534f execute_function_todo
	../../gcc-trunk/gcc/passes.c:1925
0xc3623e execute_todo
	../../gcc-trunk/gcc/passes.c:1979
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.

untested patch:

Index: gcc/gimple-fold.c
===================================================================
--- gcc/gimple-fold.c	(revision 265240)
+++ gcc/gimple-fold.c	(working copy)
@@ -2715,6 +2715,7 @@ gimple_fold_builtin_stxcpy_chk (gimple_stmt_iterat
 		return false;
 
 	      gimple_seq stmts = NULL;
+	      len = force_gimple_operand (len, &stmts, true, NULL_TREE);
 	      len = gimple_convert (&stmts, loc, size_type_node, len);
 	      len = gimple_build (&stmts, loc, PLUS_EXPR, size_type_node, len,
 				  build_int_cst (size_type_node, 1));
Index: gcc/tree-ssa-strlen.c
===================================================================
--- gcc/tree-ssa-strlen.c	(revision 265240)
+++ gcc/tree-ssa-strlen.c	(working copy)
@@ -2602,6 +2602,13 @@ handle_builtin_strcat (enum built_in_function bcod
       len = force_gimple_operand_gsi (gsi, len, true, NULL_TREE, true,
 				      GSI_SAME_STMT);
     }
+  if (objsz)
+    {
+      objsz = fold_build2_loc (loc, MINUS_EXPR, TREE_TYPE (objsz),
+			       objsz, unshare_expr (dstlen));
+      objsz = force_gimple_operand_gsi (gsi, objsz, true, NULL_TREE, true,
+					GSI_SAME_STMT);
+    }
   if (endptr)
     dst = fold_convert_loc (loc, TREE_TYPE (dst), unshare_expr (endptr));
   else

I can confirm applying Bernd Edlinger's "2018-10-21 18:16:41 UTC" patch on current trunk prevents all 3 reported errors, and allows using CPPFLAGS="-D_FORTIFY_SOURCE=2" and the additions of "-O2" into {libiberty,gcc}/configure.


Obviously, no one is obligated to provide me any insight.  (Of course, no one was even obligated to fix this, thanks Bernd for doing so!)  I'd love hearing: (1) how stage 2 was succeeding but stage 3 was failing, in a short "explain it to me like I can compile and use gcc and know the theory of bootstrapping, but have no idea what this patch is really doing"; and (2) if this patch fixes a somewhat "hidden/silent" bug that needed to be fixed even for people not compiling using these compilation flags, or if the fix is limited in scope to those using these flags who would run into the compilation error.

(In reply to jamespharvey20 from comment #11)
> I can confirm applying Bernd Edlinger's "2018-10-21 18:16:41 UTC" patch on
> current trunk prevents all 3 reported errors, and allows using
> CPPFLAGS="-D_FORTIFY_SOURCE=2" and the additions of "-O2" into
> {libiberty,gcc}/configure.
> 

Then you are lucky, because current trunk does not compile for me.

I had to roll back to r265240 first.

> 
> Obviously, no one is obligated to provide me any insight.  (Of course, no
> one was even obligated to fix this, thanks Bernd for doing so!)  I'd love
> hearing: (1) how stage 2 was succeeding but stage 3 was failing, in a short
> "explain it to me like I can compile and use gcc and know the theory of
> bootstrapping, but have no idea what this patch is really doing"; and (2) if
> this patch fixes a somewhat "hidden/silent" bug that needed to be fixed even
> for people not compiling using these compilation flags, or if the fix is
> limited in scope to those using these flags who would run into the
> compilation error.

I think stage 2 uses -fno-checking while stage 3 does -fchecking=1,
that explains why only stage 3 runs into the assertions.

My patch does two things, first it gimplifies the expression
"(long unsigned int) ((ssizetype) _19 <= 7 ? 7 - (ssizetype) _19 : 0);"

This fixes the ICE that was exposed by my patch.

And secondly it fixes the length parameter of the transformation
strcat_chk => strcpy_chk.  This wrong transformation is likely
already there in 8.0.

So yes, this is a real bug, and thanks for reporting, by the way.

Author: edlinger
Date: Sun Nov  4 19:46:08 2018
New Revision: 265777

URL: https://gcc.gnu.org/viewcvs?rev=265777&root=gcc&view=rev
Log:
gcc:
2018-11-04  Bernd Edlinger  <bernd.edlinger@hotmail.de>

        PR tree-optimization/87672
        * gimple-fold.c (gimple_fold_builtin_stxcpy_chk): Gimplify.
        * tree-ssa-strlen.c (handle_builtin_strcat): Adjust object size.

testsuite:
2018-11-04  Bernd Edlinger  <bernd.edlinger@hotmail.de>

        PR tree-optimization/87672
        * gcc.dg/pr87672.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/pr87672.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/gimple-fold.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-strlen.c

fixed.