RFR: 8253717: Relocate stack overflow code out of thread.hpp/cpp [v2]

[Coleen Phillimore]

On Tue, 6 Oct 2020 22:07:55 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8253717: Relocate stack overflow code out of thread.hpp/cpp
>
> src/hotspot/share/jvmci/vmStructs_jvmci.cpp line 183:
> 
>> 181:   nonstatic_field(JavaThread,                  _should_post_on_exceptions_flag,
>> int)                                   \ 182:   nonstatic_field(JavaThread,
>> _jni_environment,                              JNIEnv)                                \ 183:
>> nonstatic_field(JavaThread,                  _stack_overflow_state._reserved_stack_activation,
>> address)                            \
> 
> This doesn't look right. I thought these had to be direct entries for fields in given classes - not indirections??

Yes, this does work.  I confirmed it on the graal internal slack page with Doug Simon.  And there are tests that use
this.

> src/hotspot/share/runtime/thread.cpp line 1669:
> 
>> 1667:   _on_thread_list(false),
>> 1668:   DEBUG_ONLY(_java_call_counter(0) COMMA)
>> 1669:   _entry_point(nullptr),
> 
> I didn't realize we had made the NULL -> nullptr switch yet ??

Kim said I should use it:
Kim Barrett  10:12 AM
Quoting the style guide:
nullptr
Prefer nullptr (n2431) to NULL. Don't use (constexpr or literal) 0 for pointers.
For historical reasons there are widespread uses of both NULL and of integer 0 as a pointer value.
I don't know how many people actually read the changes I made.

> src/hotspot/share/runtime/thread.cpp line 2953:
> 
>> 2951: // Verification
>> 2952:
>> 2953: void JavaThread::frames_do(void f(frame*, const RegisterMap* map)) {
> 
> Where does this come from ??

It was moved from code above that was in the middle of the stack overflow code.  I moved it to right before it's called.

> src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/GraalHotSpotVMConfig.java
> line 398:
>> 396:     public final int jvmciCountersThreadOffset = getFieldOffset("JavaThread::_jvmci_counters", Integer.class,
>> "jlong*"); 397:     public final int doingUnsafeAccessOffset = getFieldOffset("JavaThread::_doing_unsafe_access",
>> Integer.class, "bool", Integer.MAX_VALUE, JVMCI || JDK >= 14); 398:     public final int
>> javaThreadReservedStackActivationOffset = JDK <= 8 ? 0 :
>> getFieldOffset("JavaThread::_stack_overflow_state._reserved_stack_activation", Integer.class, "address"); // JDK-8046936
> 
> Again unclear this can actually work.

Also code that I discussed with graal folks.

-------------

PR: https://git.openjdk.java.net/jdk/pull/522