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

[Dean Long]

On Wed, 10 Jul 2024 15:19:49 GMT, Evgeny Astigeevich <eastigeevich at openjdk.org> wrote:

>>> So for Leyden it sounds like you need to change is_always_within_branch_range().
>> 
>> Or perhaps just adapt MacroAssembler::far_branches(). It returns false if the code cache max range exceeds branch_range. In Leyden we can make it return false when we are generating AOT code.
>
>> > So for Leyden it sounds like you need to change is_always_within_branch_range().
>> 
>> Or perhaps just adapt MacroAssembler::far_branches(). It returns false if the code cache max range exceeds branch_range. In Leyden we can make it return false when we are generating AOT code.
> 
> We might need to adapt [`target_needs_far_branch`](https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp#L677) for AOT code generation.

@eastig, your understanding is correct.

>IMO we should fix CodeBuffer::expand. It should go through relocations and fix only those which are not trampolines.

That's roughly what this patch does.  I detect expand by checking dest->blob() and orig_addr.  However, I don't see an easy way to detect trampoline vs non trampoline calls in the shared code iterator.  Instead, I removed the fast-path trampoline lookup during expand and find the trampoline call-sites by iterating their stubs to find owners.

> We also should somehow guard CodeBuffer::relocate_code_to that it can only work with finalized CodeBuffers.

It is used by expand().  But maybe you meant copy_code_to().  I would like to keep additional changes to a minimum, to make back-ports easier.  I suggest a separate RFE for further improvements.

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

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