JDK-8254189 : Improve comments for StackOverFlow and fix in_xxx() functions
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 16
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2020-10-07
  • Updated: 2021-01-07
  • Resolved: 2020-10-29
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 16
16 b23Fixed
Related Reports
Relates :  
Description
When reviewing JDK-8253717 we stumbled over the exclusiveness of the xxx_base values, which is not well known:

<quote>
https://github.com/openjdk/jdk/pull/522

From [~stuefe] code review comments for JDK-8253717:

+----------------+
|                      |   <-- stack_end()
| red zone       |
|                      |
+----------------+
|                     |   <-- red_zone_base()
| yellow zone    |
|                     |
     ....
|                     |
+----------------+
                         <-- stack_base()
Maybe its just me but I always have to think a bit more here. With downward growing stacks normal range thinking is reversed wrt to openness, so stack_base() points outside the stack and stack_end() is in the stack. This is true for all base values - they point to locations outside the zone they base.

Maybe that is clear to all others but it sometimes surprises me.

Also:

stack_base() points to one-beyond-the-highest address in stack and therefore outside the stack. If the stack is 8 pages, stack_base points to the start of the 9th page. Therefore stack_base may actually point into a different memory region, eg the stack of a neighboring thread, should they happen to be allocated without gap.

stack_red_zone_base() points to one-beyond-the-highest address in the red zone resp. the lowest address in the yellow zone. So it points outside the red zone.

And so forth, for all other "base" values. All are one-beyond pointers.

src/hotspot/share/runtime/stackOverflow.hpp:
  bool in_stack_reserved_zone(address a) {
    return (a <= stack_reserved_zone_base()) &&
           (a >= (address)((intptr_t)stack_reserved_zone_base() - stack_reserved_zone_size()));
  }
 @tstuefe
tstuefe 13 hours ago Member
Same here, a==reserved_zone_base is strictly speaking outside the reserved zone.

David Holmes remarked:

With regard to the discussion on whether "*_base()" functions are inclusive or exclusive, the stack_base() function is exclusive and we fixed a number of checks that were incorrectly checking <= stack_base() rather than < stack_base(). See:
https://bugs.openjdk.java.net/browse/JDK-8234372
and related issues.

</quote>

So, this is by design, but surprising if you don't keep in mind the reverse nature of thread stacks. Two things can be improved:

- there could be comments throughout StackOverFlow and for Thread::stack_base and ::stack_end
- the StackOverFlow::in...() functions should be revised. Some of them include the base pointer location, which is wrong.


Comments
Changeset: 4031cb41 Author: Thomas Stuefe <stuefe@openjdk.org> Date: 2020-10-29 06:30:03 +0000 URL: https://git.openjdk.java.net/jdk/commit/4031cb41
29-10-2020

Also from code review comments from JDK-8253717 [~stuefe] this needs to be fixed: Note that in_stack_red_zone() conflicts with in_stack_yellow_reserved_zone() since both in_stack_yellow_reserved_zone() and is_stack_red_zone() return true for a=red zone base. Strictly speaking it is wrong here since red zone base is not in red zone.
12-10-2020

[~stuefe] I was just about to link these issues, but you've already done it!
08-10-2020

The stack_base() is exclusive. We already went through this code extensively - see JDK-8234372
07-10-2020

Is the stack_base actually off by a page?
07-10-2020