On 9/19/2013 6:09 PM, Christian Thalinger wrote:
+ #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?
Hi Christian,
I assume by method you are implying methods within the oop class
itself? That would work only in the case of fastdebug builds where an
oop is defined as a class. In non-fastdebug builds, an oop is a
(oopDesc *). The macros provided a way to preserve the existing cast to
& from an oop to a numerical type in all builds, even non-fastdebug ones.
Thanks for the initial review,
Lois
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
