FieldRVA alignment (#817)

* FieldRVA alignment In support of dotnet/runtime#60948 the linker (an assembly rewriter) will need to be able to preserve the alignment of RVA based fields which are to be used to create the data for `CreateSpan<T>` records This is implemented by adding a concept that RVA fields detect their required alignment by examining the PackingSize of the type of the field (if the field type is defined locally in the module) * Update Mono.Cecil.Metadata/Buffers.cs Co-authored-by: Aaron Robinson <arobins@microsoft.com> * Enhace logic used to ensure type providing PackingSize is local to the module. Co-authored-by: Aaron Robinson <arobins@microsoft.com>
jbevain · Jan 19, 2022 · a56b5bd · a56b5bd
1 parent 75372c7
commit a56b5bd
Show file tree

Hide file tree

Showing 5 changed files with 147 additions and 3 deletions.
diff --git a/Mono.Cecil.Metadata/Buffers.cs b/Mono.Cecil.Metadata/Buffers.cs
@@ -256,17 +256,33 @@ public uint AddResource (byte [] resource)
 
 	sealed class DataBuffer : ByteBuffer {
 
+		int buffer_align = 4;
+
 		public DataBuffer ()
 			: base (0)
 		{
 		}
 
-		public RVA AddData (byte [] data)
+		void Align (int align)
+		{
+			align--;
+                        // Compute the number of bytes to align the current position.
+                        // Values of 0 will be written.
+			WriteBytes (((position + align) & ~align) - position);
+		}
+
+		public RVA AddData (byte [] data, int align)
 		{
+			if (buffer_align < align)
+				buffer_align = align;
+
+			Align (align);
 			var rva = (RVA) position;
 			WriteBytes (data);
 			return rva;
 		}
+
+		public int BufferAlign => buffer_align;
 	}
 
 	abstract class HeapBuffer : ByteBuffer {

diff --git a/Mono.Cecil.PE/ImageWriter.cs b/Mono.Cecil.PE/ImageWriter.cs
@@ -694,7 +694,7 @@ void BuildTextMap ()
 
 			map.AddMap (TextSegment.Code, metadata.code.length, !pe64 ? 4 : 16);
 			map.AddMap (TextSegment.Resources, metadata.resources.length, 8);
-			map.AddMap (TextSegment.Data, metadata.data.length, 4);
+			map.AddMap (TextSegment.Data, metadata.data.length, metadata.data.BufferAlign);
 			if (metadata.data.length > 0)
 				metadata.table_heap.FixupData (map.GetRVA (TextSegment.Data));
 			map.AddMap (TextSegment.StrongNameSignature, GetStrongNameLength (), 4);

diff --git a/Mono.Cecil/AssemblyWriter.cs b/Mono.Cecil/AssemblyWriter.cs
@@ -1613,8 +1613,21 @@ void AddField (FieldDefinition field)
 		void AddFieldRVA (FieldDefinition field)
 		{
 			var table = GetTable<FieldRVATable> (Table.FieldRVA);
+
+			// To allow for safe implementation of metadata rewriters for code which uses CreateSpan<T>
+			// if the Field RVA refers to a locally defined type with a pack > 1, align the InitialValue
+			// to pack boundary. This logic is restricted to only being affected by metadata local to the module
+			// as PackingSize is only used when examining a type local to the module being written.
+
+			int align = 1;
+			if (field.FieldType.IsDefinition && !field.FieldType.IsGenericInstance) {
+				var type = field.FieldType.Resolve ();
+
+				if ((type.Module == module) && (type.PackingSize > 1))
+					align = type.PackingSize;
+			}
 			table.AddRow (new FieldRVARow (
-				data.AddData (field.InitialValue),
+				data.AddData (field.InitialValue, align),
 				field.token.RID));
 		}
 

diff --git a/Test/Mono.Cecil.Tests/FieldTests.cs b/Test/Mono.Cecil.Tests/FieldTests.cs
@@ -1,4 +1,5 @@
 using System;
+using System.IO;
 
 using Mono.Cecil.PE;
 
@@ -133,6 +134,49 @@ public void FieldRVA ()
 			});
 		}
 
+		int AlignmentOfInteger(int input)
+		{
+			if (input == 0)
+				return 0x40000000;
+			if (input < 0)
+				Assert.Fail ();
+			int alignment = 1;
+			while ((input & alignment) == 0)
+				alignment *= 2;
+
+			return alignment;
+		}
+
+		[Test]
+		public void FieldRVAAlignment ()
+		{
+			TestIL ("FieldRVAAlignment.il", ilmodule => {
+
+				var path = Path.GetTempFileName ();
+
+				ilmodule.Write (path);
+
+				using (var module = ModuleDefinition.ReadModule (path, new ReaderParameters { ReadWrite = true })) {
+					var priv_impl = GetPrivateImplementationType (module);
+					Assert.IsNotNull (priv_impl);
+
+					Assert.AreEqual (6, priv_impl.Fields.Count);
+
+					foreach (var field in priv_impl.Fields)
+					{
+						Assert.IsNotNull (field);
+
+						Assert.AreNotEqual (0, field.RVA);
+						Assert.IsNotNull (field.InitialValue);
+
+						int rvaAlignment = AlignmentOfInteger (field.RVA);
+						int desiredAlignment = Math.Min(8, AlignmentOfInteger (field.InitialValue.Length));
+						Assert.GreaterOrEqual (rvaAlignment, desiredAlignment);
+					}
+				}
+			});
+		}
+
 		[Test]
 		public void GenericFieldDefinition ()
 		{

diff --git a/Test/Resources/il/FieldRVAAlignment.il b/Test/Resources/il/FieldRVAAlignment.il
@@ -0,0 +1,71 @@
+.assembly extern mscorlib
+{
+  .publickeytoken = (B7 7A 5C 56 19 34 E0 89 )                         // .z\V.4..
+  .ver 4:0:0:0
+}
+.assembly FieldRVAAlignment {}
+
+.module FieldRVAAlignment.dll
+
+.class private auto ansi '<PrivateImplementationDetails>{9B33BB20-87EF-4094-9948-34882DB2F001}'
+       extends [mscorlib]System.Object
+{
+  .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) 
+  .class explicit ansi sealed nested private '__StaticArrayInitTypeSize=3'
+         extends [mscorlib]System.ValueType
+  {
+    .pack 1
+    .size 3
+  } // end of class '__StaticArrayInitTypeSize=3'
+
+  .class explicit ansi sealed nested private '__StaticArrayInitTypeSize=16'
+         extends [mscorlib]System.ValueType
+  {
+    .pack 8
+    .size 16
+  } // end of class '__StaticArrayInitTypeSize=16'
+
+  .class explicit ansi sealed nested private '__StaticArrayInitTypeSize=20'
+         extends [mscorlib]System.ValueType
+  {
+    .pack 4
+    .size 20
+  } // end of class '__StaticArrayInitTypeSize=20'
+
+  .class explicit ansi sealed nested private '__StaticArrayInitTypeSize=5'
+         extends [mscorlib]System.ValueType
+  {
+    .pack 1
+    .size 5
+  } // end of class '__StaticArrayInitTypeSize=5'
+
+  .class explicit ansi sealed nested private '__StaticArrayInitTypeSize=6'
+         extends [mscorlib]System.ValueType
+  {
+    .pack 2
+    .size 6
+  } // end of class '__StaticArrayInitTypeSize=6'
+
+  .field static assembly valuetype '<PrivateImplementationDetails>{9B33BB20-87EF-4094-9948-34882DB2F001}'/'__StaticArrayInitTypeSize=3' '$$method0x6000001-1' at I_000020F0
+  .field static assembly valuetype '<PrivateImplementationDetails>{9B33BB20-87EF-4094-9948-34882DB2F001}'/'__StaticArrayInitTypeSize=20' '$$method0x6000001-2' at I_000020F8
+  .field static assembly valuetype '<PrivateImplementationDetails>{9B33BB20-87EF-4094-9948-34882DB2F001}'/'__StaticArrayInitTypeSize=5' '$$method0x6000001-3' at I_00002108
+  .field static assembly valuetype '<PrivateImplementationDetails>{9B33BB20-87EF-4094-9948-34882DB2F001}'/'__StaticArrayInitTypeSize=20' '$$method0x6000001-4' at I_00002110
+  .field static assembly valuetype '<PrivateImplementationDetails>{9B33BB20-87EF-4094-9948-34882DB2F001}'/'__StaticArrayInitTypeSize=6' '$$method0x6000001-5' at I_00002120
+  .field static assembly valuetype '<PrivateImplementationDetails>{9B33BB20-87EF-4094-9948-34882DB2F001}'/'__StaticArrayInitTypeSize=16' '$$method0x6000001-6' at I_00002130
+} // end of class '<PrivateImplementationDetails>{9B33BB20-87EF-4094-9948-34882DB2F001}'
+
+
+// =============================================================
+
+.data cil I_000020F0 = bytearray (
+                 01 02 03)
+.data cil I_000020F8 = bytearray (
+                 01 00 00 00 02 00 00 00 03 00 00 00 04 00 00 00 05 00 00 00) 
+.data cil I_00002108 = bytearray (
+                 04 05 06 07 08)
+.data cil I_00002110 = bytearray (
+                 01 00 00 00 02 00 00 00 03 00 00 00 05 00 00 00 06 00 00 00) 
+.data cil I_00002120 = bytearray (
+                 08 00 0C 00 0D 00) 
+.data cil I_00002130 = bytearray (
+                 01 00 00 00 02 00 00 00 03 00 00 00 05 00 00 00)