Crash in ciTypeFlow.cpp

[Tom Rodriguez]

Sorry this took so long.  I had intended to pick this up when it was  
first discussed but with vacation and other things I lost track of  
it.  Since we want everything, even trivial things, to go through our  
jprt build system, I picked this up and put it into the queue on its  
way to hotspot-comp/hotspot.  It should be completed by the end of the  
day since it is second in line.  The webrev is at http://webrev.invokedynamic.info/never/6741642 
.  INT_MAX is a good enough value since it should only contain  
positive integers.  Again sorry for the delay in handling this.

tom

On Aug 23, 2008, at 4:22 PM, Martin Buchholz wrote:

> 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