Bug 1215127 - Miscompilation of hotspot's type.o due to value range propagation on trees (tree-vrp) optimization
Summary: Miscompilation of hotspot's type.o due to value range propagation on trees (t...
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: gcc
Version: 22
Hardware: x86_64
OS: Linux
unspecified
unspecified
Target Milestone: ---
Assignee: Jakub Jelinek
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2015-04-24 11:10 UTC by Severin Gehwolf
Modified: 2015-04-24 15:26 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-04-24 15:12:30 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
reproducer.sh (441 bytes, text/plain)
2015-04-24 11:10 UTC, Severin Gehwolf
no flags Details
type.o.i (with some __attribute__ tries which I've added) (5.09 MB, text/plain)
2015-04-24 11:12 UTC, Severin Gehwolf
no flags Details
0001-spec-file-patch-for-GCC-reproducer.patch (3.97 KB, patch)
2015-04-24 11:14 UTC, Severin Gehwolf
no flags Details | Diff
relink.sh (9.37 KB, text/plain)
2015-04-24 11:14 UTC, Severin Gehwolf
no flags Details

Description Severin Gehwolf 2015-04-24 11:10:00 UTC
Created attachment 1018412 [details]
reproducer.sh

Description of problem:
GCC 5 miscompiles one object of libjvm.so (from the java-1.8.0-openjdk package) incorrectly. I've narrowed it down to type.o but I was yet unsuccessful to narrow it down further. For more background see bug 1208369.

Version-Release number of selected component (if applicable):
gcc-5.0.0-0.21.fc22.x86_64

How reproducible:
100%

Steps to Reproduce:
1. Build a java-1.8.0-openjdk fast debug build using the attached spec file patch. I can provide srpm if needed.
1.1 $ fedpkg clone java-1.8.0-openjdk
1.2 $ fedpkg switch-branch f22
1.3 $ git checkout db2c528191c5b19dca09d6e234e6bdcedaebc1bb
1.4 $ git am 0001-spec-file-patch-for-GCC-reproducer.patch
1.5.$ fedpkg local
2. In directory java-1.8.0-openjdk/$NVR.$ARCH/jdk8/build/jdk8.build/hotspot/linux_amd64_compiler2/fastdebug all the *.o objects needed.
3. copy type.o.i into that dir
4. cd into java-1.8.0-openjdk/$NVR.$ARCH/jdk8/build/jdk8.build/hotspot/linux_amd64_compiler2/fastdebug
5. $ rm -f type.o && rm -f libjvm.so && /usr/bin/g++ -fPIC -fno-rtti -fno-exceptions -fcheck-new -fvisibility=hidden -m64  -pipe -fno-strict-aliasing  -g -O3 -Werror -Wpointer-arith -Wsign-compare -Wundef -Wunused-function -Wunused-value -fstack-protector-strong -fno-devirtualize -Wno-return-local-addr -c -MMD -MP -MF ../generated/dependencies/type.o.d -fpch-deps -o type.o type.o.i && bash path/to/relink.sh
6. run reproducer.sh script which either asserts as in 
   https://bugzilla.redhat.com/show_bug.cgi?id=1208369#c17 or runs fine
7. BUILD_DIR in reproducer.sh should be java-1.8.0-openjdk/$NVR.$ARCH/jdk8/build/jdk8.build/

Actual results:
Miscompiled object with tree-vrp turned on.

Expected results:
Working type.o with tree-vrp turned on.

Additional info:
Compile attached pre-processed file type.o.i with and without tree-vrp turned on. The resulting type.o will assert in a fastdebug build of java-1.8.0-openjdk and run fine if turned off respectively.

I'll be happy to help if you have trouble reproducing the issue.

Comment 1 Severin Gehwolf 2015-04-24 11:12:05 UTC
Created attachment 1018413 [details]
type.o.i (with some __attribute__ tries which I've added)

Comment 2 Severin Gehwolf 2015-04-24 11:14:06 UTC
Created attachment 1018414 [details]
0001-spec-file-patch-for-GCC-reproducer.patch

Comment 3 Severin Gehwolf 2015-04-24 11:14:41 UTC
Created attachment 1018415 [details]
relink.sh

Comment 4 Jakub Jelinek 2015-04-24 13:18:24 UTC
With all the optimize attributes you have in there this seems to have started with http://gcc.gnu.org/r219823 , but wonder if that isn't exactly because of the optimize attributes with that revision.

Comment 5 Jakub Jelinek 2015-04-24 15:12:04 UTC
So, after removing all the __attribute__((optimize ("O0"))) stuff, I've bisected this to http://gcc.gnu.org/r215697 , which is indeed a VRP change and looking e.g. at vrp1 diffs, I see changes e.g. in TypeInt::xdual or TypeInt::make (i.e. functions that inline normalize_int_widen or normalize_long_widen).

And, by adding noinline/noclone on both normalize_{int,long}_widen, and then optimize (0) on them selectively to find which one is the problem, I've came up with short reproducer:

__attribute__((noinline, noclone)) int
normalize_long_widen (long long lo, long long hi, int w)
{
  if (lo <= hi)
    {
      if ((unsigned long long) (hi - lo) <= 3ULL)
	w = 0;
      if ((unsigned long long) (hi - lo) >= -1ULL)
	w = 3;
    }
  else
    {
      if ((unsigned long long) (lo - hi) <= 3ULL)
	w = 0;
      if ((unsigned long long) (lo - hi) >= -1ULL)
	w = 0;
    }
  return w;
}

int
main ()
{
  __builtin_printf ("%d\n", normalize_long_widen (-__LONG_LONG_MAX__ - 1,
                                                  __LONG_LONG_MAX__, 16));
  return 0;
}

and that just seems that hotspot relies on undefined behavior, as LONG_LONG_MIN is smaller than LONG_LONG_MAX (thus lo <= hi is true), but hi - lo is undefined behavior.

Guess you want something like (so that the subtraction is performed in unsigned type with well defined overflow):
--- type.o.i~	2015-04-24 14:17:08.000000000 +0200
+++ type.o.i	2015-04-24 17:07:08.258819716 +0200
@@ -129471,11 +129471,11 @@ static int normalize_int_widen( jint lo,
 
 
   if (lo <= hi) {
-    if ((juint)(hi - lo) <= ((juint)3)) w = Type::WidenMin;
-    if ((juint)(hi - lo) >= max_juint) w = Type::WidenMax;
+    if ((juint)hi - lo <= ((juint)3)) w = Type::WidenMin;
+    if ((juint)hi - lo >= max_juint) w = Type::WidenMax;
   } else {
-    if ((juint)(lo - hi) <= ((juint)3)) w = Type::WidenMin;
-    if ((juint)(lo - hi) >= max_juint) w = Type::WidenMin;
+    if ((juint)lo - hi <= ((juint)3)) w = Type::WidenMin;
+    if ((juint)lo - hi >= max_juint) w = Type::WidenMin;
   }
   return w;
 }
@@ -129753,11 +129753,11 @@ static int normalize_long_widen( jlong l
 
 
   if (lo <= hi) {
-    if ((julong)(hi - lo) <= ((juint)3)) w = Type::WidenMin;
-    if ((julong)(hi - lo) >= max_julong) w = Type::WidenMax;
+    if ((julong)hi - lo <= ((juint)3)) w = Type::WidenMin;
+    if ((julong)hi - lo >= max_julong) w = Type::WidenMax;
   } else {
-    if ((julong)(lo - hi) <= ((juint)3)) w = Type::WidenMin;
-    if ((julong)(lo - hi) >= max_julong) w = Type::WidenMin;
+    if ((julong)lo - hi <= ((juint)3)) w = Type::WidenMin;
+    if ((julong)lo - hi >= max_julong) w = Type::WidenMin;
   }
   return w;
 }

(of course applied to the source, not preprocessed source) as a fix.
Or ((juint)hi - (juint)lo) instead of (juint)hi - lo, depends on the projects coding conventions...
You could have tried -fsanitize=undefined first before filing bugs... (next time).

Comment 6 Severin Gehwolf 2015-04-24 15:26:41 UTC
(In reply to Jakub Jelinek from comment #5)
> You could have tried -fsanitize=undefined first before filing bugs... (next
> time).

Thank you very much for the analysis, Jakub. I'll certainly use -fsanitize=undefined next time before filing bugs. Thanks again!


Note You need to log in before you can comment on or make changes to this bug.