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

[Aleksey Shipilev]

On Mon, 4 Nov 2024 14:34:28 GMT, Volker Simonis <simonis at openjdk.org> wrote:

>> 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?
>
> That was actually the first thing I did. But then I thought that (especially with zero-based compressed oops) we might get quite some valid compressed oops pointers unnecessarily printed as "unknown heap location".
> On the other hand, I don't think that there's a high probability for a real invalid heap pointer to be classified as compressed oops pointer because the compressed oops detection code uses `is_valid_obj()` anyway.
> So this change is conservative in the sense that it doesn't change any behavior except that pointers which have been printed as pointing "into unknown readable memory" can now be detect as "invalid heap pointers".
> 
> If you still think we should prioritize the detection steps differently, please let me know.

Oh, OK. Compressed pointers make this whole thing a bit messy. I think current code is not handling the case of compressed interior pointers all that well; IDK if we even have those in Hotspot. 

I think there is an ambiguity between compressed pointers and regular pointers at this level, which we cannot reasonably resolve. E.g. if we have zero-based compressed oops with 2-bit shift and 16 GB heap, passing `0x1000000` as the `addr` here cannot distinguish between cases of "regular pointer, points to `0x1000000`" and "compressed pointer, decodes as `0x4000000`". I guess we would like to print both interpretations. But this is way beyond the scope for this PR.

This version would do meanwhile.

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

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