From 870ce3ed2bbc8cea09f91d328e941a0fa5a0cd21 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Wed, 18 Jan 2023 11:19:24 -0800 Subject: [PATCH] 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 9410f1e7d669f5f93db27ca2b3f2fe6b4a7ffa70. * Keep alignment values for code --- .github/workflows/main.yml | 2 +- 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 ++++++++++++++ 6 files changed, 47 insertions(+), 6 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 589e5cd32..638176463 100644 --- a/.github/workflows/main.yml +++ b/.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 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 df8afdcc8..836aa2172 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)