[patch] hotspot miscompilation of OpenJDK6 with gcc from the gcc-4_3-branch

[Martin Buchholz]

I've thought about how best to fix the enum Cell crash.
Extending the range of the enum to
  enum Cell { Cell_0, Cell_max = MAX_INT }
works in practice.  But I was left wondering whether this was actually
standards-correct.  Is it legal to use values for an enum that were
not specified in the enum declaration (e.g. 1 for Cell above).
A check of the draft C++ standard gives this wording:

"An expression of arithmetic or enumeration type can be converted to
an enumeration type explicitly. The value is
  unchanged if it is in the range of enumeration values of the
enumeration type; otherwise the resulting enumeration value
  is unspecified."

which makes it look like extending the range is Just Right.

I might be tempted to create a proper "class Cell",
but there is not a lot of state or behavior there to encapsulate.

Martin

On Fri, Jul 25, 2008 at 6:51 AM, Matthias Klose <doko at ubuntu.com> wrote:
> BUILD FAILED
>
> the build failure is not seen when reverting r136501; seen as well when just
> reverting the two hunks for record_numbers_of_iterations.
>
> seen with -O3 and -O2, not -O1.
>
> not seen on amd64 and sparc (the other two archs using OpenJDK hotspot).
>
> the miscompiled file is ciTypeFlow.cpp, compiled using
> g++-4.3 -fpic -fno-rtti -fno-exceptions -D_REENTRANT -fcheck-new -g -m32
> -march=i586 -mtune=generic -O2 -fno-strict-aliasing -DVM_LITTLE_ENDIAN
> -Wpointer-arith -Wconversion -Wsign-compare -c ciTypeFlow.cpp
>
>
> Upstream GCC [1] doesn't agree on a bug in the compiler, but in the application
> code:
>
>  "I belive this is just INVALID.  The code seems to do lots of things with
>  this enum Cell, but the C++ compiler is allowed to just allocate 1 bit of
>  storage for it.
>
>  Maybe changing the Cell declaration to
>
>    enum Cell { Cell_0, Cell_max = UINT_MAX }
>
>  fixes the issue.
>
>  See 7.2/6 for the standard wording."
>
> The suggested fix is attached; I don't see any regressions. IcedTea currently
> has a patch to work around the problem, compiling this file with -fno-ivopts.
>
>   Matthias
>
> [1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36917
>
> --- 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
>
>