Fix trampolines when startPC returns error value #7550
Conversation
compiler/runtime/OMRCodeCache.cpp
Outdated
| @@ -587,6 +587,11 @@ OMR::CodeCache::findTrampoline(TR_OpaqueMethodBlock * method) | |||
| { | |||
| void *newPC = (void *) TR::Compiler->mtd.startPC(method); | |||
|
|
|||
| if (reinterpret_cast<intptr_t>(newPC) <= 0) | |||
There was a problem hiding this comment.
this doesn't look right.
if there is no existing trampoline, the _currentStartPC (associated with the trampoline) must be NULL. it didn't mean the method hasn't been compiled though.
There was a problem hiding this comment.
I think you are right. I'll need to take a look at this again.
compiler/runtime/OMRCodeCache.cpp
Outdated
| if (trampoline && entry->_info._resolved._currentStartPC != newPC) | ||
| bool forceCreateTrampoline = false; | ||
|
|
||
| if (reinterpret_cast<intptr_t>(newPC) <= 0) |
There was a problem hiding this comment.
the existence of temp trampoline means _currentStartPC must not be NULL, such that this fix could work. however, <=0 test is not right ... you cannot assume codeCache is not in 0x[8-F]..... areas. Rather, J9TR_MethodNotCompiledBit is defined for the testing ... low tagging really it is. Not sure it has been used in OMR side though.
There was a problem hiding this comment.
I was just thinking that this check might not be right. I will look at what it should be.
|
I talked to @dsouzai and he pointed out to me that the -3 is coming from One problem is that these macros don't exist in OMR. But, this can be dealt with by modifying the OpenJ9 implementation of OMR version: Openj9 version: I changing this PR to WIP for now while I confirm the exact details. |
IBMJimmyk
changed the title
IBMJimmyk
force-pushed
the
redirectTrampoline
branch
from
520a190 to
4dbff42
Compare
|
I changed the code to recognize 0 as the error return value. The OpenJ9 change to make |
IBMJimmyk
force-pushed
the
redirectTrampoline
branch
from
4dbff42 to
936f77c
Compare
When syncing temporary trampolines, there is a call to startPC with the callee method to try and get the start PC. If the latest compilation of that method failed, startPC returns zero which is not a real start PC and can not be used as a target for a trampoline. Attempting to do so will cause an error. This change adds error checking when trying to sync trampolines. When a start PC of zero is returned from the call to startPC, the previous start PC is used instead. Signed-off-by: jimmyk <[email protected]>
IBMJimmyk
changed the title
|
jenkins build all |
|
The failing build didn't get far enough to even compile a build with my changes: |
|
yeah i think riscv failues are a known issue. |
When finding trampolines or syncing temporary trampolines, there is a call to
startPCwith the method to try and get the start PC. If the latest compilation of that method failed,startPCmay return zero or a negative value. Both cases are not a real start PC and can not be used as a target for a trampoline. Attempting to do so will cause an error.This change adds error checking when trying to create or modify a trampoline. When a zero or negative start PC is returned from the call to
startPC, the previous start PC is used instead.Fixes:
eclipse-openj9/openj9#20567
Edit:
Related to:
eclipse-openj9/openj9#20657
They don't need to go in at the same time since this is currently broken and both PRs are needed to fix the issue. Checking in OpenJ9 or OMR first won't make the problem worse.