RFR: 8343531: Improve print_location for invalid heap pointers [v2]
Aleksey Shipilev
shade at openjdk.org
Mon Nov 4 12:04:29 UTC 2024
On Mon, 4 Nov 2024 10:46:02 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.
>
> Volker Simonis has updated the pull request incrementally with one additional commit since the last revision:
>
> Small refactoring based on tschatzl's review
I think this is a fine papercut fix, and implementing block information for all GCs could be tackled separately. I do have a question, though:
src/hotspot/share/gc/shared/locationPrinter.inline.hpp line 90:
> 88: if (in_heap) {
> 89: st->print_cr(PTR_FORMAT " is an unknown heap location", p2i(addr));
> 90: return true;
So why not put this block as `else` branch in `base_oop_or_null` check at L67? This would also remove any ambiguity whether the in-heap pointer would look like a compressed pointer to object, which would be accidentally handled by the block at L64..L86?
-------------
PR Review: https://git.openjdk.org/jdk/pull/21870#pullrequestreview-2412881446
PR Review Comment: https://git.openjdk.org/jdk/pull/21870#discussion_r1827621215
More information about the hotspot-gc-dev
mailing list