From 7275abeab27cd5084e771ed46bc8fc4fd4a9efe1 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Tue, 27 Dec 2022 13:50:48 -0800 Subject: [PATCH] Fix RVA field alignment (#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 | 2 +- Mono.Cecil.PE/TextMap.cs | 25 +++++++++++++++++++++++-- Mono.Cecil/AssemblyWriter.cs | 5 +++++ Test/Mono.Cecil.Tests/FieldTests.cs | 5 +++-- Test/Resources/il/FieldRVAAlignment.il | 14 ++++++++++++++ 5 files changed, 46 insertions(+), 5 deletions(-) diff --git a/Mono.Cecil.Cil/CodeWriter.cs b/Mono.Cecil.Cil/CodeWriter.cs index 501710ebe..0ec4cc6e2 100644 --- a/Mono.Cecil.Cil/CodeWriter.cs +++ b/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 (); this.tiny_method_bodies = new Dictionary (new ByteBufferEqualityComparer ()); diff --git a/Mono.Cecil.PE/TextMap.cs b/Mono.Cecil.PE/TextMap.cs index c0061c592..548e471a7 100644 --- a/Mono.Cecil.PE/TextMap.cs +++ b/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) diff --git a/Mono.Cecil/AssemblyWriter.cs b/Mono.Cecil/AssemblyWriter.cs index b8b2b143d..27a54308c 100644 --- a/Mono.Cecil/AssemblyWriter.cs +++ b/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; } diff --git a/Test/Mono.Cecil.Tests/FieldTests.cs b/Test/Mono.Cecil.Tests/FieldTests.cs index 93ed350ae..6cbbb28a7 100644 --- a/Test/Mono.Cecil.Tests/FieldTests.cs +++ b/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); } } diff --git a/Test/Resources/il/FieldRVAAlignment.il b/Test/Resources/il/FieldRVAAlignment.il index 8149f7ab2..b3d431c74 100644 --- a/Test/Resources/il/FieldRVAAlignment.il +++ b/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 '{9B33BB20-87EF-4094-9948-34882DB2F001}'/'__StaticArrayInitTypeSize=3' '$$method0x6000001-1' at I_000020F0 .field static assembly valuetype '{9B33BB20-87EF-4094-9948-34882DB2F001}'/'__StaticArrayInitTypeSize=20' '$$method0x6000001-2' at I_000020F8 .field static assembly valuetype '{9B33BB20-87EF-4094-9948-34882DB2F001}'/'__StaticArrayInitTypeSize=5' '$$method0x6000001-3' at I_00002108 .field static assembly valuetype '{9B33BB20-87EF-4094-9948-34882DB2F001}'/'__StaticArrayInitTypeSize=20' '$$method0x6000001-4' at I_00002110 .field static assembly valuetype '{9B33BB20-87EF-4094-9948-34882DB2F001}'/'__StaticArrayInitTypeSize=6' '$$method0x6000001-5' at I_00002120 .field static assembly valuetype '{9B33BB20-87EF-4094-9948-34882DB2F001}'/'__StaticArrayInitTypeSize=16' '$$method0x6000001-6' at I_00002130 + .field static assembly valuetype '{9B33BB20-87EF-4094-9948-34882DB2F001}'/'__StaticArrayInitTypeSize=3' 'separator' at I_00002140 + .field static assembly valuetype '{9B33BB20-87EF-4094-9948-34882DB2F001}'/'__StaticArrayInitTypeSize=4Align=32' '32_align' at I_00002150 + } // end of class '{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)