RFR: 8343531: Improve print_location for invalid heap pointers

[Thomas Schatzl]

On Mon, 4 Nov 2024 09:43:18 GMT, Volker Simonis <simonis at openjdk.org> wrote:

> Currently `BlockLocationPrinter<CollectedHeapT>::print_location()` checks for a pointer if it points into the heap and if that's true, it either prints it as an oop if `is_valid_obj()` is true or it tries to find the the start address of an oop for that pointer by calling `CollectedHeapT::heap()->block_start()`.
> 
> However, the `block_start()` functionality is not fully implemented for all GCs (e.g. the young generation of `ParallelScavengeHeap`) and for these cases `block_start()` returns NULL. Because of this NULL return value `os::print_location()` will finally qualify the corresponding pointer as pointing "into unknown readable memory" although we already know that it actually points into an invalid heap area.
> 
> In such cases, print at least that the pointer is pointing into an unknown part of the heap instead of just saying that it points into unknown memory. 
> 
> I've manually tested the new functionality in GDB.

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).

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

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