-
Notifications
You must be signed in to change notification settings - Fork 518
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
[runtime] Store assemblies' MVID in the generated static registrar code. #15795
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
229f9c3
[runtime] Store assemblies' MVID in the generated static registrar code.
rolfbjarne 072efca
Update generated designer file.
rolfbjarne c08d00c
Merge branch 'main' into registrar-store-mvid
rolfbjarne f15c607
Make validation opt-in.
rolfbjarne b3c59b7
REDO VALIDATION TO BE PER-ASSEMBLY.
rolfbjarne 2efc4c3
Rework verification to be per-assembly.
rolfbjarne 75076d3
Adjust test now that we store MVIDs in generated registrar code.
rolfbjarne File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ | |
#nullable enable | ||
|
||
using System; | ||
using System.Diagnostics.CodeAnalysis; | ||
using System.Reflection; | ||
using System.Collections.Generic; | ||
using System.Runtime.InteropServices; | ||
|
@@ -19,6 +20,10 @@ | |
using Registrar; | ||
#endif | ||
|
||
#if !COREBUILD | ||
using Xamarin.Bundler; | ||
#endif | ||
|
||
#if !NET | ||
using NativeHandle = System.IntPtr; | ||
#endif | ||
|
@@ -56,8 +61,8 @@ internal unsafe static void Initialize (Runtime.InitializationOptions* options) | |
|
||
|
||
for (int i = 0; i < map->assembly_count; i++) { | ||
var ptr = Marshal.ReadIntPtr (map->assembly, i * IntPtr.Size); | ||
Runtime.Registrar.SetAssemblyRegistered (Marshal.PtrToStringAuto (ptr)); | ||
var assembly = map->assemblies [i]; | ||
Runtime.Registrar.SetAssemblyRegistered (Marshal.PtrToStringAuto (assembly.name)); | ||
} | ||
} | ||
|
||
|
@@ -292,27 +297,27 @@ unsafe static bool CompareTokenReference (string asm_name, int mod_token, int ty | |
if ((token_reference & 0x1) == 0x1) { | ||
// full token reference | ||
var idx = (int) (token_reference >> 1); | ||
var entry = map->full_token_references + (IntPtr.Size + 8) * idx; | ||
var entry = map->full_token_references [idx]; | ||
// first compare what's most likely to fail (the type's metadata token) | ||
var token = (uint) Marshal.ReadInt32 (entry + IntPtr.Size + 4); | ||
var token = entry.token; | ||
type_token |= 0x02000000 /* TypeDef - the token type is explicit in the full token reference, but not present in the type_token argument, so we have to add it before comparing */; | ||
if (type_token != token) | ||
return false; | ||
|
||
// then the module token | ||
var module_token = (uint) Marshal.ReadInt32 (entry + IntPtr.Size); | ||
var module_token = entry.module_token; | ||
if (mod_token != module_token) | ||
return false; | ||
|
||
// leave the assembly name for the end, since it's the most expensive comparison (string comparison) | ||
assembly_name = Marshal.ReadIntPtr (entry); | ||
assembly_name = map->assemblies [entry.assembly_index].name; | ||
} else { | ||
// packed token reference | ||
if (token_reference >> 8 != type_token) | ||
return false; | ||
|
||
var assembly_index = (token_reference >> 1) & 0x7F; | ||
assembly_name = Marshal.ReadIntPtr (map->assembly, (int) assembly_index * IntPtr.Size); | ||
assembly_name = map->assemblies [(int) assembly_index].name; | ||
} | ||
|
||
return Runtime.StringEquals (assembly_name, asm_name); | ||
|
@@ -380,10 +385,11 @@ static unsafe int FindMapIndex (Runtime.MTClassMap *array, int lo, int hi, IntPt | |
internal unsafe static MemberInfo? ResolveFullTokenReference (uint token_reference) | ||
{ | ||
// sizeof (MTFullTokenReference) = IntPtr.Size + 4 + 4 | ||
var entry = Runtime.options->RegistrationMap->full_token_references + (IntPtr.Size + 8) * (int) (token_reference >> 1); | ||
var assembly_name = Marshal.ReadIntPtr (entry); | ||
var module_token = (uint) Marshal.ReadInt32 (entry + IntPtr.Size); | ||
var token = (uint) Marshal.ReadInt32 (entry + IntPtr.Size + 4); | ||
var idx = (int) (token_reference >> 1); | ||
var entry = Runtime.options->RegistrationMap->full_token_references [idx]; | ||
var assembly_name = Runtime.options->RegistrationMap->assemblies [entry.assembly_index].name; | ||
var module_token = entry.module_token; | ||
var token = entry.token; | ||
|
||
#if LOG_TYPELOAD | ||
Console.WriteLine ($"ResolveFullTokenReference (0x{token_reference:X}) assembly name: {assembly_name} module token: 0x{module_token:X} token: 0x{token:X}."); | ||
|
@@ -430,7 +436,7 @@ static unsafe int FindMapIndex (Runtime.MTClassMap *array, int lo, int hi, IntPt | |
Console.WriteLine ($"ResolveTokenReference (0x{token_reference:X}) assembly index: {assembly_index} token: 0x{token:X}."); | ||
#endif | ||
|
||
var assembly_name = Marshal.ReadIntPtr (map->assembly, (int) assembly_index * IntPtr.Size); | ||
var assembly_name = map->assemblies [(int) assembly_index].name; | ||
var assembly = ResolveAssembly (assembly_name); | ||
var module = ResolveModule (assembly, 0x1); | ||
|
||
|
@@ -474,20 +480,80 @@ static Module ResolveModule (Assembly assembly, uint token) | |
throw ErrorHelper.CreateError (8020, $"Could not find the module with MetadataToken 0x{token:X} in the assembly {assembly}."); | ||
} | ||
|
||
// Restrict this code to desktop for now, which is where most of the problems with outdated generated static registrar code occur. | ||
#if __MACOS__ || __MACCATALYST__ | ||
static bool? verify_static_registrar_code; | ||
static object? verification_lock; | ||
static Dictionary<IntPtr, object?>? verified_assemblies; // Use Dictionary instead of HashSet to avoid pulling in System.Core.dll. | ||
unsafe static void VerifyStaticRegistrarCode (IntPtr assembly_name, Assembly assembly) | ||
{ | ||
if (verify_static_registrar_code is null) { | ||
verify_static_registrar_code = !string.IsNullOrEmpty (Environment.GetEnvironmentVariable ("XAMARIN_VALIDATE_STATIC_REGISTRAR_CODE")); | ||
verification_lock = new object (); | ||
} | ||
if (verify_static_registrar_code != true) | ||
return; | ||
|
||
lock (verification_lock!) { | ||
if (verified_assemblies is null) { | ||
verified_assemblies = new Dictionary<IntPtr, object?> (Runtime.IntPtrEqualityComparer); | ||
} else if (verified_assemblies.ContainsKey (assembly_name)) { | ||
return; | ||
} | ||
verified_assemblies [assembly_name] = null; | ||
} | ||
|
||
var map = Runtime.options->RegistrationMap; | ||
if (map is null) | ||
return; | ||
|
||
for (var i = 0; i < map->assembly_count; i++) { | ||
var entry = map->assemblies [i]; | ||
var name = Marshal.PtrToStringAuto (entry.name)!; | ||
if (!Runtime.StringEquals (assembly_name, name)) | ||
continue; | ||
try { | ||
var mvid = Marshal.PtrToStringAuto (entry.mvid)!; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's theoretically possible to construct the GUID from a ReadOnlySpan (although possibly not on mono?) |
||
var runtime_mvid = assembly.ManifestModule.ModuleVersionId; | ||
var registered_mvid = Guid.Parse (mvid); | ||
if (registered_mvid == runtime_mvid) | ||
continue; | ||
throw ErrorHelper.CreateError (8044, Errors.MX8044 /* The assembly {0} has been modified since the app was built, invalidating the generated static registrar code. The MVID for the loaded assembly is {1}, while the MVID for the assembly the generated static registrar code corresponds to is {2}. */, name, runtime_mvid, registered_mvid); | ||
} catch (Exception e) { | ||
throw ErrorHelper.CreateError (8043, e, Errors.MX8043 /* An exception occurred while validating the static registrar code for {0}: {1} */, name, e.Message); | ||
} | ||
} | ||
} | ||
#endif // __MACOS__ || __MACCATALYST__ | ||
|
||
static Assembly ResolveAssembly (IntPtr assembly_name) | ||
{ | ||
if (TryResolveAssembly (assembly_name, out var asm)) { | ||
#if __MACOS__ || __MACCATALYST__ | ||
VerifyStaticRegistrarCode (assembly_name, asm); | ||
#endif | ||
return asm; | ||
} | ||
|
||
throw ErrorHelper.CreateError (8019, $"Could not find the assembly {Marshal.PtrToStringAuto (assembly_name)} in the loaded assemblies."); | ||
} | ||
|
||
static bool TryResolveAssembly (IntPtr assembly_name, [NotNullWhen (true)] out Assembly? assembly) | ||
{ | ||
// Find the assembly. We've already loaded all the assemblies that contain registered types, so just look at those assemblies. | ||
foreach (var asm in AppDomain.CurrentDomain.GetAssemblies ()) { | ||
if (!Runtime.StringEquals (assembly_name, asm.GetName ().Name)) | ||
continue; | ||
|
||
#if LOG_TYPELOAD | ||
Console.WriteLine ($"ResolveAssembly (0x{assembly_name:X}): {asm.FullName}."); | ||
Console.WriteLine ($"TryResolveAssembly (0x{assembly_name:X}): {asm.FullName}."); | ||
#endif | ||
return asm; | ||
assembly = asm; | ||
return true; | ||
} | ||
|
||
throw ErrorHelper.CreateError (8019, $"Could not find the assembly {Marshal.PtrToStringAuto (assembly_name)} in the loaded assemblies."); | ||
assembly = null; | ||
return false; | ||
} | ||
|
||
internal unsafe static uint GetTokenReference (Type type, bool throw_exception = true) | ||
|
@@ -514,7 +580,7 @@ internal unsafe static uint GetTokenReference (Type type, bool throw_exception = | |
// Find the assembly index in our list of registered assemblies. | ||
int assembly_index = -1; | ||
for (int i = 0; i < map->assembly_count; i++) { | ||
var name_ptr = Marshal.ReadIntPtr (map->assembly, (int) i * IntPtr.Size); | ||
var name_ptr = map->assemblies [(int) i].name; | ||
if (Runtime.StringEquals (name_ptr, asm_name)) { | ||
assembly_index = i; | ||
break; | ||
|
@@ -542,15 +608,16 @@ static unsafe uint GetFullTokenReference (string assembly_name, int module_token | |
{ | ||
var map = Runtime.options->RegistrationMap; | ||
for (int i = 0; i < map->full_token_reference_count; i++) { | ||
var ptr = map->full_token_references + (i * (IntPtr.Size + 8)); | ||
var asm_ptr = Marshal.ReadIntPtr (ptr); | ||
var token = Marshal.ReadInt32 (ptr + IntPtr.Size + 4); | ||
var ftr = map->full_token_references [i]; | ||
var token = ftr.token; | ||
if (token != metadata_token) | ||
continue; | ||
var mod_token = Marshal.ReadInt32 (ptr + IntPtr.Size); | ||
var mod_token = ftr.module_token; | ||
if (mod_token != module_token) | ||
continue; | ||
if (!Runtime.StringEquals (asm_ptr, assembly_name)) | ||
var assembly_index = ftr.assembly_index; | ||
var assembly = map->assemblies [assembly_index]; | ||
if (!Runtime.StringEquals (assembly.name, assembly_name)) | ||
continue; | ||
|
||
return ((uint) i << 1) + 1; | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
using System; | ||
using System.Runtime.InteropServices; | ||
|
||
using Foundation; | ||
using ObjCRuntime; | ||
|
||
namespace MySimpleApp | ||
{ | ||
public class Program | ||
{ | ||
static int Main (string[] args) | ||
{ | ||
Console.WriteLine (Environment.GetEnvironmentVariable ("MAGIC_WORD")); | ||
|
||
#if INCLUDED_ADDITIONAL_CODE | ||
GC.KeepAlive (typeof (AdditionalClass)); | ||
#endif | ||
|
||
return StaticRegistrarValidationTest (); | ||
} | ||
|
||
static int StaticRegistrarValidationTest () | ||
{ | ||
try { | ||
using var obj = new SomeObj (); | ||
obj.Whatever (); | ||
xamarin_IntPtr_objc_msgSend_IntPtr_ref_IntPtr_exception (obj.Handle, Selector.GetHandle ("whatever"), IntPtr.Zero, IntPtr.Zero, out var gchandle); | ||
Console.WriteLine ($"GCH: {gchandle}"); | ||
if (gchandle != IntPtr.Zero) { | ||
var gch = GCHandle.FromIntPtr (gchandle); | ||
var exc = (Exception) gch.Target; | ||
gch.Free (); | ||
throw exc; | ||
} | ||
return 1; // We're not supposed to get here | ||
} catch (Exception e) { | ||
Console.WriteLine ($"E: {e}"); | ||
if (e.Message.Contains ("The assembly MyRegistrarApp has been modified since the app was built, invalidating the generated static registrar code.")) | ||
return 0; | ||
return 2; | ||
} | ||
} | ||
|
||
[DllImport ("__Internal")] | ||
static extern IntPtr xamarin_IntPtr_objc_msgSend_IntPtr_ref_IntPtr_exception (IntPtr handle, IntPtr selector, IntPtr p0, IntPtr p1, out IntPtr gchandle); | ||
} | ||
|
||
public class SomeObj : NSObject | ||
{ | ||
[Export ("whatever")] | ||
public IntPtr Whatever () | ||
{ | ||
return new IntPtr (0xdeadf00d); | ||
} | ||
} | ||
|
||
public class DeadClass {} // Some code for the linker to remove | ||
|
||
#if INCLUDED_ADDITIONAL_CODE | ||
public class AdditionalClass { | ||
} | ||
#endif | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
include ../shared.mk |
7 changes: 7 additions & 0 deletions
7
tests/dotnet/MyRegistrarApp/MacCatalyst/MyRegistrarApp.csproj
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
<PropertyGroup> | ||
<TargetFramework>net$(BundledNETCoreAppTargetFrameworkVersion)-maccatalyst</TargetFramework> | ||
</PropertyGroup> | ||
<Import Project="..\shared.csproj" /> | ||
</Project> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
TOP=../../.. | ||
include $(TOP)/tests/common/shared-dotnet-test.mk |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
include ../shared.mk |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
<PropertyGroup> | ||
<TargetFramework>net$(BundledNETCoreAppTargetFrameworkVersion)-ios</TargetFramework> | ||
</PropertyGroup> | ||
<Import Project="..\shared.csproj" /> | ||
</Project> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
include ../shared.mk |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
<PropertyGroup> | ||
<TargetFramework>net$(BundledNETCoreAppTargetFrameworkVersion)-macos</TargetFramework> | ||
</PropertyGroup> | ||
<Import Project="..\shared.csproj" /> | ||
</Project> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<Project> | ||
<PropertyGroup> | ||
<OutputType>Exe</OutputType> | ||
|
||
<ApplicationTitle>MyRegistrarApp</ApplicationTitle> | ||
<ApplicationId>com.xamarin.myregistrarapp</ApplicationId> | ||
|
||
<ExcludeNUnitLiteReference>true</ExcludeNUnitLiteReference> | ||
<ExcludeTouchUnitReference>true</ExcludeTouchUnitReference> | ||
|
||
<DefineConstants>$(DefineConstants);$(AdditionalDefineConstants)</DefineConstants> | ||
</PropertyGroup> | ||
|
||
<Import Project="../../common/shared-dotnet.csproj" /> | ||
|
||
<ItemGroup> | ||
<Compile Include="../*.cs" /> | ||
</ItemGroup> | ||
</Project> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
TOP=../../../.. | ||
TESTNAME=MyRegistrarApp | ||
include $(TOP)/tests/common/shared-dotnet.mk |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
include ../shared.mk |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused here a bit, why not just compare the two byte sequences?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't need to marshal the string to do the comparison, I think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because I couldn't think of any built-in method to do that, and I didn't feel like writing a new implementation either :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, in case you want to, I think SequenceEqual can be used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main annoyance with that is that I don't have the string length, so I could either P/Invoke strlen (which isn't necessarily cheap), or write my own routine (in which case I'd just write a string-comparing routine instead of a string-length computation routine. Not a big issue, but this will happen once per assembly, and it's opt-in, so I don't think it's all that important.