From 50c3a262c95b970397809f063e2c76a324a93bb9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 6 Nov 2025 23:49:01 +0000 Subject: [PATCH 1/5] Initial plan From 15dba36df55266ba46b8f0e2f81585923bce71f3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 7 Nov 2025 00:21:50 +0000 Subject: [PATCH 2/5] Introduce CodeDelta abstraction for ARM thumb bit handling - Add CodeDelta addend to all EmitReloc/EmitPointerReloc calls targeting IMethodNode - Add overload to ARMEmitter.EmitJMP to accept addend parameter - Remove thumb bit handling from ObjectWriter symbol definitions - Remove duplicate thumb bit masking logic from EmitOrResolveRelocation This standardizes thumb bit handling by using factory.Target.CodeDelta in relocations instead of manually adding it to symbol values in ObjectWriter. Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com> --- .../Target_ARM/ARMEmitter.cs | 8 +++++- .../Compiler/ObjectWriter/ObjectWriter.cs | 28 ++----------------- .../Compiler/DependencyAnalysis/EETypeNode.cs | 4 +-- .../FatFunctionPointerNode.cs | 2 +- .../ModuleInitializerListNode.cs | 4 +-- .../DependencyAnalysis/NonGCStaticsNode.cs | 2 +- .../StackTraceMethodMappingNode.cs | 2 +- .../Compiler/TypePreinit.cs | 2 +- 8 files changed, 18 insertions(+), 34 deletions(-) diff --git a/src/coreclr/tools/Common/Compiler/DependencyAnalysis/Target_ARM/ARMEmitter.cs b/src/coreclr/tools/Common/Compiler/DependencyAnalysis/Target_ARM/ARMEmitter.cs index adc1ef8d3cb9a3..33fd589ebef706 100644 --- a/src/coreclr/tools/Common/Compiler/DependencyAnalysis/Target_ARM/ARMEmitter.cs +++ b/src/coreclr/tools/Common/Compiler/DependencyAnalysis/Target_ARM/ARMEmitter.cs @@ -193,9 +193,15 @@ public void EmitMOV(Register destination, ISymbolNode symbol) // b.w symbol public void EmitJMP(ISymbolNode symbol) + { + EmitJMP(symbol, 0); + } + + // b.w symbol with addend + public void EmitJMP(ISymbolNode symbol, int addend) { Debug.Assert(!symbol.RepresentsIndirectionCell); - Builder.EmitReloc(symbol, RelocType.IMAGE_REL_BASED_THUMB_BRANCH24); + Builder.EmitReloc(symbol, RelocType.IMAGE_REL_BASED_THUMB_BRANCH24, addend); Builder.EmitByte(0); Builder.EmitByte(0xF0); Builder.EmitByte(0); diff --git a/src/coreclr/tools/Common/Compiler/ObjectWriter/ObjectWriter.cs b/src/coreclr/tools/Common/Compiler/ObjectWriter/ObjectWriter.cs index 6cf90bb47ccb97..984e814c451e5b 100644 --- a/src/coreclr/tools/Common/Compiler/ObjectWriter/ObjectWriter.cs +++ b/src/coreclr/tools/Common/Compiler/ObjectWriter/ObjectWriter.cs @@ -152,20 +152,6 @@ or IMAGE_REL_BASED_THUMB_BRANCH24 or IMAGE_REL_BASED_THUMB_MOV32_PCREL && // Resolve the relocation to already defined symbol and write it into data fixed (byte *pData = data) { - // RyuJIT generates the Thumb bit in the addend and we also get it from - // the symbol value. The AAELF ABI specification defines the R_ARM_THM_JUMP24 - // and R_ARM_THM_MOVW_PREL_NC relocations using the formula ((S + A) | T) – P. - // The thumb bit is thus supposed to be only added once. - // For R_ARM_THM_JUMP24 the thumb bit cannot be encoded, so mask it out. - // - // R2R doesn't use add the thumb bit to the symbol value, so we don't need to do this here. -#if !READYTORUN - long maskThumbBitOut = relocType is IMAGE_REL_BASED_THUMB_BRANCH24 or IMAGE_REL_BASED_THUMB_MOV32_PCREL ? 1 : 0; - long maskThumbBitIn = relocType is IMAGE_REL_BASED_THUMB_MOV32_PCREL ? 1 : 0; -#else - long maskThumbBitOut = 0; - long maskThumbBitIn = 0; -#endif long adjustedAddend = addend; adjustedAddend -= relocType switch @@ -176,9 +162,8 @@ or IMAGE_REL_BASED_THUMB_BRANCH24 or IMAGE_REL_BASED_THUMB_MOV32_PCREL && _ => 0 }; - adjustedAddend += definedSymbol.Value & ~maskThumbBitOut; + adjustedAddend += definedSymbol.Value; adjustedAddend += Relocation.ReadValue(relocType, (void*)pData); - adjustedAddend |= definedSymbol.Value & maskThumbBitIn; adjustedAddend -= offset; if (relocType is IMAGE_REL_BASED_THUMB_BRANCH24 && !Relocation.FitsInThumb2BlRel24((int)adjustedAddend)) @@ -385,20 +370,13 @@ public void EmitObject(Stream outputFileStream, IReadOnlyCollection Date: Thu, 6 Nov 2025 16:49:18 -0800 Subject: [PATCH 3/5] Restore recordSize = true for R2R --- .../tools/Common/Compiler/ObjectWriter/ObjectWriter.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/coreclr/tools/Common/Compiler/ObjectWriter/ObjectWriter.cs b/src/coreclr/tools/Common/Compiler/ObjectWriter/ObjectWriter.cs index 984e814c451e5b..a9ef9d7cd02b12 100644 --- a/src/coreclr/tools/Common/Compiler/ObjectWriter/ObjectWriter.cs +++ b/src/coreclr/tools/Common/Compiler/ObjectWriter/ObjectWriter.cs @@ -370,7 +370,11 @@ public void EmitObject(Stream outputFileStream, IReadOnlyCollection Date: Fri, 7 Nov 2025 19:42:39 +0000 Subject: [PATCH 4/5] Update ARM EmitJMP calls to pass CodeDelta addend Add factory.Target.CodeDelta as addend parameter to all EmitJMP calls in ARM-specific files that jump to IMethodNode or AssemblyStubNode targets: - ARMUnboxingStubNode - ARMTentativeMethodNode - ARMJumpStubNode (with type check) - ARMReadyToRunHelperNode (5 locations) - ARMReadyToRunGenericHelperNode (5 locations) This ensures the thumb bit is properly encoded in the relocation addend rather than in the symbol value, fixing invalid instruction errors on ARM. Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com> --- .../DependencyAnalysis/Target_ARM/ARMJumpStubNode.cs | 3 ++- .../Target_ARM/ARMReadyToRunGenericHelperNode.cs | 10 +++++----- .../Target_ARM/ARMReadyToRunHelperNode.cs | 12 ++++++------ .../Target_ARM/ARMTentativeMethodNode.cs | 2 +- .../Target_ARM/ARMUnboxingStubNode.cs | 2 +- 5 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMJumpStubNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMJumpStubNode.cs index 3890dc5498eaaf..39fedd5c8090bc 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMJumpStubNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMJumpStubNode.cs @@ -11,7 +11,8 @@ protected override void EmitCode(NodeFactory factory, ref ARMEmitter encoder, bo { if (!_target.RepresentsIndirectionCell) { - encoder.EmitJMP(_target); // b methodEntryPoint + int delta = (_target is IMethodNode || _target is AssemblyStubNode) ? factory.Target.CodeDelta : 0; + encoder.EmitJMP(_target, delta); // b methodEntryPoint } else { diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMReadyToRunGenericHelperNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMReadyToRunGenericHelperNode.cs index 9a3a50d5f55612..846069ed592bd7 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMReadyToRunGenericHelperNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMReadyToRunGenericHelperNode.cs @@ -49,7 +49,7 @@ protected void EmitDictionaryLookup(NodeFactory factory, ref ARMEmitter encoder, { encoder.EmitCMP(result, 0); encoder.EmitRETIfNotEqual(); - encoder.EmitJMP(GetBadSlotHelper(factory)); + encoder.EmitJMP(GetBadSlotHelper(factory), factory.Target.CodeDelta); } } @@ -83,7 +83,7 @@ protected sealed override void EmitCode(NodeFactory factory, ref ARMEmitter enco encoder.EmitMOV(encoder.TargetRegister.Arg1, encoder.TargetRegister.Result); encoder.EmitSUB(encoder.TargetRegister.Arg0, (byte)cctorContextSize); - encoder.EmitJMP(factory.HelperEntrypoint(HelperEntrypoint.EnsureClassConstructorRunAndReturnNonGCStaticBase)); + encoder.EmitJMP(factory.HelperEntrypoint(HelperEntrypoint.EnsureClassConstructorRunAndReturnNonGCStaticBase), factory.Target.CodeDelta); } } break; @@ -116,7 +116,7 @@ protected sealed override void EmitCode(NodeFactory factory, ref ARMEmitter enco encoder.EmitMOV(encoder.TargetRegister.Arg1, encoder.TargetRegister.Result); encoder.EmitMOV(encoder.TargetRegister.Arg0, encoder.TargetRegister.Arg2); encoder.EmitSUB(encoder.TargetRegister.Arg0, cctorContextSize); - encoder.EmitJMP(factory.HelperEntrypoint(HelperEntrypoint.EnsureClassConstructorRunAndReturnGCStaticBase)); + encoder.EmitJMP(factory.HelperEntrypoint(HelperEntrypoint.EnsureClassConstructorRunAndReturnGCStaticBase), factory.Target.CodeDelta); } } break; @@ -155,7 +155,7 @@ protected sealed override void EmitCode(NodeFactory factory, ref ARMEmitter enco // Second arg: index of the type in the ThreadStatic section of the modules encoder.EmitLDR(encoder.TargetRegister.Arg1, encoder.TargetRegister.Arg1, factory.Target.PointerSize); - encoder.EmitJMP(helperEntrypoint); + encoder.EmitJMP(helperEntrypoint, factory.Target.CodeDelta); } break; @@ -182,7 +182,7 @@ protected sealed override void EmitCode(NodeFactory factory, ref ARMEmitter enco Debug.Assert(target.Constructor.Method.Signature.Length == 2); } - encoder.EmitJMP(target.Constructor); + encoder.EmitJMP(target.Constructor, factory.Target.CodeDelta); } break; diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMReadyToRunHelperNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMReadyToRunHelperNode.cs index b7db3ab474e48c..1a3bf0850249a9 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMReadyToRunHelperNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMReadyToRunHelperNode.cs @@ -33,7 +33,7 @@ protected override void EmitCode(NodeFactory factory, ref ARMEmitter encoder, bo encoder.EmitMOV(encoder.TargetRegister.Arg1, factory.TypeNonGCStaticsSymbol(target)); encoder.EmitMOV(encoder.TargetRegister.Arg0/*Result*/, encoder.TargetRegister.Arg1); encoder.EmitSUB(encoder.TargetRegister.Arg0, NonGCStaticsNode.GetClassConstructorContextSize(factory.Target)); - encoder.EmitJMP(factory.HelperEntrypoint(HelperEntrypoint.EnsureClassConstructorRunAndReturnNonGCStaticBase)); + encoder.EmitJMP(factory.HelperEntrypoint(HelperEntrypoint.EnsureClassConstructorRunAndReturnNonGCStaticBase), factory.Target.CodeDelta); } } break; @@ -53,14 +53,14 @@ protected override void EmitCode(NodeFactory factory, ref ARMEmitter encoder, bo if (!factory.PreinitializationManager.HasLazyStaticConstructor(target)) { - encoder.EmitJMP(factory.HelperEntrypoint(HelperEntrypoint.GetThreadStaticBaseForType)); + encoder.EmitJMP(factory.HelperEntrypoint(HelperEntrypoint.GetThreadStaticBaseForType), factory.Target.CodeDelta); } else { encoder.EmitMOV(encoder.TargetRegister.Arg2, factory.TypeNonGCStaticsSymbol(target)); encoder.EmitSUB(encoder.TargetRegister.Arg2, NonGCStaticsNode.GetClassConstructorContextSize(factory.Target)); // TODO: performance optimization - inline the check verifying whether we need to trigger the cctor - encoder.EmitJMP(factory.HelperEntrypoint(HelperEntrypoint.EnsureClassConstructorRunAndReturnThreadStaticBase)); + encoder.EmitJMP(factory.HelperEntrypoint(HelperEntrypoint.EnsureClassConstructorRunAndReturnThreadStaticBase), factory.Target.CodeDelta); } } break; @@ -80,7 +80,7 @@ protected override void EmitCode(NodeFactory factory, ref ARMEmitter encoder, bo encoder.EmitLDR(encoder.TargetRegister.Arg1, encoder.TargetRegister.Result); encoder.EmitMOV(encoder.TargetRegister.Arg0, factory.TypeNonGCStaticsSymbol(target)); encoder.EmitSUB(encoder.TargetRegister.Arg0, NonGCStaticsNode.GetClassConstructorContextSize(factory.Target)); - encoder.EmitJMP(factory.HelperEntrypoint(HelperEntrypoint.EnsureClassConstructorRunAndReturnGCStaticBase)); + encoder.EmitJMP(factory.HelperEntrypoint(HelperEntrypoint.EnsureClassConstructorRunAndReturnGCStaticBase), factory.Target.CodeDelta); } } break; @@ -118,7 +118,7 @@ protected override void EmitCode(NodeFactory factory, ref ARMEmitter encoder, bo Debug.Assert(target.Constructor.Method.Signature.Length == 2); } - encoder.EmitJMP(target.Constructor); + encoder.EmitJMP(target.Constructor, factory.Target.CodeDelta); } break; @@ -128,7 +128,7 @@ protected override void EmitCode(NodeFactory factory, ref ARMEmitter encoder, bo if (targetMethod.OwningType.IsInterface) { encoder.EmitMOV(encoder.TargetRegister.Arg1, factory.InterfaceDispatchCell(targetMethod)); - encoder.EmitJMP(factory.ExternFunctionSymbol("RhpResolveInterfaceMethod")); + encoder.EmitJMP(factory.ExternFunctionSymbol("RhpResolveInterfaceMethod"), factory.Target.CodeDelta); } else { diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMTentativeMethodNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMTentativeMethodNode.cs index 982a444c46a123..60125c68feb082 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMTentativeMethodNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMTentativeMethodNode.cs @@ -9,7 +9,7 @@ public partial class TentativeMethodNode { protected override void EmitCode(NodeFactory factory, ref ARMEmitter encoder, bool relocsOnly) { - encoder.EmitJMP(GetTarget(factory)); + encoder.EmitJMP(GetTarget(factory), factory.Target.CodeDelta); } } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMUnboxingStubNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMUnboxingStubNode.cs index 072121dfc635d1..fe6772afb5e996 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMUnboxingStubNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMUnboxingStubNode.cs @@ -10,7 +10,7 @@ public partial class UnboxingStubNode protected override void EmitCode(NodeFactory factory, ref ARMEmitter encoder, bool relocsOnly) { encoder.EmitADD(encoder.TargetRegister.Arg0, (byte)factory.Target.PointerSize); // add r0, sizeof(void*); - encoder.EmitJMP(GetUnderlyingMethodEntrypoint(factory)); // b methodEntryPoint + encoder.EmitJMP(GetUnderlyingMethodEntrypoint(factory), factory.Target.CodeDelta); // b methodEntryPoint } } } From 98dcfdd84d90e925d25949106f4bc7a39de8481f Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Fri, 7 Nov 2025 11:50:14 -0800 Subject: [PATCH 5/5] Apply suggestion from @jkoritzinsky --- .../Compiler/DependencyAnalysis/Target_ARM/ARMJumpStubNode.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMJumpStubNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMJumpStubNode.cs index 39fedd5c8090bc..f5edce2b96278e 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMJumpStubNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMJumpStubNode.cs @@ -11,8 +11,7 @@ protected override void EmitCode(NodeFactory factory, ref ARMEmitter encoder, bo { if (!_target.RepresentsIndirectionCell) { - int delta = (_target is IMethodNode || _target is AssemblyStubNode) ? factory.Target.CodeDelta : 0; - encoder.EmitJMP(_target, delta); // b methodEntryPoint + encoder.EmitJMP(_target, factory.Target.CodeDelta); // b methodEntryPoint } else {