From dcddc2e6709ba931d91d20397fa116d4cfa4dae9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Tue, 25 Jun 2024 10:32:36 +0200 Subject: [PATCH 1/5] Report `RhpInitialDynamicInterfaceDispatch` reference in gfids Some subset of tests is failing with control flow guard enabled because we're not able to indirectly call `RhpInitialDynamicInterfaceDispatch`. It's not clear why since we address take this method in C++ source files as well, but we should be reporting this from our object file too and doing that fixes it. --- .../DependencyAnalysis/ExternSymbolNode.cs | 8 +++++++ .../InterfaceDispatchCellNode.cs | 20 ++---------------- .../DependencyAnalysis/NodeFactory.cs | 10 +++++++++ .../Target_ARM/ARMNodeFactory.cs | 21 ------------------- .../Compiler/ObjectWriter/ObjectWriter.cs | 2 +- .../ILCompiler.Compiler.csproj | 1 - 6 files changed, 21 insertions(+), 41 deletions(-) delete mode 100644 src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMNodeFactory.cs diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ExternSymbolNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ExternSymbolNode.cs index ca02f10f177de..6685fde083127 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ExternSymbolNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ExternSymbolNode.cs @@ -55,4 +55,12 @@ public override string ToString() return _name.ToString(); } } + + public class AddressTakenExternSymbolNode : ExternSymbolNode + { + public AddressTakenExternSymbolNode(Utf8String name) + : base(name) { } + + public override int ClassCode => -45645737; + } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/InterfaceDispatchCellNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/InterfaceDispatchCellNode.cs index 523386d130303..e0bf93690edf8 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/InterfaceDispatchCellNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/InterfaceDispatchCellNode.cs @@ -61,15 +61,7 @@ public override IEnumerable GetStaticDependencies(NodeFacto factory.MetadataManager.GetDependenciesDueToVirtualMethodReflectability(ref result, factory, _targetMethod); - TargetArchitecture targetArchitecture = factory.Target.Architecture; - if (targetArchitecture == TargetArchitecture.ARM) - { - result.Add(factory.InitialInterfaceDispatchStub, "Initial interface dispatch stub"); - } - else - { - result.Add(factory.ExternSymbol("RhpInitialDynamicInterfaceDispatch"), "Initial interface dispatch stub"); - } + result.Add(factory.InitialInterfaceDispatchStub, "Initial interface dispatch stub"); // We counter-intuitively ask for a constructed type symbol. This is needed due to IDynamicInterfaceCastable. // If this dispatch cell is ever used with an object that implements IDynamicIntefaceCastable, user code will @@ -81,15 +73,7 @@ public override IEnumerable GetStaticDependencies(NodeFacto public override void EncodeData(ref ObjectDataBuilder objData, NodeFactory factory, bool relocsOnly) { - TargetArchitecture targetArchitecture = factory.Target.Architecture; - if (targetArchitecture == TargetArchitecture.ARM) - { - objData.EmitPointerReloc(factory.InitialInterfaceDispatchStub); - } - else - { - objData.EmitPointerReloc(factory.ExternSymbol("RhpInitialDynamicInterfaceDispatch")); - } + objData.EmitPointerReloc(factory.InitialInterfaceDispatchStub); // We counter-intuitively ask for a constructed type symbol. This is needed due to IDynamicInterfaceCastable. // If this dispatch cell is ever used with an object that implements IDynamicIntefaceCastable, user code will diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs index 586ec5eb0be77..7c8997f7d6cdb 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs @@ -44,6 +44,11 @@ public NodeFactory( ObjectDataInterner dataInterner) { _target = context.Target; + + InitialInterfaceDispatchStub = _target.Architecture == TargetArchitecture.ARM + ? new InitialInterfaceDispatchStubNode() + : new AddressTakenExternSymbolNode("RhpInitialDynamicInterfaceDispatch"); + _context = context; _compilationModuleGroup = compilationModuleGroup; _vtableSliceProvider = vtableSliceProvider; @@ -102,6 +107,11 @@ public NameMangler NameMangler get; } + public ISymbolNode InitialInterfaceDispatchStub + { + get; + } + public PreinitializationManager PreinitializationManager { get; diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMNodeFactory.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMNodeFactory.cs deleted file mode 100644 index 3bcd30e2ddb40..0000000000000 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMNodeFactory.cs +++ /dev/null @@ -1,21 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace ILCompiler.DependencyAnalysis -{ - public partial class NodeFactory - { - private InitialInterfaceDispatchStubNode _initialInterfaceDispatchStubNode; - - public InitialInterfaceDispatchStubNode InitialInterfaceDispatchStub - { - get - { - _initialInterfaceDispatchStubNode ??= new InitialInterfaceDispatchStubNode(); - - return _initialInterfaceDispatchStubNode; - } - } - - } -} diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/ObjectWriter.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/ObjectWriter.cs index ebccf0bba30a3..dca7910a6b595 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/ObjectWriter.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/ObjectWriter.cs @@ -469,7 +469,7 @@ private void EmitObject(string objectFilePath, IReadOnlyCollection - From 31774eb3cab195d1acca35b4a33a77ae26b92607 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Tue, 25 Jun 2024 10:34:33 +0200 Subject: [PATCH 2/5] Temp enable CFG --- .../nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets b/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets index dead34d2bfbf8..a4289e505312b 100644 --- a/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets +++ b/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets @@ -16,6 +16,8 @@ The .NET Foundation licenses this file to you under the MIT license. + Guard + true Shared $(IntermediateOutputPath)native\ From f71b07387ae4fc43b01192253945b79e46f58275 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Tue, 25 Jun 2024 13:42:13 +0200 Subject: [PATCH 3/5] Temp disable assert that broke everything --- src/coreclr/jit/hwintrinsicxarch.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/hwintrinsicxarch.cpp b/src/coreclr/jit/hwintrinsicxarch.cpp index 89a2b1ba6042d..0a2ec34077247 100644 --- a/src/coreclr/jit/hwintrinsicxarch.cpp +++ b/src/coreclr/jit/hwintrinsicxarch.cpp @@ -267,17 +267,17 @@ static CORINFO_InstructionSet lookupInstructionSet(const char* className) { if (strncmp(className + 6, "128", 3) == 0) { - assert((className[9] == '\0') || (strcmp(className + 9, "`1") == 0)); + //assert((className[9] == '\0') || (strcmp(className + 9, "`1") == 0)); return InstructionSet_Vector128; } else if (strncmp(className + 6, "256", 3) == 0) { - assert((className[9] == '\0') || (strcmp(className + 9, "`1") == 0)); + //assert((className[9] == '\0') || (strcmp(className + 9, "`1") == 0)); return InstructionSet_Vector256; } else if (strncmp(className + 6, "512", 3) == 0) { - assert((className[9] == '\0') || (strcmp(className + 9, "`1") == 0)); + //assert((className[9] == '\0') || (strcmp(className + 9, "`1") == 0)); return InstructionSet_Vector512; } } From 17588111616eec311d009c27b108dca3b690ad49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Tue, 25 Jun 2024 23:58:32 +0200 Subject: [PATCH 4/5] Revert "Temp disable assert that broke everything" This reverts commit f71b07387ae4fc43b01192253945b79e46f58275. --- src/coreclr/jit/hwintrinsicxarch.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/hwintrinsicxarch.cpp b/src/coreclr/jit/hwintrinsicxarch.cpp index 0a2ec34077247..89a2b1ba6042d 100644 --- a/src/coreclr/jit/hwintrinsicxarch.cpp +++ b/src/coreclr/jit/hwintrinsicxarch.cpp @@ -267,17 +267,17 @@ static CORINFO_InstructionSet lookupInstructionSet(const char* className) { if (strncmp(className + 6, "128", 3) == 0) { - //assert((className[9] == '\0') || (strcmp(className + 9, "`1") == 0)); + assert((className[9] == '\0') || (strcmp(className + 9, "`1") == 0)); return InstructionSet_Vector128; } else if (strncmp(className + 6, "256", 3) == 0) { - //assert((className[9] == '\0') || (strcmp(className + 9, "`1") == 0)); + assert((className[9] == '\0') || (strcmp(className + 9, "`1") == 0)); return InstructionSet_Vector256; } else if (strncmp(className + 6, "512", 3) == 0) { - //assert((className[9] == '\0') || (strcmp(className + 9, "`1") == 0)); + assert((className[9] == '\0') || (strcmp(className + 9, "`1") == 0)); return InstructionSet_Vector512; } } From a08f8891dcbb3bf285d5bd135e044d837d964485 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Tue, 25 Jun 2024 23:58:38 +0200 Subject: [PATCH 5/5] Revert "Temp enable CFG" This reverts commit 31774eb3cab195d1acca35b4a33a77ae26b92607. --- .../nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets b/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets index a4289e505312b..dead34d2bfbf8 100644 --- a/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets +++ b/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets @@ -16,8 +16,6 @@ The .NET Foundation licenses this file to you under the MIT license. - Guard - true Shared $(IntermediateOutputPath)native\