You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The ip seems to be in epilog. There should not be interruptible points in epilog. That is what could trigger the assert.
I will take a look. Must be some recent change.
The problem is not with the call being right before an epilog, but with detecting epilogs on linux-ARM64 in general.
The #101647 started using LR as a general purpose register when doing indirect calls and that broke our logic that detects whether a thread is in an epilog on linux-arm64
Using LR as a GPR is not illegal, just something that we did not do before, so once we see LR (or FP) loaded with some value, the epilog detection logic assume that we are in an epilog.
Long-term (and assuming that some platforms will never learn how to unwind in epilogs), I think we should encode the epilog ranges, in GC info, I suppose. It will not be trivial as there could be more than one epilog in a method, so encoding them all and then searching through them could be a bit tricky.
However, it will be more portable approach.
For now we can have a simpler fix - we do not have to use LR for indirect calls, In fact REG_INDIRECT_CALL_TARGET_REG seems like a better fit for this anyways.
IIRC, we have been using LR as a general-purpose register for a long time, it just did not happen very often in practice.
We would have seen this assert once in a while then.
I think one of the reasons why #101647 could unconditionally use LR for indirect calls register is that LR is not generally used as GPR. (that is not a case on arm32 though, where registers are in a short supply and LR is used as a scratch)
We need to confirm this until we have a better solution (i.e. #101932)
Not sure if that is sufficient to prevent its use as a GPR. Looks like that is the intent there.
If yes, we may want to add a comment about NativeAOT dependency, while there is a dependency.
I think one of the reasons why #101647 could unconditionally use LR for indirect calls register is that LR is not generally used as GPR.
I picked LR because the upcoming bl is going to unconditionally change it anyway (setting it to the return address), so using it for something is guaranteed to impact nothing (well... that was my thinking). Any other register could potentially be used as an argument register for non-standard calling conventions.
@jakobbotsch@AndyAyersMS - Is the exclusion of LR from availableIntRegs in LSRA sufficient for preventing its use as a GPR?
Yes, that should be sufficient (I verified this as well).
I'm not sure why exactly REG_LR is excluded, but I don't think it's deliberately because of this particular issue. It seems like it was done in dotnet/coreclr#14606 at which point REG_LR was not contained in REG_VAR_ORDER, yet #45135 added it into REG_VAR_ORDER without adding it to availableIntRegs.
ABI seems to permit using LR as a scratch, but I would not be surprised if using LR as GPR could break something. Some asm stub could be forgetting to treat is as a scratch or some tooling issue. It could be just a defensive change while dealing with other bugs.
I will add a comment about the dependency that we know about. We can consider allowing using LR as scratch once #101932 is addressed.
Activity
dotnet-policy-service commentedon May 6, 2024
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.
jkotas commentedon May 6, 2024
@VSadov Could you please take a look? We seem to be hitting it for calls right before epilogs:
Example:
VSadov commentedon May 6, 2024
The ip seems to be in epilog. There should not be interruptible points in epilog. That is what could trigger the assert.
I will take a look. Must be some recent change.
VSadov commentedon May 6, 2024
The problem is not with the call being right before an epilog, but with detecting epilogs on linux-ARM64 in general.
The #101647 started using LR as a general purpose register when doing indirect calls and that broke our logic that detects whether a thread is in an epilog on linux-arm64
Using LR as a GPR is not illegal, just something that we did not do before, so once we see LR (or FP) loaded with some value, the epilog detection logic assume that we are in an epilog.
Long-term (and assuming that some platforms will never learn how to unwind in epilogs), I think we should encode the epilog ranges, in GC info, I suppose. It will not be trivial as there could be more than one epilog in a method, so encoding them all and then searching through them could be a bit tricky.
However, it will be more portable approach.
For now we can have a simpler fix - we do not have to use LR for indirect calls, In fact
REG_INDIRECT_CALL_TARGET_REGseems like a better fit for this anyways.jkotas commentedon May 6, 2024
IIRC, we have been using LR as a general-purpose register for a long time, it just did not happen very often in practice.
VSadov commentedon May 6, 2024
We would have seen this assert once in a while then.
I think one of the reasons why #101647 could unconditionally use LR for indirect calls register is that LR is not generally used as GPR. (that is not a case on arm32 though, where registers are in a short supply and LR is used as a scratch)
We need to confirm this until we have a better solution (i.e. #101932)
VSadov commentedon May 6, 2024
It looks like LSRA excludes LR from availableIntRegs
runtime/src/coreclr/jit/lsra.cpp
Lines 790 to 792 in 91b32f6
Not sure if that is sufficient to prevent its use as a GPR. Looks like that is the intent there.
If yes, we may want to add a comment about NativeAOT dependency, while there is a dependency.
VSadov commentedon May 6, 2024
@jakobbotsch @AndyAyersMS - Is the exclusion of LR from
availableIntRegsin LSRA sufficient for preventing its use as a GPR?(just want to make sure the current assumption is safe)
jakobbotsch commentedon May 6, 2024
I picked
LRbecause the upcomingblis going to unconditionally change it anyway (setting it to the return address), so using it for something is guaranteed to impact nothing (well... that was my thinking). Any other register could potentially be used as an argument register for non-standard calling conventions.Yes, that should be sufficient (I verified this as well).
jakobbotsch commentedon May 6, 2024
I'm not sure why exactly
REG_LRis excluded, but I don't think it's deliberately because of this particular issue. It seems like it was done in dotnet/coreclr#14606 at which pointREG_LRwas not contained inREG_VAR_ORDER, yet #45135 added it intoREG_VAR_ORDERwithout adding it toavailableIntRegs.VSadov commentedon May 6, 2024
ABI seems to permit using LR as a scratch, but I would not be surprised if using LR as GPR could break something. Some asm stub could be forgetting to treat is as a scratch or some tooling issue. It could be just a defensive change while dealing with other bugs.
I will add a comment about the dependency that we know about. We can consider allowing using LR as scratch once #101932 is addressed.
jkotas commentedon May 6, 2024
It would be a bug that we would want to fix. (I have seen it used as a scratch in C/C++ compiled code too.)