[RyuJit] Remove `gtCallAddr` by SingleAccretion · Pull Request #125211 · dotnet/runtime

SingleAccretion · 2026-03-05T11:22:17Z

Call nodes have two fields that serve identical purposes - being the target of a call: gtControlExpr and gtCallAddr. The only difference was that their use was disjoint w.r.t. the call type. This is a complication that does not seem valuable. This change removes gtCallAddr in favor of using gtControlExpr for all types of calls. For indirect calls, it will always be valid, but may be nullptr for helper or user calls (same rules as before).

No diffs. The Linux x64 TP diff is an artifact of less inlining for the changed GenTreeCall::IsHelperCall (should be resolved with updated PGO).

dotnet-policy-service · 2026-03-05T11:23:48Z

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

'gtCallAddr' and `gtControlExpr` serve the same purpose. There is no need to have both.

SingleAccretion · 2026-03-10T19:42:28Z

@dotnet/jit-contrib

kg

Thanks for doing these valuable refactorings! Everything I understand LGTM.
There are a few spots where the transformation isn't an obvious 1:1 but all of those look like cleanups. It might have been easier to review if there were fewer distinct changes batched into this PR but overall it's good!
Will wait for a second pair of eyes though.

SingleAccretion · 2026-03-11T10:53:06Z

There are a few spots where the transformation isn't an obvious 1:1 but all of those look like cleanups. It might have been easier to review if there were fewer distinct changes batched into this PR but overall it's good!

A lot of the volume is in replacing eeGetHelperNum(gtCallMethHnd) with GetHelperNum, which one would think is not related...

Previously, gtCallMethHnd was in a union with gtCallAddr. eeGetHelperNum checks for helpers via & 1. In case of CT_INDIRECT calls, that 'handle' value would actually be a GenTree* value, one aligned to pointer size, so the check would reliably filter out indirect calls as well as user calls. It was still not valid per C++ union rules, but it 'worked'.

Now, since these are no longer in a union, it doesn't quite work the same way (although I think I found all the places where we need to null out the call handle, it is not illegal for it to have an invalid value for an indirect call). GetHelperNum is the reliable replacement to this pattern since it checks the call type directly.

jakobbotsch

Nice cleanup.

… recursive tail call detection (#127293) > [!NOTE] > This PR was generated with the assistance of GitHub Copilot. Fixes#126930 ## Problem PR #125211 removed the union between `gtCallMethHnd` and `gtCallAddr` in `GenTreeCall`, making them separate fields. However, `gtNewCallNode` was not updated to initialize `gtCallMethHnd` for `CT_INDIRECT` calls. Since the JIT arena allocator doesn't zero memory in Release builds, `gtCallMethHnd` contains stale arena data. When the stale data coincidentally matches the compiled method's handle, `gtIsRecursiveCall()` falsely returns true for indirect calls. Combined with `!call->IsVirtual()` being true for `CORINFO_VIRTUALCALL_LDVIRTFTN` calls (which lack `GTF_CALL_VIRT_*` flags), `fgMorphRecursiveFastTailCallIntoLoop` incorrectly transforms the call into a backward jump — creating an infinite allocating loop. This only manifests non-deterministically depending on arena memory layout, which is why it appears on x64 CI but not on arm64 or in local builds. ## Fix Three surgical changes: 1. **`gentree.cpp` (`gtNewCallNode`)**: Initialize `gtCallMethHnd = NO_METHOD_HANDLE` for `CT_INDIRECT` calls — the primary fix that prevents stale data. 2. **`compiler.h` (`gtIsRecursiveCall`)**: Defense-in-depth — return `false` for `CT_INDIRECT` calls since indirect calls can never be recursive by definition. 3. **`gentree.cpp` (`gtCloneExprCallHelper`)**: Same initialization for cloned indirect calls to prevent the same issue in clone paths. Co-authored-by: Andrew Ayers <andya@Andrews-MacBook-Pro.local> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

SingleAccretion · 2026-03-05T11:22:17Z

Call nodes have two fields that serve identical purposes - being the target of a call: gtControlExpr and gtCallAddr. The only difference was that their use was disjoint w.r.t. the call type. This is a complication that does not seem valuable. This change removes gtCallAddr in favor of using gtControlExpr for all types of calls. For indirect calls, it will always be valid, but may be nullptr for helper or user calls (same rules as before).

No diffs. The Linux x64 TP diff is an artifact of less inlining for the changed GenTreeCall::IsHelperCall (should be resolved with updated PGO).

dotnet-policy-service · 2026-03-05T11:23:48Z

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

'gtCallAddr' and `gtControlExpr` serve the same purpose. There is no need to have both.

SingleAccretion · 2026-03-10T19:42:28Z

@dotnet/jit-contrib

kg

Thanks for doing these valuable refactorings! Everything I understand LGTM.
There are a few spots where the transformation isn't an obvious 1:1 but all of those look like cleanups. It might have been easier to review if there were fewer distinct changes batched into this PR but overall it's good!
Will wait for a second pair of eyes though.

SingleAccretion · 2026-03-11T10:53:06Z

There are a few spots where the transformation isn't an obvious 1:1 but all of those look like cleanups. It might have been easier to review if there were fewer distinct changes batched into this PR but overall it's good!

A lot of the volume is in replacing eeGetHelperNum(gtCallMethHnd) with GetHelperNum, which one would think is not related...

Previously, gtCallMethHnd was in a union with gtCallAddr. eeGetHelperNum checks for helpers via & 1. In case of CT_INDIRECT calls, that 'handle' value would actually be a GenTree* value, one aligned to pointer size, so the check would reliably filter out indirect calls as well as user calls. It was still not valid per C++ union rules, but it 'worked'.

Now, since these are no longer in a union, it doesn't quite work the same way (although I think I found all the places where we need to null out the call handle, it is not illegal for it to have an invalid value for an indirect call). GetHelperNum is the reliable replacement to this pattern since it checks the call type directly.

jakobbotsch

Nice cleanup.

… recursive tail call detection (#127293) > [!NOTE] > This PR was generated with the assistance of GitHub Copilot. Fixes#126930 ## Problem PR #125211 removed the union between `gtCallMethHnd` and `gtCallAddr` in `GenTreeCall`, making them separate fields. However, `gtNewCallNode` was not updated to initialize `gtCallMethHnd` for `CT_INDIRECT` calls. Since the JIT arena allocator doesn't zero memory in Release builds, `gtCallMethHnd` contains stale arena data. When the stale data coincidentally matches the compiled method's handle, `gtIsRecursiveCall()` falsely returns true for indirect calls. Combined with `!call->IsVirtual()` being true for `CORINFO_VIRTUALCALL_LDVIRTFTN` calls (which lack `GTF_CALL_VIRT_*` flags), `fgMorphRecursiveFastTailCallIntoLoop` incorrectly transforms the call into a backward jump — creating an infinite allocating loop. This only manifests non-deterministically depending on arena memory layout, which is why it appears on x64 CI but not on arm64 or in local builds. ## Fix Three surgical changes: 1. **`gentree.cpp` (`gtNewCallNode`)**: Initialize `gtCallMethHnd = NO_METHOD_HANDLE` for `CT_INDIRECT` calls — the primary fix that prevents stale data. 2. **`compiler.h` (`gtIsRecursiveCall`)**: Defense-in-depth — return `false` for `CT_INDIRECT` calls since indirect calls can never be recursive by definition. 3. **`gentree.cpp` (`gtCloneExprCallHelper`)**: Same initialization for cloned indirect calls to prevent the same issue in clone paths. Co-authored-by: Andrew Ayers <andya@Andrews-MacBook-Pro.local> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

SingleAccretion · 2026-03-05T11:22:17Z

Call nodes have two fields that serve identical purposes - being the target of a call: gtControlExpr and gtCallAddr. The only difference was that their use was disjoint w.r.t. the call type. This is a complication that does not seem valuable. This change removes gtCallAddr in favor of using gtControlExpr for all types of calls. For indirect calls, it will always be valid, but may be nullptr for helper or user calls (same rules as before).

No diffs. The Linux x64 TP diff is an artifact of less inlining for the changed GenTreeCall::IsHelperCall (should be resolved with updated PGO).

dotnet-policy-service · 2026-03-05T11:23:48Z

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

'gtCallAddr' and `gtControlExpr` serve the same purpose. There is no need to have both.

SingleAccretion · 2026-03-10T19:42:28Z

@dotnet/jit-contrib

kg

Thanks for doing these valuable refactorings! Everything I understand LGTM.
There are a few spots where the transformation isn't an obvious 1:1 but all of those look like cleanups. It might have been easier to review if there were fewer distinct changes batched into this PR but overall it's good!
Will wait for a second pair of eyes though.

SingleAccretion · 2026-03-11T10:53:06Z

There are a few spots where the transformation isn't an obvious 1:1 but all of those look like cleanups. It might have been easier to review if there were fewer distinct changes batched into this PR but overall it's good!

A lot of the volume is in replacing eeGetHelperNum(gtCallMethHnd) with GetHelperNum, which one would think is not related...

Previously, gtCallMethHnd was in a union with gtCallAddr. eeGetHelperNum checks for helpers via & 1. In case of CT_INDIRECT calls, that 'handle' value would actually be a GenTree* value, one aligned to pointer size, so the check would reliably filter out indirect calls as well as user calls. It was still not valid per C++ union rules, but it 'worked'.

Now, since these are no longer in a union, it doesn't quite work the same way (although I think I found all the places where we need to null out the call handle, it is not illegal for it to have an invalid value for an indirect call). GetHelperNum is the reliable replacement to this pattern since it checks the call type directly.

jakobbotsch

Nice cleanup.

… recursive tail call detection (#127293) > [!NOTE] > This PR was generated with the assistance of GitHub Copilot. Fixes#126930 ## Problem PR #125211 removed the union between `gtCallMethHnd` and `gtCallAddr` in `GenTreeCall`, making them separate fields. However, `gtNewCallNode` was not updated to initialize `gtCallMethHnd` for `CT_INDIRECT` calls. Since the JIT arena allocator doesn't zero memory in Release builds, `gtCallMethHnd` contains stale arena data. When the stale data coincidentally matches the compiled method's handle, `gtIsRecursiveCall()` falsely returns true for indirect calls. Combined with `!call->IsVirtual()` being true for `CORINFO_VIRTUALCALL_LDVIRTFTN` calls (which lack `GTF_CALL_VIRT_*` flags), `fgMorphRecursiveFastTailCallIntoLoop` incorrectly transforms the call into a backward jump — creating an infinite allocating loop. This only manifests non-deterministically depending on arena memory layout, which is why it appears on x64 CI but not on arm64 or in local builds. ## Fix Three surgical changes: 1. **`gentree.cpp` (`gtNewCallNode`)**: Initialize `gtCallMethHnd = NO_METHOD_HANDLE` for `CT_INDIRECT` calls — the primary fix that prevents stale data. 2. **`compiler.h` (`gtIsRecursiveCall`)**: Defense-in-depth — return `false` for `CT_INDIRECT` calls since indirect calls can never be recursive by definition. 3. **`gentree.cpp` (`gtCloneExprCallHelper`)**: Same initialization for cloned indirect calls to prevent the same issue in clone paths. Co-authored-by: Andrew Ayers <andya@Andrews-MacBook-Pro.local> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

SingleAccretion · 2026-03-05T11:22:17Z

Call nodes have two fields that serve identical purposes - being the target of a call: gtControlExpr and gtCallAddr. The only difference was that their use was disjoint w.r.t. the call type. This is a complication that does not seem valuable. This change removes gtCallAddr in favor of using gtControlExpr for all types of calls. For indirect calls, it will always be valid, but may be nullptr for helper or user calls (same rules as before).

No diffs. The Linux x64 TP diff is an artifact of less inlining for the changed GenTreeCall::IsHelperCall (should be resolved with updated PGO).

dotnet-policy-service · 2026-03-05T11:23:48Z

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

'gtCallAddr' and `gtControlExpr` serve the same purpose. There is no need to have both.

SingleAccretion · 2026-03-10T19:42:28Z

@dotnet/jit-contrib

kg

Thanks for doing these valuable refactorings! Everything I understand LGTM.
There are a few spots where the transformation isn't an obvious 1:1 but all of those look like cleanups. It might have been easier to review if there were fewer distinct changes batched into this PR but overall it's good!
Will wait for a second pair of eyes though.

SingleAccretion · 2026-03-11T10:53:06Z

There are a few spots where the transformation isn't an obvious 1:1 but all of those look like cleanups. It might have been easier to review if there were fewer distinct changes batched into this PR but overall it's good!

A lot of the volume is in replacing eeGetHelperNum(gtCallMethHnd) with GetHelperNum, which one would think is not related...

Previously, gtCallMethHnd was in a union with gtCallAddr. eeGetHelperNum checks for helpers via & 1. In case of CT_INDIRECT calls, that 'handle' value would actually be a GenTree* value, one aligned to pointer size, so the check would reliably filter out indirect calls as well as user calls. It was still not valid per C++ union rules, but it 'worked'.

Now, since these are no longer in a union, it doesn't quite work the same way (although I think I found all the places where we need to null out the call handle, it is not illegal for it to have an invalid value for an indirect call). GetHelperNum is the reliable replacement to this pattern since it checks the call type directly.

jakobbotsch

Nice cleanup.

… recursive tail call detection (#127293) > [!NOTE] > This PR was generated with the assistance of GitHub Copilot. Fixes#126930 ## Problem PR #125211 removed the union between `gtCallMethHnd` and `gtCallAddr` in `GenTreeCall`, making them separate fields. However, `gtNewCallNode` was not updated to initialize `gtCallMethHnd` for `CT_INDIRECT` calls. Since the JIT arena allocator doesn't zero memory in Release builds, `gtCallMethHnd` contains stale arena data. When the stale data coincidentally matches the compiled method's handle, `gtIsRecursiveCall()` falsely returns true for indirect calls. Combined with `!call->IsVirtual()` being true for `CORINFO_VIRTUALCALL_LDVIRTFTN` calls (which lack `GTF_CALL_VIRT_*` flags), `fgMorphRecursiveFastTailCallIntoLoop` incorrectly transforms the call into a backward jump — creating an infinite allocating loop. This only manifests non-deterministically depending on arena memory layout, which is why it appears on x64 CI but not on arm64 or in local builds. ## Fix Three surgical changes: 1. **`gentree.cpp` (`gtNewCallNode`)**: Initialize `gtCallMethHnd = NO_METHOD_HANDLE` for `CT_INDIRECT` calls — the primary fix that prevents stale data. 2. **`compiler.h` (`gtIsRecursiveCall`)**: Defense-in-depth — return `false` for `CT_INDIRECT` calls since indirect calls can never be recursive by definition. 3. **`gentree.cpp` (`gtCloneExprCallHelper`)**: Same initialization for cloned indirect calls to prevent the same issue in clone paths. Co-authored-by: Andrew Ayers <andya@Andrews-MacBook-Pro.local> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

SingleAccretion · 2026-03-05T11:22:17Z

Call nodes have two fields that serve identical purposes - being the target of a call: gtControlExpr and gtCallAddr. The only difference was that their use was disjoint w.r.t. the call type. This is a complication that does not seem valuable. This change removes gtCallAddr in favor of using gtControlExpr for all types of calls. For indirect calls, it will always be valid, but may be nullptr for helper or user calls (same rules as before).

No diffs. The Linux x64 TP diff is an artifact of less inlining for the changed GenTreeCall::IsHelperCall (should be resolved with updated PGO).

dotnet-policy-service · 2026-03-05T11:23:48Z

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

'gtCallAddr' and `gtControlExpr` serve the same purpose. There is no need to have both.

SingleAccretion · 2026-03-10T19:42:28Z

@dotnet/jit-contrib

kg

Thanks for doing these valuable refactorings! Everything I understand LGTM.
There are a few spots where the transformation isn't an obvious 1:1 but all of those look like cleanups. It might have been easier to review if there were fewer distinct changes batched into this PR but overall it's good!
Will wait for a second pair of eyes though.

SingleAccretion · 2026-03-11T10:53:06Z

There are a few spots where the transformation isn't an obvious 1:1 but all of those look like cleanups. It might have been easier to review if there were fewer distinct changes batched into this PR but overall it's good!

A lot of the volume is in replacing eeGetHelperNum(gtCallMethHnd) with GetHelperNum, which one would think is not related...

Previously, gtCallMethHnd was in a union with gtCallAddr. eeGetHelperNum checks for helpers via & 1. In case of CT_INDIRECT calls, that 'handle' value would actually be a GenTree* value, one aligned to pointer size, so the check would reliably filter out indirect calls as well as user calls. It was still not valid per C++ union rules, but it 'worked'.

Now, since these are no longer in a union, it doesn't quite work the same way (although I think I found all the places where we need to null out the call handle, it is not illegal for it to have an invalid value for an indirect call). GetHelperNum is the reliable replacement to this pattern since it checks the call type directly.

jakobbotsch

Nice cleanup.

… recursive tail call detection (#127293) > [!NOTE] > This PR was generated with the assistance of GitHub Copilot. Fixes#126930 ## Problem PR #125211 removed the union between `gtCallMethHnd` and `gtCallAddr` in `GenTreeCall`, making them separate fields. However, `gtNewCallNode` was not updated to initialize `gtCallMethHnd` for `CT_INDIRECT` calls. Since the JIT arena allocator doesn't zero memory in Release builds, `gtCallMethHnd` contains stale arena data. When the stale data coincidentally matches the compiled method's handle, `gtIsRecursiveCall()` falsely returns true for indirect calls. Combined with `!call->IsVirtual()` being true for `CORINFO_VIRTUALCALL_LDVIRTFTN` calls (which lack `GTF_CALL_VIRT_*` flags), `fgMorphRecursiveFastTailCallIntoLoop` incorrectly transforms the call into a backward jump — creating an infinite allocating loop. This only manifests non-deterministically depending on arena memory layout, which is why it appears on x64 CI but not on arm64 or in local builds. ## Fix Three surgical changes: 1. **`gentree.cpp` (`gtNewCallNode`)**: Initialize `gtCallMethHnd = NO_METHOD_HANDLE` for `CT_INDIRECT` calls — the primary fix that prevents stale data. 2. **`compiler.h` (`gtIsRecursiveCall`)**: Defense-in-depth — return `false` for `CT_INDIRECT` calls since indirect calls can never be recursive by definition. 3. **`gentree.cpp` (`gtCloneExprCallHelper`)**: Same initialization for cloned indirect calls to prevent the same issue in clone paths. Co-authored-by: Andrew Ayers <andya@Andrews-MacBook-Pro.local> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

SingleAccretion · 2026-03-05T11:22:17Z

Call nodes have two fields that serve identical purposes - being the target of a call: gtControlExpr and gtCallAddr. The only difference was that their use was disjoint w.r.t. the call type. This is a complication that does not seem valuable. This change removes gtCallAddr in favor of using gtControlExpr for all types of calls. For indirect calls, it will always be valid, but may be nullptr for helper or user calls (same rules as before).

No diffs. The Linux x64 TP diff is an artifact of less inlining for the changed GenTreeCall::IsHelperCall (should be resolved with updated PGO).

dotnet-policy-service · 2026-03-05T11:23:48Z

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

'gtCallAddr' and `gtControlExpr` serve the same purpose. There is no need to have both.

SingleAccretion · 2026-03-10T19:42:28Z

@dotnet/jit-contrib

kg

Thanks for doing these valuable refactorings! Everything I understand LGTM.
There are a few spots where the transformation isn't an obvious 1:1 but all of those look like cleanups. It might have been easier to review if there were fewer distinct changes batched into this PR but overall it's good!
Will wait for a second pair of eyes though.

SingleAccretion · 2026-03-11T10:53:06Z

There are a few spots where the transformation isn't an obvious 1:1 but all of those look like cleanups. It might have been easier to review if there were fewer distinct changes batched into this PR but overall it's good!

A lot of the volume is in replacing eeGetHelperNum(gtCallMethHnd) with GetHelperNum, which one would think is not related...

Previously, gtCallMethHnd was in a union with gtCallAddr. eeGetHelperNum checks for helpers via & 1. In case of CT_INDIRECT calls, that 'handle' value would actually be a GenTree* value, one aligned to pointer size, so the check would reliably filter out indirect calls as well as user calls. It was still not valid per C++ union rules, but it 'worked'.

Now, since these are no longer in a union, it doesn't quite work the same way (although I think I found all the places where we need to null out the call handle, it is not illegal for it to have an invalid value for an indirect call). GetHelperNum is the reliable replacement to this pattern since it checks the call type directly.

jakobbotsch

Nice cleanup.

… recursive tail call detection (#127293) > [!NOTE] > This PR was generated with the assistance of GitHub Copilot. Fixes#126930 ## Problem PR #125211 removed the union between `gtCallMethHnd` and `gtCallAddr` in `GenTreeCall`, making them separate fields. However, `gtNewCallNode` was not updated to initialize `gtCallMethHnd` for `CT_INDIRECT` calls. Since the JIT arena allocator doesn't zero memory in Release builds, `gtCallMethHnd` contains stale arena data. When the stale data coincidentally matches the compiled method's handle, `gtIsRecursiveCall()` falsely returns true for indirect calls. Combined with `!call->IsVirtual()` being true for `CORINFO_VIRTUALCALL_LDVIRTFTN` calls (which lack `GTF_CALL_VIRT_*` flags), `fgMorphRecursiveFastTailCallIntoLoop` incorrectly transforms the call into a backward jump — creating an infinite allocating loop. This only manifests non-deterministically depending on arena memory layout, which is why it appears on x64 CI but not on arm64 or in local builds. ## Fix Three surgical changes: 1. **`gentree.cpp` (`gtNewCallNode`)**: Initialize `gtCallMethHnd = NO_METHOD_HANDLE` for `CT_INDIRECT` calls — the primary fix that prevents stale data. 2. **`compiler.h` (`gtIsRecursiveCall`)**: Defense-in-depth — return `false` for `CT_INDIRECT` calls since indirect calls can never be recursive by definition. 3. **`gentree.cpp` (`gtCloneExprCallHelper`)**: Same initialization for cloned indirect calls to prevent the same issue in clone paths. Co-authored-by: Andrew Ayers <andya@Andrews-MacBook-Pro.local> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

SingleAccretion · 2026-03-05T11:22:17Z

Call nodes have two fields that serve identical purposes - being the target of a call: gtControlExpr and gtCallAddr. The only difference was that their use was disjoint w.r.t. the call type. This is a complication that does not seem valuable. This change removes gtCallAddr in favor of using gtControlExpr for all types of calls. For indirect calls, it will always be valid, but may be nullptr for helper or user calls (same rules as before).

No diffs. The Linux x64 TP diff is an artifact of less inlining for the changed GenTreeCall::IsHelperCall (should be resolved with updated PGO).

dotnet-policy-service · 2026-03-05T11:23:48Z

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

'gtCallAddr' and `gtControlExpr` serve the same purpose. There is no need to have both.

SingleAccretion · 2026-03-10T19:42:28Z

@dotnet/jit-contrib

kg

Thanks for doing these valuable refactorings! Everything I understand LGTM.
There are a few spots where the transformation isn't an obvious 1:1 but all of those look like cleanups. It might have been easier to review if there were fewer distinct changes batched into this PR but overall it's good!
Will wait for a second pair of eyes though.

SingleAccretion · 2026-03-11T10:53:06Z

There are a few spots where the transformation isn't an obvious 1:1 but all of those look like cleanups. It might have been easier to review if there were fewer distinct changes batched into this PR but overall it's good!

A lot of the volume is in replacing eeGetHelperNum(gtCallMethHnd) with GetHelperNum, which one would think is not related...

Previously, gtCallMethHnd was in a union with gtCallAddr. eeGetHelperNum checks for helpers via & 1. In case of CT_INDIRECT calls, that 'handle' value would actually be a GenTree* value, one aligned to pointer size, so the check would reliably filter out indirect calls as well as user calls. It was still not valid per C++ union rules, but it 'worked'.

Now, since these are no longer in a union, it doesn't quite work the same way (although I think I found all the places where we need to null out the call handle, it is not illegal for it to have an invalid value for an indirect call). GetHelperNum is the reliable replacement to this pattern since it checks the call type directly.

jakobbotsch

Nice cleanup.

… recursive tail call detection (#127293) > [!NOTE] > This PR was generated with the assistance of GitHub Copilot. Fixes#126930 ## Problem PR #125211 removed the union between `gtCallMethHnd` and `gtCallAddr` in `GenTreeCall`, making them separate fields. However, `gtNewCallNode` was not updated to initialize `gtCallMethHnd` for `CT_INDIRECT` calls. Since the JIT arena allocator doesn't zero memory in Release builds, `gtCallMethHnd` contains stale arena data. When the stale data coincidentally matches the compiled method's handle, `gtIsRecursiveCall()` falsely returns true for indirect calls. Combined with `!call->IsVirtual()` being true for `CORINFO_VIRTUALCALL_LDVIRTFTN` calls (which lack `GTF_CALL_VIRT_*` flags), `fgMorphRecursiveFastTailCallIntoLoop` incorrectly transforms the call into a backward jump — creating an infinite allocating loop. This only manifests non-deterministically depending on arena memory layout, which is why it appears on x64 CI but not on arm64 or in local builds. ## Fix Three surgical changes: 1. **`gentree.cpp` (`gtNewCallNode`)**: Initialize `gtCallMethHnd = NO_METHOD_HANDLE` for `CT_INDIRECT` calls — the primary fix that prevents stale data. 2. **`compiler.h` (`gtIsRecursiveCall`)**: Defense-in-depth — return `false` for `CT_INDIRECT` calls since indirect calls can never be recursive by definition. 3. **`gentree.cpp` (`gtCloneExprCallHelper`)**: Same initialization for cloned indirect calls to prevent the same issue in clone paths. Co-authored-by: Andrew Ayers <andya@Andrews-MacBook-Pro.local> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

SingleAccretion · 2026-03-05T11:22:17Z

Call nodes have two fields that serve identical purposes - being the target of a call: gtControlExpr and gtCallAddr. The only difference was that their use was disjoint w.r.t. the call type. This is a complication that does not seem valuable. This change removes gtCallAddr in favor of using gtControlExpr for all types of calls. For indirect calls, it will always be valid, but may be nullptr for helper or user calls (same rules as before).

No diffs. The Linux x64 TP diff is an artifact of less inlining for the changed GenTreeCall::IsHelperCall (should be resolved with updated PGO).

dotnet-policy-service · 2026-03-05T11:23:48Z

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

'gtCallAddr' and `gtControlExpr` serve the same purpose. There is no need to have both.

SingleAccretion · 2026-03-10T19:42:28Z

@dotnet/jit-contrib

kg

Thanks for doing these valuable refactorings! Everything I understand LGTM.
There are a few spots where the transformation isn't an obvious 1:1 but all of those look like cleanups. It might have been easier to review if there were fewer distinct changes batched into this PR but overall it's good!
Will wait for a second pair of eyes though.

SingleAccretion · 2026-03-11T10:53:06Z

There are a few spots where the transformation isn't an obvious 1:1 but all of those look like cleanups. It might have been easier to review if there were fewer distinct changes batched into this PR but overall it's good!

A lot of the volume is in replacing eeGetHelperNum(gtCallMethHnd) with GetHelperNum, which one would think is not related...

Previously, gtCallMethHnd was in a union with gtCallAddr. eeGetHelperNum checks for helpers via & 1. In case of CT_INDIRECT calls, that 'handle' value would actually be a GenTree* value, one aligned to pointer size, so the check would reliably filter out indirect calls as well as user calls. It was still not valid per C++ union rules, but it 'worked'.

Now, since these are no longer in a union, it doesn't quite work the same way (although I think I found all the places where we need to null out the call handle, it is not illegal for it to have an invalid value for an indirect call). GetHelperNum is the reliable replacement to this pattern since it checks the call type directly.

jakobbotsch

Nice cleanup.

… recursive tail call detection (#127293) > [!NOTE] > This PR was generated with the assistance of GitHub Copilot. Fixes#126930 ## Problem PR #125211 removed the union between `gtCallMethHnd` and `gtCallAddr` in `GenTreeCall`, making them separate fields. However, `gtNewCallNode` was not updated to initialize `gtCallMethHnd` for `CT_INDIRECT` calls. Since the JIT arena allocator doesn't zero memory in Release builds, `gtCallMethHnd` contains stale arena data. When the stale data coincidentally matches the compiled method's handle, `gtIsRecursiveCall()` falsely returns true for indirect calls. Combined with `!call->IsVirtual()` being true for `CORINFO_VIRTUALCALL_LDVIRTFTN` calls (which lack `GTF_CALL_VIRT_*` flags), `fgMorphRecursiveFastTailCallIntoLoop` incorrectly transforms the call into a backward jump — creating an infinite allocating loop. This only manifests non-deterministically depending on arena memory layout, which is why it appears on x64 CI but not on arm64 or in local builds. ## Fix Three surgical changes: 1. **`gentree.cpp` (`gtNewCallNode`)**: Initialize `gtCallMethHnd = NO_METHOD_HANDLE` for `CT_INDIRECT` calls — the primary fix that prevents stale data. 2. **`compiler.h` (`gtIsRecursiveCall`)**: Defense-in-depth — return `false` for `CT_INDIRECT` calls since indirect calls can never be recursive by definition. 3. **`gentree.cpp` (`gtCloneExprCallHelper`)**: Same initialization for cloned indirect calls to prevent the same issue in clone paths. Co-authored-by: Andrew Ayers <andya@Andrews-MacBook-Pro.local> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

SingleAccretion changed the title ~~[RyuJit] Remove 'gtCallAddr'~~[RyuJit] Remove gtCallAddrMar 5, 2026

github-actionsBot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 5, 2026

dotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Mar 5, 2026

SingleAccretion force-pushed the gtCallAddr branch 2 times, most recently from e4f897c to c904dacCompare March 5, 2026 14:34

This was referenced Mar 5, 2026
[android] Android.Device_Emulator.JIT.Test failing on emulators with CoreCLR #112633
Open
[Android][CoreCLR] System.Security.Cryptography.Tests killed by lowmemorykiller #118603
Open

SingleAccretion force-pushed the gtCallAddr branch from c904dac to 66d65caCompare March 5, 2026 19:53

This was referenced Mar 6, 2026
The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008
Open
[android-arm64] The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#6408
Closed

SingleAccretion force-pushed the gtCallAddr branch from 66d65ca to e77be20Compare March 6, 2026 11:20

SingleAccretion force-pushed the gtCallAddr branch from e77be20 to f345512Compare March 10, 2026 19:22

Remove 'gtCallAddr'
8c27adf
'gtCallAddr' and `gtControlExpr` serve the same purpose. There is no need to have both.

SingleAccretion force-pushed the gtCallAddr branch from f345512 to 8c27adfCompare March 10, 2026 19:24

SingleAccretion marked this pull request as ready for review March 10, 2026 19:42

SingleAccretion closed this Mar 10, 2026

SingleAccretion reopened this Mar 10, 2026

build-analysisBot mentioned this pull request Mar 10, 2026
Test failure: Assertion failed. state is RuntimeAsyncTask<T> #125015
Closed

kg reviewed Mar 11, 2026
View reviewed changes

Comment threadsrc/coreclr/jit/gentree.cpp

kg approved these changes Mar 11, 2026
View reviewed changes

Merge branch 'main' into gtCallAddr
4233acf

jakobbotsch approved these changes Mar 11, 2026
View reviewed changes

JulieLeeMSFT merged commit 05596e3 into dotnet:mainApr 13, 2026
136 checks passed

dotnet-maestroBot mentioned this pull request Apr 15, 2026
[main] Source code updates from dotnet/runtime dotnet/dotnet#6035
Merged

svick mentioned this pull request Apr 21, 2026
Test failure: Microsoft.Extensions.Hosting.Tests.HostBuilderTests.ScopeValidationtEnabledInDevelopmentWithServiceProviderChanges #126930
Closed

AndyAyersMS mentioned this pull request Apr 22, 2026
JIT: Fix uninitialized gtCallMethHnd for indirect calls causing false recursive tail call detection #127293
Merged

github-actionsBot locked and limited conversation to collaborators May 14, 2026