From d63d979403244b2c5a174ee32dcf95054add91a4 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Fri, 10 Dec 2021 12:52:35 -0800 Subject: [PATCH 1/3] 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` 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) --- Mono.Cecil.Metadata/Buffers.cs | 16 +++++- Mono.Cecil.PE/ImageWriter.cs | 2 +- Mono.Cecil/AssemblyWriter.cs | 13 ++++- Test/Mono.Cecil.Tests/FieldTests.cs | 44 ++++++++++++++++ Test/Resources/il/FieldRVAAlignment.il | 71 ++++++++++++++++++++++++++ 5 files changed, 143 insertions(+), 3 deletions(-) create mode 100644 Test/Resources/il/FieldRVAAlignment.il diff --git a/Mono.Cecil.Metadata/Buffers.cs b/Mono.Cecil.Metadata/Buffers.cs index 0dbf56852..1fbbdc2e8 100644 --- a/Mono.Cecil.Metadata/Buffers.cs +++ b/Mono.Cecil.Metadata/Buffers.cs @@ -256,17 +256,31 @@ 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--; + 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 index 0648f59f0..ad0286e82 100644 --- a/Mono.Cecil.PE/ImageWriter.cs +++ b/Mono.Cecil.PE/ImageWriter.cs @@ -690,7 +690,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 index e60ab1d8d..e9822489d 100644 --- a/Mono.Cecil/AssemblyWriter.cs +++ b/Mono.Cecil/AssemblyWriter.cs @@ -1643,8 +1643,19 @@ void AddField (FieldDefinition field) void AddFieldRVA (FieldDefinition field) { var table = GetTable (Table.FieldRVA); + + // To allow for safe implementation of metadata rewriters for code which uses CreateSpan + // if the Field RVA refers to a locally defined type with a pack > 1, align the InitialValue + // to pack boundary. + + int align = 1; + if (field.FieldType.IsDefinition && !field.FieldType.IsGenericInstance) { + var type = field.FieldType.Resolve (); + if (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 index 4f575de6e..93ed350ae 100644 --- 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 new file mode 100644 index 000000000..8149f7ab2 --- /dev/null +++ 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 '{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 '{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 +} // end of class '{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) From ebd76d42669961e9c044dbea3b7be07f0f43b31b Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 4 Jan 2022 14:07:39 -0800 Subject: [PATCH 2/3] Update Mono.Cecil.Metadata/Buffers.cs Co-authored-by: Aaron Robinson --- Mono.Cecil.Metadata/Buffers.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Mono.Cecil.Metadata/Buffers.cs b/Mono.Cecil.Metadata/Buffers.cs index 1fbbdc2e8..b32dd43ef 100644 --- a/Mono.Cecil.Metadata/Buffers.cs +++ b/Mono.Cecil.Metadata/Buffers.cs @@ -266,6 +266,8 @@ public DataBuffer () 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); } From cff2f9aee2564ea890a1eb41df63511e8bedba1f Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 11 Jan 2022 18:00:24 -0800 Subject: [PATCH 3/3] Enhace logic used to ensure type providing PackingSize is local to the module. --- Mono.Cecil/AssemblyWriter.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Mono.Cecil/AssemblyWriter.cs b/Mono.Cecil/AssemblyWriter.cs index e9822489d..c87de8582 100644 --- a/Mono.Cecil/AssemblyWriter.cs +++ b/Mono.Cecil/AssemblyWriter.cs @@ -1646,12 +1646,14 @@ void AddFieldRVA (FieldDefinition field) // To allow for safe implementation of metadata rewriters for code which uses CreateSpan // if the Field RVA refers to a locally defined type with a pack > 1, align the InitialValue - // to pack boundary. + // 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.PackingSize > 1) + + if ((type.Module == module) && (type.PackingSize > 1)) align = type.PackingSize; } table.AddRow (new FieldRVARow (