Skip to content
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 @@ -59,10 +59,13 @@ private void IndexReferences()
// The OrderBy is used to ensure that we by default select the highest version number.
foreach (var info in assemblyInfoByFileName.Values
.OrderBy(info => info.Name)
.ThenBy(info => info.NetCoreVersion ?? emptyVersion)
.ThenBy(info => info.Version ?? emptyVersion))
{
foreach (var index in info.IndexStrings)
{
assemblyInfoById[index] = info;
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@
using System.Reflection;
using System.Security.Cryptography;
using System.Text;
using System.Reflection.Metadata;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Horrible nitpick: Format Usings 😄 (I will show myself out)

using System.Text.RegularExpressions;

namespace Semmle.Extraction.CSharp.DependencyFetching
{
/// <summary>
/// Stores information about an assembly file (DLL).
/// </summary>
internal sealed class AssemblyInfo
internal sealed partial class AssemblyInfo
{
/// <summary>
/// The file containing the assembly.
Expand All @@ -28,6 +30,17 @@ internal sealed class AssemblyInfo
/// </summary>
public System.Version? Version { get; }

/// <summary>
/// The version number of the .NET Core framework that this assembly targets.
///
/// This is extracted from the `TargetFrameworkAttribute` of the assembly, e.g.
/// ```
/// [assembly:TargetFramework(".NETCoreApp,Version=v7.0")]
/// ```
/// yields version 7.0.
/// </summary>
public Version? NetCoreVersion { get; }

/// <summary>
/// The public key token of the assembly.
/// </summary>
Expand Down Expand Up @@ -97,13 +110,14 @@ private AssemblyInfo(string id, string filename)
Filename = filename;
}

private AssemblyInfo(string filename, string name, Version version, string culture, string publicKeyToken)
private AssemblyInfo(string filename, string name, Version version, string culture, string publicKeyToken, Version? netCoreVersion)
{
Filename = filename;
Name = name;
Version = version;
Culture = culture;
PublicKeyToken = publicKeyToken;
NetCoreVersion = netCoreVersion;
}

/// <summary>
Expand Down Expand Up @@ -150,7 +164,7 @@ public static AssemblyInfo ReadFromFile(string filename)
var metadata = pereader.GetMetadata();
unsafe
{
var reader = new System.Reflection.Metadata.MetadataReader(metadata.Pointer, metadata.Length);
var reader = new MetadataReader(metadata.Pointer, metadata.Length);
var def = reader.GetAssemblyDefinition();

// This is how you compute the public key token from the full public key.
Expand All @@ -162,7 +176,39 @@ public static AssemblyInfo ReadFromFile(string filename)
publicKeyString.AppendFormat("{0:x2}", b);

var culture = def.Culture.IsNil ? "neutral" : reader.GetString(def.Culture);
return new AssemblyInfo(filename, reader.GetString(def.Name), def.Version, culture, publicKeyString.ToString());
Version? netCoreVersion = null;

foreach (var attrHandle in def.GetCustomAttributes().Select(reader.GetCustomAttribute))
{
var ctorHandle = attrHandle.Constructor;
if (ctorHandle.Kind != HandleKind.MemberReference)
{
continue;
}

var mHandle = reader.GetMemberReference((MemberReferenceHandle)ctorHandle).Parent;
if (mHandle.Kind != HandleKind.TypeReference)
{
continue;
}

var name = reader.GetString(reader.GetTypeReference((TypeReferenceHandle)mHandle).Name);

if (name is "TargetFrameworkAttribute")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this pattern matching equivalent to name == "TargetFrameworkAttribute"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

{
var decoded = attrHandle.DecodeValue(new DummyAttributeDecoder());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this DecodeValue call be wrapped in a try-catch? Can the DummyAttributeDecoder throw NotImplementedExceptions?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would actually like to be made aware if any of the NotImplementedExceptions are thrown.

Copy link
Copy Markdown
Contributor

@michaelnebel michaelnebel Aug 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it would be good to know if this exception is thrown, but it seems a bit harsh to propagate the exception.
Maybe it is worth logging it as debug or information?

if (
decoded.FixedArguments.Length > 0 &&
decoded.FixedArguments[0].Value is string value &&
NetCoreAppRegex().Match(value).Groups.TryGetValue("version", out var match))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This NetCoreAppRegex() call is inside a foreach. I'm not sure if there's a performance hit calling this multiple times.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would think not, since the regex is RegexOptions.Compiled?

{
netCoreVersion = new Version(match.Value);
}
break;
}
}

return new AssemblyInfo(filename, reader.GetString(def.Name), def.Version, culture, publicKeyString.ToString(), netCoreVersion);
}
}
catch (BadImageFormatException)
Expand All @@ -176,5 +222,33 @@ public static AssemblyInfo ReadFromFile(string filename)

throw new AssemblyLoadException();
}

[GeneratedRegex(@"^\.NETCoreApp,Version=v(?<version>\d+\.\d+)$", RegexOptions.IgnoreCase | RegexOptions.Compiled | RegexOptions.Singleline)]
private static partial Regex NetCoreAppRegex();

private class DummyAttributeDecoder : ICustomAttributeTypeProvider<int>
{
public int GetPrimitiveType(PrimitiveTypeCode typeCode) => 0;

public int GetSystemType() => throw new NotImplementedException();

public int GetSZArrayType(int elementType) =>
throw new NotImplementedException();

public int GetTypeFromDefinition(MetadataReader reader, TypeDefinitionHandle handle, byte rawTypeKind) =>
throw new NotImplementedException();

public int GetTypeFromReference(MetadataReader reader, TypeReferenceHandle handle, byte rawTypeKind) =>
throw new NotImplementedException();

public int GetTypeFromSerializedName(string name) =>
throw new NotImplementedException();

public PrimitiveTypeCode GetUnderlyingEnumType(int type) =>
throw new NotImplementedException();

public bool IsSystemType(int type) => throw new NotImplementedException();

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -122,12 +122,12 @@ public DependencyManager(string srcDir, IDependencyOptions options, ILogger logg
ResolveConflicts();

// Output the findings
foreach (var r in usedReferences.Keys)
foreach (var r in usedReferences.Keys.OrderBy(r => r))
{
progressMonitor.ResolvedReference(r);
}

foreach (var r in unresolvedReferences)
foreach (var r in unresolvedReferences.OrderBy(r => r.Key))
{
progressMonitor.UnresolvedReference(r.Key, r.Value);
}
Expand Down Expand Up @@ -232,7 +232,8 @@ private void ResolveConflicts()
}
}

sortedReferences = sortedReferences.OrderBy(r => r.Version).ToList();
var emptyVersion = new Version(0, 0);
sortedReferences = sortedReferences.OrderBy(r => r.NetCoreVersion ?? emptyVersion).ThenBy(r => r.Version ?? emptyVersion).ToList();

var finalAssemblyList = new Dictionary<string, AssemblyInfo>();

Expand All @@ -253,9 +254,9 @@ private void ResolveConflicts()
foreach (var r in sortedReferences)
{
var resolvedInfo = finalAssemblyList[r.Name];
if (resolvedInfo.Version != r.Version)
if (resolvedInfo.Version != r.Version || resolvedInfo.NetCoreVersion != r.NetCoreVersion)
{
progressMonitor.ResolvedConflict(r.Id, resolvedInfo.Id);
progressMonitor.ResolvedConflict(r.Id, resolvedInfo.Id + resolvedInfo.NetCoreVersion is null ? "" : $" (.NET Core {resolvedInfo.NetCoreVersion})");
++conflictedReferences;
}
}
Expand Down