Bug 87672

Summary: [9 regression] 81512c36 causes ICE in bootstrap stage 3 using "-D_FORTIFY_SOURCE=2" (invalid operand in unary operation, incorrect sharing of tree nodes, verify_gimple failed)
Product: gcc Reporter: jamespharvey20
Component: tree-optimizationAssignee: Bernd Edlinger <bernd.edlinger>
Status: RESOLVED FIXED    
Severity: normal CC: bernd.edlinger, dcb314, dimhen, egallager, jamespharvey20, law, segher, ubizjak
Priority: P3    
Version: 9.0   
Target Milestone: 9.0   
See Also: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87387
Host: x86_64 Target: x86_64
Build: x86_64 Known to work: 8.0
Known to fail: 9.0 Last reconfirmed: 2018-10-21 00:00:00
Attachments: [Part 0] i386.ii
[Part 1] i386.ii
[Part 2] i386.ii
[Part 3] i386.ii
[Part 4] i386.ii
[Part 5] i386.ii

Description jamespharvey20 2018-10-21 04:14:55 UTC
Created attachment 44867 [details]
[Part 0] i386.ii

On trunk commit 7961f40be4b4a5d9c8531e6f78ecf330411d5d9f (2018-09-25) this succeeds:

(in gcc git dir)
$ sed -i "/ac_cpp=/s/\$CPPFLAGS/\$CPPFLAGS -O2/" {libiberty,gcc}/configure
(in gcc build dir)
$ export CPPFLAGS="-D_FORTIFY_SOURCE=2"
$ <gcc git dir>/configure --disable-werror
{ [make] || [make -j] }

Trunk commit 81512c36496bd5c3bc35746603dc988f0bc85f57 or anything newer, including current trunk/master, causes the following build failure.  With my basic knowledge of the bootstrap process, I'm very surprised it compiles in stage 2 but then fails in stage 3, but that's what happens.  I lost track of how many times I've confirmed this, performing a bisect, at first going down a wrong path of what I thought was causing this, then creating reduced steps.  It's 100% reproduceable, where before commit 81512c compiles, and at or after it fails with this error message.  (With all of Arch's normal build flags the first two errors are given but the internal compiler error isn't, until it's pruned down.)

It's not a hardware issue.  Kernel compiles fine.  System is rock solid.  Memtest passes 16+ hours.

In case someone a response is going to be given that I shouldn't be modifying these configure files this way, or using D_FORTIFY_SOURCE, I'll respond to that.  Arch Linux has been compiling gcc this exact way since 4.8.0-2 (2013-04-12) without problems.  (Arch of course customizes more than this, but I've reduced it down to the combination of the sed and CPPFLAGS.)  I understand that upstream changes can and should require distributions to modify their build process, especially distribution-specific ones, but:

(1) Even if true, this commit is causing an internal compiler error which should not happen.

(2) Even if true, it's worrisome (at least to me, given my limited boostrap process knowledge) that stage 2 can complete, but stage 3 can fail.

(3) I just view this as if Arch's normal gcc build process tests/stresses gcc's code a bit more than others might.  (No idea if other distributions perform these steps or not.)

After make failed, I re-ran the failing command with the "-save-temps" option to get the preprocess file.  The max attachment here is 1000 KB, and it's 5M, so I'm attaching 6 separate files from split.  I've also verified combining them in order is identical to my original i386.ii.

System is up-to-date Arch.  gcc 8.2.1, binutils 2.31.1, linux 4.18.14.  You can pull up the version of any other package at archlinux.org/packages by typing the package name into Keywords.



...
Configuring stage 3 in ./gcc
...
/home/jamespharvey20/gcc.mirror.git.build.git/build/./prev-gcc/xg++ -B/home/jamespharvey20/gcc.mirror.git.build.git/build/./prev-gcc/ -B/usr/local/x86_64-pc-linux-gnu/bin/ -nostdinc++ -B/home/jamespharvey20/gcc.mirror.git.build.git/build/prev-x86_64-pc-linux-gnu/libstdc++-v3/src/.libs -B/home/jamespharvey20/gcc.mirror.git.build.git/build/prev-x86_64-pc-linux-gnu/libstdc++-v3/libsupc++/.libs  -I/home/jamespharvey20/gcc.mirror.git.build.git/build/prev-x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu  -I/home/jamespharvey20/gcc.mirror.git.build.git/build/prev-x86_64-pc-linux-gnu/libstdc++-v3/include  -I/home/jamespharvey20/gcc.mirror.git.build.git/gcc.mirror.git.local/libstdc++-v3/libsupc++ -L/home/jamespharvey20/gcc.mirror.git.build.git/build/prev-x86_64-pc-linux-gnu/libstdc++-v3/src/.libs -L/home/jamespharvey20/gcc.mirror.git.build.git/build/prev-x86_64-pc-linux-gnu/libstdc++-v3/libsupc++/.libs -fno-PIE -c   -g -O2 -fchecking=1 -DIN_GCC     -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common  -DHAVE_CONFIG_H -I. -I. -I../../gcc.mirror.git.local/gcc -I../../gcc.mirror.git.local/gcc/. -I../../gcc.mirror.git.local/gcc/../include -I../../gcc.mirror.git.local/gcc/../libcpp/include  -I../../gcc.mirror.git.local/gcc/../libdecnumber -I../../gcc.mirror.git.local/gcc/../libdecnumber/bid -I../libdecnumber -I../../gcc.mirror.git.local/gcc/../libbacktrace  -D_FORTIFY_SOURCE=2 -o i386.o -MT i386.o -MMD -MP -MF ./.deps/i386.TPo ../../gcc.mirror.git.local/gcc/config/i386/i386.c
../../gcc.mirror.git.local/gcc/config/i386/i386.c: In function ‘const char* output_fix_trunc(rtx_insn*, rtx_def**, bool)’:
../../gcc.mirror.git.local/gcc/config/i386/i386.c:19200:1: error: invalid operand in unary operation
19200 | output_fix_trunc (rtx_insn *insn, rtx *operands, bool fisttp)
      | ^~~~~~~~~~~~~~~~
../../gcc.mirror.git.local/gcc/config/i386/i386.c:19200:1: error: incorrect sharing of tree nodes
(ssizetype) _19
_58 = (long unsigned int) ((ssizetype) _19 <= 7 ? 7 - (ssizetype) _19 : 0);
during GIMPLE pass: vrp
../../gcc.mirror.git.local/gcc/config/i386/i386.c:19200:1: internal compiler error: verify_gimple failed
Please submit a full bug report,
with preprocessed source if appropriate.
See <https://gcc.gnu.org/bugs/> for instructions.
make[3]: *** [Makefile:2245: i386.o] Error 1
make[3]: Leaving directory '/home/jamespharvey20/gcc.mirror.git.build.git/build/gcc'
make[2]: *** [Makefile:4689: all-stage3-gcc] Error 2
make[2]: Leaving directory '/home/jamespharvey20/gcc.mirror.git.build.git/build'
make[1]: *** [Makefile:24118: stage3-bubble] Error 2
make[1]: Leaving directory '/home/jamespharvey20/gcc.mirror.git.build.git/build'
make: *** [Makefile:952: all] Error 2
Comment 1 jamespharvey20 2018-10-21 04:17:30 UTC
Created attachment 44868 [details]
[Part 1] i386.ii
Comment 2 jamespharvey20 2018-10-21 04:18:01 UTC
Created attachment 44869 [details]
[Part 2] i386.ii
Comment 3 jamespharvey20 2018-10-21 04:18:51 UTC
Created attachment 44870 [details]
[Part 3] i386.ii
Comment 4 jamespharvey20 2018-10-21 04:19:17 UTC
Created attachment 44871 [details]
[Part 4] i386.ii
Comment 5 jamespharvey20 2018-10-21 04:19:41 UTC
Created attachment 44872 [details]
[Part 5] i386.ii
Comment 6 jamespharvey20 2018-10-21 10:13:46 UTC
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.)
Comment 7 Bernd Edlinger 2018-10-21 11:18:06 UTC
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%]
Comment 8 Bernd Edlinger 2018-10-21 11:31:28 UTC
thus confirmed.
Comment 9 Bernd Edlinger 2018-10-21 14:59:46 UTC
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.
Comment 10 Bernd Edlinger 2018-10-21 18:16:41 UTC
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
Comment 11 jamespharvey20 2018-10-22 08:33:27 UTC
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.
Comment 12 Bernd Edlinger 2018-10-22 09:11:09 UTC
(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.
Comment 13 Bernd Edlinger 2018-11-04 19:46:41 UTC
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
Comment 14 Bernd Edlinger 2018-11-04 19:52:21 UTC
fixed.