Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[question] crossgen2 generates misaligned relocations #96066

Open
t-mustafin opened this issue Dec 15, 2023 · 9 comments
Open

[question] crossgen2 generates misaligned relocations #96066

t-mustafin opened this issue Dec 15, 2023 · 9 comments
Labels
area-crossgen2-coreclr question Answer questions and provide assistance, not an issue with source code or documentation.

Comments

@t-mustafin
Copy link
Contributor

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?

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Dec 15, 2023
@t-mustafin
Copy link
Contributor Author

@jkotas
Copy link
Member

jkotas commented Dec 15, 2023

Why crossgen2 uses PadAlignment only for Win32

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

@jkotas
Copy link
Member

jkotas commented Dec 15, 2023

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.

@jkotas jkotas added question Answer questions and provide assistance, not an issue with source code or documentation. and removed untriaged New issue has not been triaged by the area owner labels Dec 15, 2023
@t-mustafin
Copy link
Contributor Author

@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
Copy link
Member

jkotas commented Dec 15, 2023

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
Copy link
Contributor Author

t-mustafin commented Dec 15, 2023

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 --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
Copy link
Member

jkotas commented Dec 15, 2023

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.

@jkotas jkotas changed the title [question] crossgen2 grenerates misaligned relocations [question] crossgen2 generates misaligned relocations Dec 16, 2023
@t-mustafin
Copy link
Contributor Author

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
Copy link
Member

jkotas commented Dec 18, 2023

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

Yes, it is the same as WriteFieldRvas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-crossgen2-coreclr question Answer questions and provide assistance, not an issue with source code or documentation.
Projects
None yet
Development

No branches or pull requests

2 participants