RFR: 8280601: ClhsdbThreadContext.java test is triggering codecache related assert in PointerFinder.find() [v2]

[Chris Plummer]

On Wed, 26 Jan 2022 14:27:54 GMT, Kevin Walls <kevinw at openjdk.org> wrote:

>> Chris Plummer has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Guard against invalid CodeBlobs that may throw exceptions.
>
> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/PointerLocation.java line 305:
> 
>> 303:       } else {
>> 304:         if (Assert.ASSERTS_ENABLED) {
>> 305:           Assert.that(isInBlobUnknownLocation(), "Should have known location in CodeBlob");
> 
> Line 305's Assert message is hard to understand: "Should have known location in CodeBlob", when it would accept knowing that the location is unknown.
> 
> It means that none of the isInBlobXXX flags are true, and probably impossible to trigger at the moment.
> If you ever wanted to delete it, or make it say "Should have known or unknown location in CodeBlob" then I'd agree. 8-)

Basically it is asserting that if the code we just passed through didn't detect a known location, then PointerFinder better have set the "unknown location" flag. Maybe a better assert would just check all 4 "isXXX" results, and assert that at least one of them is true (with an appropriate message), but then maybe then it should also assert that at most one is true, and now we are down the path of just how valuable this type of assert is in the first place. In any case, it's fairly unimportant code to start with, so remove the assert.

And if you want some light reading on the subject, see https://www.goodreads.com/quotes/4111881-there-are-known-knowns-things-we-know-that-we-know :)

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

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