Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8254189

Improve comments for StackOverFlow and fix in_xxx() functions

    XMLWordPrintable

    Details

    • Type: Enhancement
    • Status: Resolved
    • Priority: P4
    • Resolution: Fixed
    • Affects Version/s: 16
    • Fix Version/s: 16
    • Component/s: hotspot
    • Labels:
      None
    • Subcomponent:
    • Resolved In Build:
      b23

      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.

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              stuefe Thomas Stuefe
              Reporter:
              coleenp Coleen Phillimore
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: