RFR (L) JDK-7195622: CheckUnhandledOops has limited usefulness now

Lois Foltan lois.foltan at oracle.com
Thu Sep 19 08:10:40 PDT 2013


Please review the following fix:
     Webrev:
http://cr.openjdk.java.net/~hseigel/bug_jdk7195622.0/

Bug: JDK8 b44 hotspot:src/share/vm/oops/klass.hpp: Error:Initializing 
const volatile oop& requires ... &
         CheckUnhandledOops has limited usefulness now bug links at:

https://bugs.openjdk.java.net/browse/JDK-7180556
https://bugs.openjdk.java.net/browse/JDK-7195622

Summary of fix:

Update the C++ oop structure definition in oopsHierarchy.hpp to solve 
several problems with the current definition when compiled with various 
C++ compilers across supported platforms.  These changes initially 
address the problem reported in JDK-7180556 and continue with additional 
improvements to allow CHECK_UNHANDLED_OOPS to be defined in all 
fastdebug builds on all platforms as suggested in JDK-7195622.  Several 
notes concerning this fix:

             1. A review should start at understanding the changes made 
to oopsHierarchy.hpp
                     a.  Addition of a non-volatile copy constructor to 
address compile time errors
                          reported in JDK-7180556 and also currently by 
g++ compilers on Linux.
                     b.  Addition of member wise assignment operators to 
handle many instances
                          of [non]volatile to [non]volatile oops within 
the JVM.
                         Note: Solaris compilers would not allow for the 
member wise assignment operators
                                    of every flavor of non-volatile to 
volatile and vice versa.  However, unlike g++ compilers,
                                    Solaris compilers had no issue 
passing a volatile "this" pointer to a non-volatile
                                    assignment operator.  So the g++ 
compilers needed these different flavors
                                    of the assignment operator and 
Solaris did not.
                     d. For similar reasons as 1b, addition of a 
volatile explicit conversion from oop -> void *.
                         g++ specifically complained when trying to pass 
a volatile "this" pointer.
                     e.  Removal of the ambiguous behavior of having 
overloaded  copy constructor and
                          explicit user conversion member functions 
defined of both integral and void *.
                          All C++ compilers, except Solaris, issue a 
compile time error concerning this ambiguity.

             2. Change #1e had the consequence of C++ compilers now 
generating compile time
                 errors where the practice has been to cast an oop to 
and from an integral type such
                 as intptr_t.  To allow for this practice to continue, 
when oop is a structure and not
                 a OopDesc *, as is the case when CHECK_UNHANDLED_OOPS 
is defined, two new
                 macros were introduced within globalDefinitions.hpp, 
CAST_TO_OOP and CAST_FROM_OOP.

             3. Due to the change in #1a & #1b, the oop structure in no 
longer considered a POD (plain old data)
                 by the g++ compilers on Linux and MacOS.  This caused 
several changes to be needed
                 throughout the JVM to add an (void *) cast of an oop 
when invoking print_cr().

             4. Many instances of an assignment to a volatile oop 
required a "const_cast<oop&>" to
                 cast away the volatileness of the lvalue. There is 
already precedence for this
                 type of change within utilities/taskqueue.hpp.  The 
following comment was in place:

             // g++ complains if the volatile result of the assignment is
             // unused, so we cast the volatile away.  We cannot cast
        directly
             // to void, because gcc treats that as not using the result
        of the
             // assignment.  However, casting to E& means that we trigger an
             // unused-value warning.  So, we cast the E& to void.

             5. The addition of the volatile keyword to the 
GenericTaskQueue's pop_local() & pop_global()
                 member functions was to accommodate the Solaris C++ 
compiler's complaint about the assignment
                 of the volatile elements of the private member data 
_elems when GenericTaskQueue is instantiated
                 with a non-volatile oop.  Line #723 in pop_local().  
This was a result of #1b, Solaris' lack of
                 allowing for all flavors of volatile/non-volatile 
member wise assignment operators.
                 Per Liden of the GC group did pre-review this specific 
change with regards to performance
                 implications and was not concerned.

             6. In utilities/hashtable.cpp, required 
CHECK_UNHANDLED_OOPS conditional for the instantiation
                 of "template class Hashtable<oop, mtSymbol>".  With 
CHECK_UHANDLED_OOPS specified for a
                 fastdebug build, an unresolved symbol occurred.
                         philli:%  nm --demangle 
build//linux_amd64_compiler2/fastdebug/libjvm.so | grep Hashtable | grep 
seed
                                          U Hashtable<oop, (unsigned 
short)2304>::_seed
                         0000000000848890 t Hashtable<oop, (unsigned 
short)256>::seed()
                         ...

Making these improvements allows for CHECK_UNHANDLED_OOPS to be defined 
in all fastdebug builds across the supported platforms.

Builds:
Solaris (12u1 & 12u3 C++ compilers),
MacOS (llvm-g++ & clang++ compilers),
Linux (g++ v4.4.3 & g++ v4.7.3),
VS2010

Tests:
JTREG on MacOS,
vm.quick.testlist on LInux,
nsk.regression.testlist, nsk.stress.testlist on LInux,
JCK vm on Windows

Thank you, Lois








-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130919/7322e0b7/attachment-0001.html 


More information about the hotspot-runtime-dev mailing list