On Thu, 15 Jul 2021 12:25:54 GMT, Jorn Vernee <jver...@openjdk.org> wrote:
>> src/hotspot/share/prims/universalUpcallHandler.cpp line 76:
>>
>>> 74:
>>> 75: // modelled after JavaCallWrapper::JavaCallWrapper
>>> 76: Thread*
>>> ProgrammableUpcallHandler::on_entry(OptimizedEntryBlob::FrameData* context)
>>> {
>>
>> This should return JavaThread not Thread.
>
> Thanks.
>
> I've been careful here to return a `Thread*` since the result is stored in
> `r15_thread` and I thought converting between sub and super types could
> potentially result in different pointers due to the way super types are laid
> out within a subtype. I thought it worked like this:
>
>
> Subclass
> +---
> | {Subclass vtable pointer}
> | +--- (base class Super)
> | | {Super vtable pointer}
> | +---
> +---
>
>
> So, I thought plainly using a `JavaThread*` in generated machine code where a
> `Thread*` was expected could cause trouble, since the pointer needs to be
> offset for the type conversion.
>
> But now that I'm looking at some cases with compiler explorer, the pointer
> offset only seems to be needed when using multiple inheritance, for instance:
>
>
> class SuperA {
> public:
> virtual void foo();
> };
>
> class SuperB {
> public:
> virtual void bar();
> };
>
> class Sub : public SuperA, public SuperB {
> public:
> virtual void baz();
> };
>
>
> Results in:
>
>
> class Sub size(16):
> +---
> 0 | +--- (base class SuperA)
> 0 | | {vfptr}
> | +---
> 8 | +--- (base class SuperB)
> 8 | | {vfptr}
> | +---
> +---
>
> Sub::$vftable@SuperA@:
> | &Sub_meta
> | 0
> 0 | &SuperA::foo
> 1 | &Sub::baz
>
> Sub::$vftable@SuperB@:
> | -8
> 0 | &SuperB::bar
>
> Sub::baz this adjustor: 0
>
>
> (https://godbolt.org/z/1665fWzff)
>
> It seems that the sub type just reuses the vtable pointer of the first super
> type (probably to avoid having to do this pointer offsetting). Though,
> converting between `SuperB*` and `Sub*` would require offsetting the pointer.
> I'm still not sure this is guaranteed to work like this with all compilers
> though (the example is with MSVC, which has options to dump class layouts).
>
> The result of `on_entry` is stored in `r15_thread`, so I guess I'm wondering
> if it's safe to store a `JavaThread*` instead of a `Thread*` in `r15`, and
> other code, which may expect `r15` to hold a `Thread*`, is guaranteed to keep
> working? (FWIW, after changing the return type to `JavaThread*` the tests
> that exercise this code still pass on Windows with MSVC, and on WSL Linux
> with GCC).
Sorry, I sent the wrong godbolt link: https://godbolt.org/z/1665fWzff
-------------
PR: https://git.openjdk.java.net/jdk17/pull/149
