Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relax constraints on duplicated members in metadata buffers #478

Merged
merged 4 commits into from
Aug 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public MetadataToken AddTypeReference(TypeReference? type, bool allowDuplicates,
Metadata.StringsStream.GetStringIndex(type.Name),
Metadata.StringsStream.GetStringIndex(type.Namespace));

var token = preserveRid
var token = preserveRid && type.MetadataToken.Rid != 0
? table.Insert(type.MetadataToken.Rid, row, allowDuplicates)
: table.Add(row, allowDuplicates);

Expand Down Expand Up @@ -205,7 +205,7 @@ public MetadataToken AddAssemblyReference(AssemblyReference? assembly, bool allo
Metadata.StringsStream.GetStringIndex(assembly.Culture),
Metadata.BlobStream.GetBlobIndex(assembly.HashValue));

var token = preserveRid
var token = preserveRid && assembly.MetadataToken.Rid != 0
? table.Insert(assembly.MetadataToken.Rid, row, allowDuplicates)
: table.Add(row, allowDuplicates);

Expand Down Expand Up @@ -243,7 +243,7 @@ public MetadataToken AddModuleReference(ModuleReference? reference, bool allowDu
var table = Metadata.TablesStream.GetDistinctTable<ModuleReferenceRow>(TableIndex.ModuleRef);

var row = new ModuleReferenceRow(Metadata.StringsStream.GetStringIndex(reference.Name));
var token = preserveRid
var token = preserveRid && reference.MetadataToken.Rid != 0
? table.Insert(reference.MetadataToken.Rid, row, allowDuplicates)
: table.Add(row, allowDuplicates);

Expand Down
18 changes: 3 additions & 15 deletions src/AsmResolver.DotNet/Builder/DotNetDirectoryFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,6 @@ private IMetadataBuffer CreateMetadataBuffer(ModuleDefinition module)

private void ImportBasicTablesIfSpecified(ModuleDefinition module, DotNetDirectoryBuffer buffer)
{
if (module.DotNetDirectory is null)
return;

// NOTE: The order of this table importing is crucial.
//
// Assembly refs should always be imported prior to importing type refs, which should be imported before
Expand Down Expand Up @@ -246,9 +243,6 @@ private void ImportBasicTablesIfSpecified(ModuleDefinition module, DotNetDirecto

private void ImportTypeSpecsIfSpecified(ModuleDefinition module, DotNetDirectoryBuffer buffer)
{
if (module.DotNetDirectory is null)
return;

if ((MetadataBuilderFlags & MetadataBuilderFlags.PreserveTypeSpecificationIndices) != 0)
{
ImportTables<TypeSpecification>(module, TableIndex.TypeSpec,
Expand All @@ -258,9 +252,6 @@ private void ImportTypeSpecsIfSpecified(ModuleDefinition module, DotNetDirectory

private void ImportMemberRefsIfSpecified(ModuleDefinition module, DotNetDirectoryBuffer buffer)
{
if (module.DotNetDirectory is null)
return;

if ((MetadataBuilderFlags & MetadataBuilderFlags.PreserveMemberReferenceIndices) != 0)
{
ImportTables<MemberReference>(module, TableIndex.MemberRef,
Expand All @@ -270,9 +261,6 @@ private void ImportMemberRefsIfSpecified(ModuleDefinition module, DotNetDirector

private void ImportRemainingTablesIfSpecified(ModuleDefinition module, DotNetDirectoryBuffer buffer)
{
if (module.DotNetDirectory is null)
return;

if ((MetadataBuilderFlags & MetadataBuilderFlags.PreserveStandAloneSignatureIndices) != 0)
{
ImportTables<StandAloneSignature>(module, TableIndex.StandAloneSig,
Expand All @@ -289,10 +277,10 @@ private void ImportRemainingTablesIfSpecified(ModuleDefinition module, DotNetDir
private static void ImportTables<TMember>(ModuleDefinition module, TableIndex tableIndex,
Func<TMember, MetadataToken> importAction)
{
int count = module.DotNetDirectory!.Metadata
!.GetStream<TablesStream>()
int count = module.DotNetDirectory?.Metadata?
.GetStream<TablesStream>()
.GetTable(tableIndex)
.Count;
.Count ?? 0;

for (uint rid = 1; rid <= count; rid++)
importAction((TMember) module.LookupMember(new MetadataToken(tableIndex, rid)));
Expand Down
12 changes: 7 additions & 5 deletions src/AsmResolver.DotNet/Builder/TokenMapping.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,14 @@ public void Register(IMetadataMember member, MetadataToken newToken)
if (member.MetadataToken.Table != newToken.Table)
throw new ArgumentException($"Cannot assign a {newToken.Table} metadata token to a {member.MetadataToken.Table}.");

if (TryGetNewToken(member, out var existingToken))
{
if (existingToken.Rid != newToken.Rid)
throw new ArgumentException($"Member {member.SafeToString()} was already assigned a metadata token.");
// We allow for members to be duplicated in the mapping, but we will only keep track of the first token that
// was registered for this member. This may result in a slightly different binary if token preservation is
// enabled (e.g., a member may originally have referenced a duplicated member as opposed to the first one,
// and this would always make it reference the first one), but since both members are semantically equivalent,
// referencing one or the other should also be semantics-preserving. Note that both duplicated members will
// still be present in the final binary with their original RIDs.
if (TryGetNewToken(member, out _))
return;
}

switch (member.MetadataToken.Table)
{
Expand Down
6 changes: 3 additions & 3 deletions src/AsmResolver/Collections/BitList.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,15 @@ public bool this[int index]
{
get
{
if (index >= Count)
if (index < 0 || index >= Count)
throw new IndexOutOfRangeException();

(int wordIndex, int bitIndex) = SplitWordBitIndex(index);
return (_words[wordIndex] >> bitIndex & 1) != 0;
}
set
{
if (index >= Count)
if (index < 0 || index >= Count)
throw new IndexOutOfRangeException();

(int wordIndex, int bitIndex) = SplitWordBitIndex(index);
Expand Down Expand Up @@ -119,7 +119,7 @@ public int IndexOf(bool item)
/// <inheritdoc />
public void Insert(int index, bool item)
{
if (index > Count)
if (index < 0 || index > Count)
throw new IndexOutOfRangeException();

EnsureCapacity(Count++);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,36 @@ public void PreserveDuplicatedTypeRefs()
newObjectReferences.Select(r => r.MetadataToken).ToHashSet());
}

[Fact]
public void PreserveDuplicatedTypeRefsInBaseType()
{
// Prepare temp module with two references to System.Object
var module = new ModuleDefinition("Test");
var assembly = new AssemblyDefinition("Test", new Version(1, 0, 0, 0));
assembly.Modules.Add(module);

var ref1 = (TypeReference) module.CorLibTypeFactory.CorLibScope
.CreateTypeReference("System", "Object")
.ImportWith(module.DefaultImporter);
var ref2 = (TypeReference) module.CorLibTypeFactory.CorLibScope
.CreateTypeReference("System", "Object")
.ImportWith(module.DefaultImporter);

// Force assign new tokens to instruct builder that both type references need to be added.
module.TokenAllocator.AssignNextAvailableToken(ref1);
module.TokenAllocator.AssignNextAvailableToken(ref2);

module.TopLevelTypes.Add(new TypeDefinition(null, "A", TypeAttributes.Public, ref1));
module.TopLevelTypes.Add(new TypeDefinition(null, "B", TypeAttributes.Public, ref2));

// Rebuild.
var image = module.ToPEImage(new ManagedPEImageBuilder(MetadataBuilderFlags.PreserveTypeReferenceIndices));

// Verify that both object references are still there.
var newModule = ModuleDefinition.FromImage(image);
Assert.Equal(2, newModule.GetImportedTypeReferences().Count(t => t.Name == "Object"));
}

[Fact]
public void PreserveNestedTypeRefOrdering()
{
Expand All @@ -113,5 +143,6 @@ public void PreserveNestedTypeRefOrdering()

Assert.Equal(originalTypeRefs, newTypeRefs.Take(originalTypeRefs.Count), Comparer);
}

}
}