Fix RVA field alignment (jbevain#60)

This ensures that section starts are aligned by adjusting the
previous Range's length to ensure the computed start of a new
Range meets the alignment requirements. It was done this way
rather than just computing an aligned start for the new Range,
because the TextMap assumes that the Ranges are contiguous - see
for example GetNextRVA.

GetNextRVA was used to compute the Code RVA, before adding that
segment to the TextMap. This meant that the alignment (only added
later) wasn't taken into account when writing out the code, but
then later the TextMap was modified to include the alignment.

This is fixed by first inserting an aligned zero-length Code
segment before writing the code, then inserting it again once the
length is known. Removing the Code alignment altogether would
work too, but the alignment constants are kept in there, because
it looks like they were added intentionally, even though the
reason is unknown.

Note that before this change, the start of the Code segment (at
least on x64) was not 16-byte aligned in the first place, so this
change will actually move the beginning of the Code segment.

Mono.Cecil.Cil/CodeWriter.cs

@@ -32,7 +32,7 @@ sealed class CodeWriter : ByteBuffer {
 		public CodeWriter (MetadataBuilder metadata)
 			: base (0)
 		{
-			this.code_base = metadata.text_map.GetNextRVA (TextSegment.CLIHeader);
+			this.code_base = metadata.text_map.GetRVA (TextSegment.Code);
 			this.metadata = metadata;
 			this.standalone_signatures = new Dictionary<uint, MetadataToken> ();
 			this.tiny_method_bodies = new Dictionary<ByteBuffer, RVA> (new ByteBufferEqualityComparer ());

Mono.Cecil.PE/TextMap.cs

@@ -9,6 +9,7 @@
 //
 
 using System;
+using System.Diagnostics;
 
 using RVA = System.UInt32;
 
@@ -47,11 +48,31 @@ public void AddMap (TextSegment segment, int length)
 			map [(int) segment] = new Range (GetStart (segment), (uint) length);
 		}
 
-		public void AddMap (TextSegment segment, int length, int align)
+		uint AlignUp (uint value, uint align)
 		{
 			align--;
+			return (value + align) & ~align;
+		}
 
-			AddMap (segment, (length + align) & ~align);
+		public void AddMap (TextSegment segment, int length, int align)
+		{
+			var index = (int) segment;
+			uint start;
+			if (index != 0) {
+				// Align up the previous segment's length so that the new
+				// segment's start will be aligned.
+				index--;
+				Range previous = map [index];
+				start = AlignUp (previous.Start + previous.Length, (uint) align);
+				map [index].Length = start - previous.Start;
+			} else {
+				start = ImageWriter.text_rva;
+				// Should already be aligned.
+				Debug.Assert (start == AlignUp (start, (uint) align));
+			}
+			Debug.Assert (start == GetStart (segment));
+
+			map [(int) segment] = new Range (start, (uint) length);
 		}
 
 		public void AddMap (TextSegment segment, Range range)

Mono.Cecil/AssemblyWriter.cs

@@ -987,6 +987,11 @@ TextMap CreateTextMap ()
 			var map = new TextMap ();
 			map.AddMap (TextSegment.ImportAddressTable, module.Architecture == TargetArchitecture.I386 ? 8 : 0);
 			map.AddMap (TextSegment.CLIHeader, 0x48, 8);
+			var pe64 = module.Architecture == TargetArchitecture.AMD64 || module.Architecture == TargetArchitecture.IA64 || module.Architecture == TargetArchitecture.ARM64;
+			// Alignment of the code segment must be set before the code is written
+			// These alignment values are probably not necessary, but are being left in
+			// for now in case something requires them.
+			map.AddMap (TextSegment.Code, 0, !pe64 ? 4 : 16);
 			return map;
 		}
 

Test/Mono.Cecil.Tests/FieldTests.cs

@@ -160,7 +160,7 @@ public void FieldRVAAlignment ()
 					var priv_impl = GetPrivateImplementationType (module);
 					Assert.IsNotNull (priv_impl);
 
-					Assert.AreEqual (6, priv_impl.Fields.Count);
+					Assert.AreEqual (8, priv_impl.Fields.Count);
 
 					foreach (var field in priv_impl.Fields)
 					{
@@ -170,7 +170,8 @@ public void FieldRVAAlignment ()
 						Assert.IsNotNull (field.InitialValue);
 
 						int rvaAlignment = AlignmentOfInteger (field.RVA);
-						int desiredAlignment = Math.Min(8, AlignmentOfInteger (field.InitialValue.Length));
+						var fieldType = field.FieldType.Resolve ();
+						int desiredAlignment = fieldType.PackingSize;
 						Assert.GreaterOrEqual (rvaAlignment, desiredAlignment);
 					}
 				}

Test/Resources/il/FieldRVAAlignment.il

@@ -46,12 +46,22 @@
     .size 6
   } // end of class '__StaticArrayInitTypeSize=6'
 
+  .class explicit ansi sealed nested private '__StaticArrayInitTypeSize=4Align=32'
+        extends [mscorlib]System.ValueType
+  {
+    .pack 32
+    .size 4
+  }
+
   .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
+  .field static assembly valuetype '<PrivateImplementationDetails>{9B33BB20-87EF-4094-9948-34882DB2F001}'/'__StaticArrayInitTypeSize=3' 'separator' at I_00002140
+  .field static assembly valuetype '<PrivateImplementationDetails>{9B33BB20-87EF-4094-9948-34882DB2F001}'/'__StaticArrayInitTypeSize=4Align=32' '32_align' at I_00002150
+
 } // end of class '<PrivateImplementationDetails>{9B33BB20-87EF-4094-9948-34882DB2F001}'
 
 
@@ -69,3 +79,7 @@
                  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) 
+.data cil I_00002140 = bytearray (
+                 01 02 03)
+.data cil I_00002150 = bytearray (
+                 01 02 03 04)