Crash in ciTypeFlow.cpp

[Martin Buchholz]

Hi hotspot maintainers,

For a while now, there's been a crash in hotspot compiled with gcc 4.2
in ciTypeFlow.cpp (crashes in Swingset demo)

There have been a number of approaches to fixing it.
It appears that Matthias Klose has patched icedtea6 as follows:



--- openjdk/hotspot/src/share/vm/ci/ciTypeFlow.hpp~	2008-07-10
22:04:30.000000000 +0200
+++ openjdk/hotspot/src/share/vm/ci/ciTypeFlow.hpp	2008-07-25
14:32:03.544802121 +0200
@@ -130,7 +130,7 @@

   // Used as a combined index for locals and temps
   enum Cell {
-    Cell_0
+    Cell_0, Cell_max = UINT_MAX
   };

   // A StateVector summarizes the type information at some


Unfortunately, this fails to compile (at least with gcc 4.0
and OpenJDK7)

cc1plus: warnings being treated as errors
/usr/local/google/home/martin/ws/hotspot/hotspot/src/share/vm/ci/ciTypeFlow.cpp:
In member function 'const ciTypeFlow::StateVector*
ciTypeFlow::get_start_state()':
/usr/local/google/home/martin/ws/hotspot/hotspot/src/share/vm/ci/ciTypeFlow.cpp:392:
warning: comparison between signed and unsigned integer expressions
make[6]: *** [ciTypeFlow.o] Error 1


Here's another try,
and this time let's try to get it into both OpenJDK7 and OpenJDK6.
I'll do the push into OpenJDK7.

# HG changeset patch
# User martin
# Date 1219532277 25200
# Node ID 52c7e88431fc50fd682a0506cd9588c476ca7a00
# Parent  f8068895c22d848b6f0e6998886652c3d2f51b24
6666666: Crash in ciTypeFlow with gcc 4.2, enum Cell range too small
Reviewed-by:
Contributed-by: doko at ubuntu.com

diff --git a/src/share/vm/ci/ciTypeFlow.hpp b/src/share/vm/ci/ciTypeFlow.hpp
--- a/src/share/vm/ci/ciTypeFlow.hpp
+++ b/src/share/vm/ci/ciTypeFlow.hpp
@@ -127,7 +127,7 @@

   // Used as a combined index for locals and temps
   enum Cell {
-    Cell_0
+    Cell_0, Cell_max = INT_MAX
   };

   // A StateVector summarizes the type information at some


There doesn't seem to be a bug for this in bugtraq.
Sun folk, please file a bug,
and let me know which team hg forest to push this into.
For those of us using newer gccs, this is a P1 bug.

As justification, note that the existing code is illegal C++
Enum variables must take on values in the range of the enum constants,
which was not the case with the old code.

@doko: please review.  My version of this change maintains the
signedness of enum Cell, avoiding possible changes in behavior
and subtleties with signed/unsigned comparison.

Let's all try harder to get "community"-developed patches upstream.

Thanks,

Martin