Unsafe value access

[Frederic Parain]

Paul,

This looks like an issue with the calling convention returning values
in registers. The store_value_type_fields_to_buf stub is trying the
reconstruct a value types from fields in registers, it calls into
the runtime without changing the thread state (which remains
_thread_in_Java) and the method in sharedRuntime then tries to
lock the SystemDictionary which causes the assertion failure.

You can try -XX:-ValueTypeReturnedAsFields to test if this is
effectively the return convention that causes the issue.

If this is the case, I'd recommend to contact Roland, as the
author of the calling/returning convention, he knows all the
details of this code.

Regards,

Fred

On 06/27/2017 10:57 AM, Paul Sandoz wrote:
> Hi Fred,
> 
> I am running into a crash that i think is related to returning a __Value from a JNI method.
> 
> Say i have the simplest of methods, an identity function:
> 
>    public native __Value getValueIdentity(__Value o);
> 
> Implementing that in unsafe.cpp:
> 
>    UNSAFE_ENTRY(jobject, Unsafe_GetValueIdentity(JNIEnv *env, jobject unsafe, jobject jbase)) {
>        return jbase;
>    } UNSAFE_END
> 
> And calling from Java i get a crash, after calling the call of the Java method but AFAICT before the next Java expression/statement is executed, i think in generated stub code trying to handle the returned value:
> 
> #  Internal Error (/Users/sandoz/Projects/jdk10/valhalla/hotspot/src/share/vm/runtime/mutex.cpp:1384), pid=75592, tid=5123
> #  assert((!thread->is_Java_thread() || ((JavaThread *)thread)->thread_state() == _thread_in_vm) || rank() == Mutex::special) failed: wrong thread state for using locks
> ...
> V  [libjvm.dylib+0xc9e274]  VMError::report_and_die(int, char const*, char const*, __va_list_tag*, Thread*, unsigned char*, void*, void*, char const*, int, unsigned long)+0x4d4
> V  [libjvm.dylib+0xc9ea36]  VMError::report_and_die(Thread*, char const*, int, char const*, char const*, __va_list_tag*)+0x4a
> V  [libjvm.dylib+0x4cc421]  report_vm_error(char const*, int, char const*, char const*, ...)+0xcd
> V  [libjvm.dylib+0xa45f8b]  Monitor::check_prelock_state(Thread*)+0x67
> V  [libjvm.dylib+0xa45c39]  Monitor::lock(Thread*)+0x97
> V  [libjvm.dylib+0xbf4b4a]  SystemDictionary::add_loader_constraint(Symbol*, Handle, Handle, Thread*)+0x138
> V  [libjvm.dylib+0xbf4ec0]  SystemDictionary::check_signature_loaders(Symbol*, Handle, Handle, bool, Thread*)+0xde
> V  [libjvm.dylib+0x8e45eb]  LinkResolver::check_method_loader_constraints(LinkInfo const&, methodHandle const&, char const*, Thread*)+0x283
> V  [libjvm.dylib+0x8e3c07]  LinkResolver::resolve_method(LinkInfo const&, Bytecodes::Code, Thread*)+0x48f
> V  [libjvm.dylib+0x8e3752]  LinkResolver::resolve_method_statically(Bytecodes::Code, constantPoolHandle const&, int, Thread*)+0x210
> V  [libjvm.dylib+0x28ee39]  Bytecode_invoke::static_target(Thread*)+0x7b
> V  [libjvm.dylib+0xb666a2]  SharedRuntime::store_value_type_fields_to_buf(JavaThread*, long)+0x1b2
> v  ~RuntimeStub::store_value_type_fields_to_buf
> j  UnsafeTest.main([Ljava/lang/String;)V+40
> 
> Any clues to what is going on or how to further debug?
> 
> Paul.
> 
>> On 22 Jun 2017, at 12:28, Frederic Parain <frederic.parain at oracle.com> wrote:
>>
>> Paul,
>>
>> For the interpreter, the implementation of getfield/putfield for value fields is split
>> between an assembly template and some runtime methods.
>>
>> The assembly template can be found in src/cpu/x86/vm/templateTable_x86.cpp,
>> look for this method:
>>
>> void TemplateTable::getfield_or_static(int byte_no, bool is_static, RewriteControl rc, bool is_vgetfield)
>>
>> The runtime support can be found in src/share/vm/interpreter/interpreterRuntime.cpp,
>> look for methods InterpreterRuntime::qgetfield(JavaThread* thread, oopDesc* value, int offset)
>> and InterpreterRuntime::qputfield(JavaThread* thread, oopDesc* obj, oopDesc* value, int offset).
>>
>> Mr Simms did a number of experiments at the beginning of the project to measure
>> the cost of the different solutions to provide atomicity, I would recommend to sync
>> with him.
>>
>> Regards,
>>
>> Fred
>>
>>
>>> On Jun 22, 2017, at 15:18, Paul Sandoz <paul.sandoz at oracle.com> wrote:
>>>
>>> Hi,
>>>
>>> To start working on VarHandle integration i first of all need unsafe access to values held within objects. For a most basic initial implementation we can use a global lock to preserve atomicity and memory ordering effects and defer the thinking about more sophisticated locking (seq locks etc), read-mody-write operations and other memory order effects to a another day.
>>>
>>> For these purposes we just require two methods:
>>>
>>> __Value getValue(Object base, long offset, Class<? extends Value> vt)
>>>
>>> void putValue(Object base, long offset, Class<? extends Value> vt, __Value v);
>>>
>>> I included Class parameter for the value type token.
>>>
>>> How feasible would it be to implement such methods?
>>>
>>> If people point me to the hotspot interpreter code for getfield/putfield implementations for values i might be able to make some progress. Once that is done we could then consider C1/C2 support.
>>>
>>> Thoughts?
>>>
>>> Thanks,
>>> Paul.
>>>
>>>
>>
>