RFR: 8340491: Thread stack-base assertion should report which thread has the un-set stack [v2]

[David Holmes]

On Fri, 20 Sep 2024 06:39:35 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> src/hotspot/share/runtime/thread.hpp line 535:
>> 
>>> 533:  public:
>>> 534:   // Stack overflow support
>>> 535:   address stack_base() const DEBUG_ONLY(;) NOT_DEBUG({ return _stack_base; })
>> 
>> I think it is a bit cleaner to "just" outline the checking method like:
>> 
>> 
>> thread.hpp:
>>   address stack_base() const           { assert_stack_base(); return _stack_base; }
>>   void assert_stack_base() const NOT_DEBUG_RETURN;
>> 
>> thread.cpp:
>> #ifdef ASSERT
>> void  Thread::assert_stack_base() const {
>>   // Note: can't report Thread::name() here as that can require a ResourceMark which we
>>   // can't use because this gets called too early in the thread initialization.
>>   assert(_stack_base != nullptr, "Stack base not yet set for thread id:%d (0 if not set)",
>>          osthread() != nullptr ? osthread()->thread_id() : 0);
>> }
>> #endif
>
> This _is_ the way we do it in existing code, e.g.
> https://github.com/openjdk/jdk/blob/46b02f49bcc730d94e37cf17fa996fdd12bdb990/src/hotspot/share/runtime/stackWatermark.hpp#L105

So many ways to achieve the same thing ...

I would not want assert_stack_base in the public API.

I did need to fix the ifdef though.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21102#discussion_r1768071163