dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.97k stars 4.66k forks source link

[question] crossgen2 generates misaligned relocations #96066

Open t-mustafin opened 9 months ago

t-mustafin commented 9 months ago

Crossgen2 constructs some code by ObjectDataBuilder class. In some cases the code includes pieces of data, for example relocation pointers. To proper align the data ObjectDataBuilder has method PadAlignment: https://github.com/dotnet/runtime/blob/486142a4b87ed6e3134bd1a8726156fb3f55157a/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectDataBuilder.cs#L357C44-L357C44 At the same time the only platform which use PadAlignment method is Win32.

Why crossgen2 uses PadAlignment only for Win32 and generates misaligned data for other platforms? On platforms there misaligned memory operations is not prohibited they may cause performance degradation in compare with aligned operations. Is there some memory/performance investigations for this misaligned/aligned data generation?

t-mustafin commented 9 months ago

cc @jkotas @MichalStrehovsky @gbalykov @ashaurtaev

jkotas commented 9 months ago

Why crossgen2 uses PadAlignment only for Win32

Where do you see PadAlignment only used for Windows and not for other other platforms?

jkotas commented 9 months ago

The misaligned relocations should be produced for 32-bit x86 only. If you see misaligned relocations on other platforms, it is most likely a bug.

t-mustafin commented 9 months ago

@jkotas > Where do you see PadAlignment only used for Windows and not for other other platforms?

I see it in source code:

src$ grep coreclr/ -re PadAlignment
coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectDataBuilder.cs:        public void PadAlignment(int align)
coreclr/tools/Common/Compiler/Win32Resources/ResourceData.cs:            dataBuilder.PadAlignment(2); // name table is 2 byte aligned
coreclr/tools/Common/Compiler/Win32Resources/ResourceData.cs:                contentBuilder.PadAlignment(4); // Data in resource files is 4 byte aligned
coreclr/tools/Common/Compiler/Win32Resources/ResourceData.cs:            dataBuilder.PadAlignment(4); // resource data entries are 4 byte aligned
coreclr/tools/Common/Compiler/Win32Resources/ResourceData.cs:            dataBuilder.PadAlignment(4); // resource data entries are 4 byte aligned
jkotas commented 9 months ago

The Win32Resource format is used as part of R2R format on all platforms today. This code is not Windows specific. This padding for alignment is part of Win32Resource format spec.

Is there a specific place where you see misaligned relocations that are causing problems for you?

t-mustafin commented 9 months ago

We do not have any crashes caused by misalignment. During work on crossgen2 for riscv64 #95188 we found relocations are not naturally aligned and same was for arm64. Inserted alignment warnings in methods EmitLong EmitInt and EmitShort show a lot of warnings during build System.Private.CoreLib for arm64 like 8-byte value is not naturally aligned:

  EmitReloc(ILCompiler.DependencyAnalysis.ReadyToRun.Import, IMAGE_REL_BASED_DIR64, 0)
  AssertAlignment offset = 12 align = 8
     at ILCompiler.DependencyAnalysis.ObjectDataBuilder.AssertAlignment(Int32 offset, Int32 align) in /home/runtime/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectDataBuilder.cs:line 395
     at ILCompiler.DependencyAnalysis.ObjectDataBuilder.AssertAlignment(Int32 align) in /home/runtime/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectDataBuilder.cs:line 386
     at ILCompiler.DependencyAnalysis.ObjectDataBuilder.EmitLong(Int64 emit) in /home/runtime/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectDataBuilder.cs:line 124
     at ILCompiler.DependencyAnalysis.ObjectDataBuilder.EmitReloc(ISymbolNode symbol, RelocType relocType, Int32 delta) in /home/runtime/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectDataBuilder.cs:line 321
     at ILCompiler.DependencyAnalysis.ARM64.ARM64Emitter.EmitJMP(ISymbolNode symbol) in /home/runtime/src/coreclr/tools/Common/Compiler/DependencyAnalysis/Target_ARM64/ARM64Emitter.cs:line 153
     at ILCompiler.DependencyAnalysis.ReadyToRun.ImportThunk.EmitCode(NodeFactory factory, ARM64Emitter& instructionEncoder, Boolean relocsOnly) in /home/runtime/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/Target_ARM64/ImportThunk.cs:line 59
     at ILCompiler.DependencyAnalysis.AssemblyStubNode.GetData(NodeFactory factory, Boolean relocsOnly) in /home/runtime/src/coreclr/tools/Common/Compiler/DependencyAnalysis/AssemblyStubNode.cs:line 66
     at ILCompiler.DependencyAnalysis.ObjectNode.GetStaticDependencies(NodeFactory factory) in /home/runtime/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectNode.cs:line 59
     at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.GetStaticDependenciesImpl(DependencyNodeCore`1 node) in /home/runtime/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs:line 182
     at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.GetStaticDependencies(DependencyNodeCore`1 node) in /home/runtime/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs:line 222
     at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.ProcessMarkStack() in /home/runtime/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs:line 257
     at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.ComputeMarkedNodes() in /home/runtime/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs:line 308
     at ILCompiler.ReadyToRunCodegenCompilation.Compile(String outputFile) in /home/runtime/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs:line 387
     at ILCompiler.Program.RunSingleCompilation(Dictionary`2 inFilePaths, InstructionSetSupport instructionSetSupport, String compositeRootPath, Dictionary`2 unrootedInputFilePaths, HashSet`1 versionBubbleModulesHash, ReadyToRunCompilerContext typeSystemContext, Logger logger) in /home/runtime/src/coreclr/tools/aot/crossgen2/Program.cs:line 637
     at ILCompiler.Program.Run() in /home/runtime/src/coreclr/tools/aot/crossgen2/Program.cs:line 302
     at ILCompiler.Crossgen2RootCommand.<>c__DisplayClass205_0.<.ctor>b__0(ParseResult result) in /home/runtime/src/coreclr/tools/aot/crossgen2/Crossgen2RootCommand.cs:line 261
     at System.CommandLine.Invocation.InvocationPipeline.Invoke(ParseResult parseResult)
     at System.CommandLine.ParseResult.Invoke()
     at ILCompiler.Program.Main(String[] args) in /home/runtime/src/coreclr/tools/aot/crossgen2/Program.cs:line 914

and 4-byte value is not naturally aligned:

EmitReloc(ILCompiler.DependencyAnalysis.ReadyToRun.CopiedFieldRvaNode, IMAGE_REL_BASED_ADDR32NB, 0)
  AssertAlignment offset = 1759758 align = 4
     at ILCompiler.DependencyAnalysis.ObjectDataBuilder.AssertAlignment(Int32 offset, Int32 align) in /home/runtime/src/coreclr/tools/Common/Comp
iler/DependencyAnalysis/ObjectDataBuilder.cs:line 395
     at ILCompiler.DependencyAnalysis.ObjectDataBuilder.AssertAlignment(Int32 align) in /home/runtime/src/coreclr/tools/Common/Compiler/Dependenc
yAnalysis/ObjectDataBuilder.cs:line 386
     at ILCompiler.DependencyAnalysis.ObjectDataBuilder.EmitInt(Int32 emit) in /home/runtime/src/coreclr/tools/Common/Compiler/DependencyAnalysis
/ObjectDataBuilder.cs:line 104
     at ILCompiler.DependencyAnalysis.ObjectDataBuilder.EmitReloc(ISymbolNode symbol, RelocType relocType, Int32 delta) in /home/runtime/src/core
clr/tools/Common/Compiler/DependencyAnalysis/ObjectDataBuilder.cs:line 318
     at ILCompiler.DependencyAnalysis.ReadyToRun.CopiedMetadataBlobNode.WriteFieldRvas(NodeFactory factory, ObjectDataBuilder& builder, BlobReade
r& reader) in /home/runtime/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/CopiedMetadataBlobNode.cs:line 97
     at ILCompiler.DependencyAnalysis.ReadyToRun.CopiedMetadataBlobNode.GetData(NodeFactory factory, Boolean relocsOnly) in /home/runtime/src/cor
eclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/CopiedMetadataBlobNode.cs:line 134
     at ILCompiler.DependencyAnalysis.ObjectNode.GetStaticDependencies(NodeFactory factory) in /home/runtime/src/coreclr/tools/Common/Compiler/De
pendencyAnalysis/ObjectNode.cs:line 59
     at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.GetStaticDependenciesImpl(DependencyNodeCore`1 node) in /home/runtime/src/cor
eclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs:line 182
     at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.GetStaticDependencies(DependencyNodeCore`1 node) in /home/runtime/src/coreclr
/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs:line 222
     at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.ProcessMarkStack() in /home/runtime/src/coreclr/tools/aot/ILCompiler.Dependen
cyAnalysisFramework/DependencyAnalyzer.cs:line 257
     at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.ComputeMarkedNodes() in /home/runtime/src/coreclr/tools/aot/ILCompiler.Depend
encyAnalysisFramework/DependencyAnalyzer.cs:line 308
     at ILCompiler.ReadyToRunCodegenCompilation.Compile(String outputFile) in /home/runtime/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/
ReadyToRunCodegenCompilation.cs:line 387
     at ILCompiler.Program.RunSingleCompilation(Dictionary`2 inFilePaths, InstructionSetSupport instructionSetSupport, String compositeRootPath, 
Dictionary`2 unrootedInputFilePaths, HashSet`1 versionBubbleModulesHash, ReadyToRunCompilerContext typeSystemContext, Logger logger) in /home/run
time/src/coreclr/tools/aot/crossgen2/Program.cs:line 637
     at ILCompiler.Program.Run() in /home/runtime/src/coreclr/tools/aot/crossgen2/Program.cs:line 302
     at ILCompiler.Crossgen2RootCommand.<>c__DisplayClass205_0.<.ctor>b__0(ParseResult result) in /home/runtime/src/coreclr/tools/aot/crossgen2/C
rossgen2RootCommand.cs:line 261
     at System.CommandLine.Invocation.InvocationPipeline.Invoke(ParseResult parseResult)
     at System.CommandLine.ParseResult.Invoke()
     at ILCompiler.Program.Main(String[] args) in /home/runtime/src/coreclr/tools/aot/crossgen2/Program.cs:line 914
Relocation type print and alignment warning patch ```diff diff --git a/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectDataBuilder.cs b/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectDataBuilder.cs index 3762d1bd68f..1a716687232 100644 --- a/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectDataBuilder.cs +++ b/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectDataBuilder.cs @@ -85,18 +85,24 @@ public void EmitByte(byte emit) public void EmitShort(short emit) { + AssertAlignment(sizeof(short)); + EmitByte((byte)(emit & 0xFF)); EmitByte((byte)((emit >> 8) & 0xFF)); } public void EmitUShort(ushort emit) { + AssertAlignment(sizeof(ushort)); + EmitByte((byte)(emit & 0xFF)); EmitByte((byte)((emit >> 8) & 0xFF)); } public void EmitInt(int emit) { + AssertAlignment(sizeof(int)); + EmitByte((byte)(emit & 0xFF)); EmitByte((byte)((emit >> 8) & 0xFF)); EmitByte((byte)((emit >> 16) & 0xFF)); @@ -105,6 +111,8 @@ public void EmitInt(int emit) public void EmitUInt(uint emit) { + AssertAlignment(sizeof(uint)); + EmitByte((byte)(emit & 0xFF)); EmitByte((byte)((emit >> 8) & 0xFF)); EmitByte((byte)((emit >> 16) & 0xFF)); @@ -113,6 +121,8 @@ public void EmitUInt(uint emit) public void EmitLong(long emit) { + AssertAlignment(sizeof(long)); + EmitByte((byte)(emit & 0xFF)); EmitByte((byte)((emit >> 8) & 0xFF)); EmitByte((byte)((emit >> 16) & 0xFF)); @@ -245,6 +255,8 @@ public Reservation ReserveShort() public void EmitShort(Reservation reservation, short emit) { int offset = ReturnReservationTicket(reservation); + + AssertAlignment(offset, sizeof(short)); _data[offset] = (byte)(emit & 0xFF); _data[offset + 1] = (byte)((emit >> 8) & 0xFF); } @@ -257,6 +269,8 @@ public Reservation ReserveInt() public void EmitInt(Reservation reservation, int emit) { int offset = ReturnReservationTicket(reservation); + + AssertAlignment(offset, sizeof(int)); _data[offset] = (byte)(emit & 0xFF); _data[offset + 1] = (byte)((emit >> 8) & 0xFF); _data[offset + 2] = (byte)((emit >> 16) & 0xFF); @@ -266,6 +280,8 @@ public void EmitInt(Reservation reservation, int emit) public void EmitUInt(Reservation reservation, uint emit) { int offset = ReturnReservationTicket(reservation); + + AssertAlignment(offset, sizeof(uint)); _data[offset] = (byte)(emit & 0xFF); _data[offset + 1] = (byte)((emit >> 8) & 0xFF); _data[offset + 2] = (byte)((emit >> 16) & 0xFF); @@ -285,6 +301,7 @@ public void EmitReloc(ISymbolNode symbol, RelocType relocType, int delta = 0) _relocs.Add(new Relocation(relocType, _data.Count, symbol)); + System.Console.WriteLine($"EmitReloc({symbol}, {relocType}, {delta})"); // And add space for the reloc switch (relocType) { @@ -365,5 +382,21 @@ public void PadAlignment(int align) EmitZeros(align - misalignment); } } + + public void AssertAlignment(int align) + { + AssertAlignment(_data.Count, align); + } + + public void AssertAlignment(int offset, int align) + { + if (((int)offset & (align - 1)) != 0) + { + System.Console.WriteLine($"AssertAlignment offset = {offset} align = {align}"); + + System.Diagnostics.StackTrace st = new System.Diagnostics.StackTrace(true); + System.Console.WriteLine($"{st}"); + } + } } } ```
jkotas commented 9 months ago

ILCompiler.DependencyAnalysis.ReadyToRun.ImportThunk.EmitCode

The delay load thunks for arm64 do not seem to be as compact as they can be. I think we should be able to both fix the misaligned relocs and make the arm64 delay load thunks more compact at the same time.

ILCompiler.DependencyAnalysis.ReadyToRun.CopiedMetadataBlobNode.WriteFieldRvas

This looks by design. The RVA fields in ECMA-335 metadata blob are not always aligned by design.

t-mustafin commented 9 months ago

There is one more different type of stack:

  EmitReloc(ILCompiler.DependencyAnalysis.ReadyToRun.CopiedMethodILNode, IMAGE_REL_BASED_ADDR32NB, 0)
  AssertAlignment offset = 135418 align = 4
     at ILCompiler.DependencyAnalysis.ObjectDataBuilder.AssertAlignment(Int32 offset, Int32 align) in /home/runtime/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectDataBuilder.cs:line 395
     at ILCompiler.DependencyAnalysis.ObjectDataBuilder.AssertAlignment(Int32 align) in /home/runtime/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectDataBuilder.cs:line 386
     at ILCompiler.DependencyAnalysis.ObjectDataBuilder.EmitInt(Int32 emit) in /home/runtime/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectDataBuilder.cs:line 104
     at ILCompiler.DependencyAnalysis.ObjectDataBuilder.EmitReloc(ISymbolNode symbol, RelocType relocType, Int32 delta) in /home/runtime/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectDataBuilder.cs:line 318
     at ILCompiler.DependencyAnalysis.ReadyToRun.CopiedMetadataBlobNode.WriteMethodTableRvas(NodeFactory factory, ObjectDataBuilder& builder, BlobReader& reader) in /home/runtime/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/CopiedMetadataBlobNode.cs:line 61
     at ILCompiler.DependencyAnalysis.ReadyToRun.CopiedMetadataBlobNode.GetData(NodeFactory factory, Boolean relocsOnly) in /home/runtime/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/CopiedMetadataBlobNode.cs:line 125
     at ILCompiler.DependencyAnalysis.ObjectNode.GetStaticDependencies(NodeFactory factory) in /home/runtime/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectNode.cs:line 59
     at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.GetStaticDependenciesImpl(DependencyNodeCore`1 node) in /home/runtime/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs:line 182
     at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.GetStaticDependencies(DependencyNodeCore`1 node) in /home/runtime/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs:line 222
     at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.ProcessMarkStack() in /home/runtime/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs:line 257
     at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.ComputeMarkedNodes() in /home/runtime/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs:line 308
     at ILCompiler.ReadyToRunCodegenCompilation.Compile(String outputFile) in /home/runtime/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs:line 387
     at ILCompiler.Program.RunSingleCompilation(Dictionary`2 inFilePaths, InstructionSetSupport instructionSetSupport, String compositeRootPath, Dictionary`2 unrootedInputFilePaths, HashSet`1 versionBubbleModulesHash, ReadyToRunCompilerContext typeSystemContext, Logger logger) in /home/runtime/src/coreclr/tools/aot/crossgen2/Program.cs:line 637
     at ILCompiler.Program.Run() in /home/runtime/src/coreclr/tools/aot/crossgen2/Program.cs:line 302
     at ILCompiler.Crossgen2RootCommand.<>c__DisplayClass205_0.<.ctor>b__0(ParseResult result) in /home/runtime/src/coreclr/tools/aot/crossgen2/Crossgen2RootCommand.cs:line 261
     at System.CommandLine.Invocation.InvocationPipeline.Invoke(ParseResult parseResult)
     at System.CommandLine.ParseResult.Invoke()
     at ILCompiler.Program.Main(String[] args) in /home/runtime/src/coreclr/tools/aot/crossgen2/Program.cs:line 914

Is it the same as ILCompiler.DependencyAnalysis.ReadyToRun.CopiedMetadataBlobNode.WriteFieldRvas ? There is no other misalignment stack types except this three for crossgen2 SPC.dll for arm64.

jkotas commented 9 months ago

Is it the same as ILCompiler.DependencyAnalysis.ReadyToRun.CopiedMetadataBlobNode.WriteFieldRvas ?

Yes, it is the same as WriteFieldRvas.