> On Sep 7, 2020, at 5:47 PM, David Holmes <dhol...@openjdk.java.net> wrote:
>
> […]
> Commit messages:
> - Initial changes for 8252406
>
> Changes: https://git.openjdk.java.net/jdk/pull/37/files
> Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=37&range=00
> Issue: https://bugs.openjdk.java.net/browse/JDK-8252406
> Stats: 463 lines in 112 files changed: 15 ins; 124 del; 324 mod
> Patch: https://git.openjdk.java.net/jdk/pull/37.diff
> Fetch: git fetch https://git.openjdk.java.net/jdk pull/37/head:pull/37
>
> PR: https://git.openjdk.java.net/jdk/pull/37
------------------------------------------------------------------------------
src/hotspot/share/runtime/thread.hpp
506 JavaThread* as_Java_thread() {
507 assert(is_Java_thread(), "incorrect cast to JavaThread");
508 return (JavaThread*)this;
509 }
This should be using a static_cast. However, making that change will
uncover a problem.
This definition cannot correctly be located here. It must follow the
definition of the JavaThread class. At this point (in the body of the
Thread class definition), JavaThread is incomplete, so the C-style
cast is a reinterpret_cast. It's only by implementation accident (and
that we're not using multiple or virtual inheritance anywhere in the
vicinity), that a reinterpret_cast happens to work.
(The definition can remain in this file if that's more
convenient #include-wise than putting it in thread.inline.hpp, though
the latter would be the more usual placement.)
(One of the very real dangers with C-style casts is that you might not
be doing the cast you think you are doing.)
------------------------------------------------------------------------------
src/hotspot/share/runtime/thread.hpp
510 JavaThread * as_const_Java_thread() const {
511 assert(is_Java_thread(), "incorrect cast to JavaThread");
512 return (JavaThread*)this;
513 }
This should be
const JavaThread* as_Java_Thread() const {
assert(is_Java_thread(), "incorrect cast to JavaThread");
return static_cast<const JavaThread*>(this);
}
And of course, moved after the definition of JavaThread, per the above
discussion of the non-const function.
------------------------------------------------------------------------------
I reviewed all the (non-Shenandoah) gc changes, and they look good.
