+ #define CAST_TO_OOP(value) ((oop)(CHECK_UNHANDLED_OOPS_ONLY((void *))(value))) + #define CAST_FROM_OOP(new_type, value) ((new_type)(CHECK_UNHANDLED_OOPS_ONLY((void *))(value)))
Could these two macros also be a method? On Sep 19, 2013, at 8:13 AM, Lois Foltan <lois.fol...@oracle.com> wrote: > > > 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 > > > > > > > > > >
