On Wed, 26 Jan 2022 14:27:54 GMT, Kevin Walls <kev...@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

Reply via email to