RFR: 8343531: Improve print_location for invalid heap pointers [v2]

Volker Simonis simonis at openjdk.org
Mon Nov 4 11:00:31 UTC 2024


On Mon, 4 Nov 2024 10:03:34 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Volker Simonis has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Small refactoring based on tschatzl's review
>
> src/hotspot/share/gc/shared/locationPrinter.inline.hpp line 57:
> 
>> 55:   // Check if addr points into Java heap.
>> 56:   if (CollectedHeapT::heap()->is_in(addr)) {
>> 57:     // base_oop_or_null() might be unimplemented and return NULL for some GCs/generations
> 
> In such cases where the flag that we later set is dependent on the complete condition, it seems nicer to assign the result of the condition to it right away. That saves the assignment later too, having only a single assignment to it. Ymmv.
> Suggestion:
> 
>   // Check if addr points into Java heap.
>   bool in_heap = CollectedHeapT::heap()->is_in(addr);
>   if (in_heap) {
>     // base_oop_or_null() might be unimplemented and return NULL for some GCs/generations.
> 
> 
> (And drop the assignment to `in_heap` later).

Thanks for looking at this PR. Your suggestion sound like a reasonable simplification. I've updated the code accordingly.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21870#discussion_r1827548054


More information about the hotspot-gc-dev mailing list