Fix RVA field alignment (#888)

* Fix RVA field alignment

* Use non-latest images to run tests

* Revert windows-2019 change

* Don't align code segment

This wasn't aligned to begin with. Attempting to align it causes problems
due to the way CodeWriter gets the code RVA - it computes the RVA
from the CLI Header length, which doesn't take into account the code's
alignment requirements.

* Revert "Don't align code segment"

This reverts commit 9410f1e.

* Keep alignment values for code

Author: sbomer

.github/workflows/main.yml

@@ -20,7 +20,7 @@ jobs:
     - name: Test
       run: dotnet test --no-build -c Debug Mono.Cecil.sln
   linux:
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-20.04
     steps:
     - uses: actions/checkout@v1
     - name: Build

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)