From a56b5bda0ca83789e87bf9d34fc0a9e8d2148115 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 19 Jan 2022 15:05:08 -0800 Subject: [PATCH] FieldRVA alignment (#817) * 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) * Update Mono.Cecil.Metadata/Buffers.cs Co-authored-by: Aaron Robinson * Enhace logic used to ensure type providing PackingSize is local to the module. Co-authored-by: Aaron Robinson --- Mono.Cecil.Metadata/Buffers.cs | 18 ++++++- Mono.Cecil.PE/ImageWriter.cs | 2 +- Mono.Cecil/AssemblyWriter.cs | 15 +++++- Test/Mono.Cecil.Tests/FieldTests.cs | 44 ++++++++++++++++ Test/Resources/il/FieldRVAAlignment.il | 71 ++++++++++++++++++++++++++ 5 files changed, 147 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..b32dd43ef 100644 --- a/Mono.Cecil.Metadata/Buffers.cs +++ b/Mono.Cecil.Metadata/Buffers.cs @@ -256,17 +256,33 @@ 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--; + // Compute the number of bytes to align the current position. + // Values of 0 will be written. + 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 a8a3fa82b..d6fb36335 100644 --- a/Mono.Cecil.PE/ImageWriter.cs +++ b/Mono.Cecil.PE/ImageWriter.cs @@ -694,7 +694,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 3f17a828c..65151517f 100644 --- a/Mono.Cecil/AssemblyWriter.cs +++ b/Mono.Cecil/AssemblyWriter.cs @@ -1613,8 +1613,21 @@ 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. 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.Module == module) && (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)