Unsafe value access

Tue Jun 27 16:47:23 UTC 2017

Thanks, Ah ValueTypeReturnedAsFields is enabled by default! i was playing around with that.

Roland, if you need a test case i can send you something.

Using -XX:-ValueTypeReturnedAsFields got me further, i have this for Unsafe.getValue:

UNSAFE_ENTRY(jobject, Unsafe_GetValue(JNIEnv *env, jobject unsafe, jobject jbase, jlong offset)) {
  oop base = JNIHandles::resolve(jbase);
  Handle base_h(THREAD, base); // is this necessary?

  InstanceKlass* base_klass = InstanceKlass::cast(base->klass());

  fieldDescriptor fd;
  base_klass->find_field_from_offset(offset, false, &fd);
  Klass* field_k = base_klass->get_value_field_klass(fd.index());
  ValueKlass* field_vklass = ValueKlass::cast(field_k);

  // allocate instance
  instanceOop res = field_vklass->allocate_instance(CHECK_NULL);
  // copy value
  int size = field_vklass->layout_helper_size_in_bytes(field_vklass->layout_helper()); // redundant?
  field_vklass->value_store(
    ((char*)(oopDesc*)base_h()) + offset, // offset in memory from base to the exploded value components
    ((char*)(oopDesc*)res) + field_vklass->first_field_offset(),
    true, false);

  ((instanceOopDesc*)res)->print();

  return JNIHandles::make_local(env, res);
} UNSAFE_END

The print verifies the value is being copied in the instanceOop “buffer".

But the value returned in Java is null. Same applies for the identity function i showed previously.

Paul.

P.S. do you use lldb on a mac, if so how does not enable the debugger to get access to the HS sources. Perhaps i need to enable a full debug build?

> On 27 Jun 2017, at 08:36, Frederic Parain <frederic.parain at oracle.com> wrote:
> 
> 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.
>>>> 
>>>> 
>>> 