RFR: 8321509: False positive in get_trampoline fast path causes crash [v3]

[Andrew Dinn]

On Wed, 3 Jul 2024 11:14:28 GMT, Andrew Dinn <adinn at openjdk.org> wrote:

>> Dean Long has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   cleanup
>
>> To fix Leyden premain, I'd suggest to change the "nmethod expected" assert in NativeCall::destination() into conditional code that returns the "raw" destination if it is not an nmethod . . .
>> But I don't have a strong opinion on whether it should be fixed here or only in Leyden.
> 
> Yes, I agree that will work as a solution.
> 
> I would recommend making this change in main. I think it is reasonable to expect NativeCall::destination() to be able to access the target for any instruction that can be viewed as a NativeCall, irrespective of whether it is embedded in an nmethod or some other blob. Clearly, the assert confirms that the current mainline code does not use it for anything other than an nmethod but there is nothing to say that it has to remain that way. Leyden is just one potential case where we might want to use it for some other blob.

> @adinn is right. I thought that it mostly affect code during codeBlob expansion but it is not.

@vnkozlov I was more right than I even realized! I was only concerned about generated stubs but, as you point out, we will also call NativeCall::destination() when processing a native call from JITted method code while it is still residing in a CodeBuffer.

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

PR Comment: https://git.openjdk.org/jdk/pull/19796#issuecomment-2205838686