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

Coleen Phillimore coleenp at openjdk.java.net
Wed Oct 7 14:45:22 UTC 2020


On Wed, 7 Oct 2020 06:37:57 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fix comments
>
> src/hotspot/share/runtime/stackOverflow.hpp line 121:
> 
>> 119:   //  (large addresses)
>> 120:   //
>> 121:
> 
> Nice ascii art :)
> 
> I wish though it could communicate better the openness of the ranges. E.g.:
> 
> +----------------+
> |                |   <-- 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.

I just found this comment.  I think the ascii art was added by @GoeLin.  I just moved it. Your picture is upside down
but it sorta makes sense that the 'base' addresses point to the first address in the range, which is what I think they
do.

> src/hotspot/share/runtime/stackOverflow.hpp line 131:
> 
>> 129:   static size_t _stack_shadow_zone_size;
>> 130:  public:
>> 131:   static size_t stack_red_zone_size() {
> 
> Naming: since you moved these formerly-Thread-methods into this enclosing class all the "_stack" and "stack" prefixes
> for members and methods are not needed anymore.

I don't want to rename these here.

> src/hotspot/share/runtime/stackOverflow.hpp line 141:
> 
>> 139:     _stack_red_zone_size = s;
>> 140:   }
>> 141:   address stack_red_zone_base() {
> 
> could be const (as could a couple of others, I won't mark them individually)

The static member functions can't have const.  Let me see if there are others.

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

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


More information about the hotspot-dev mailing list