Merge changes from upstream by sbomer · Pull Request #186 · dotnet/cecil

sbomer · 2024-06-05T22:59:35Z

This brings in the latest cecil changes from upstream. Importantly, includes:

Changes for ByRefLike constrains (needed for Validate ILLink support for ref structs within generics runtime#98519)
Updates the version of NUnit that doesn't throw due to unknown framework version in some of our tests.
- See RunTests target hides exception thrown during NUnit fixture setup arcade#14812 for context.
- This is why I was seeing some test failures in Preserve original PDB path in Debug header jbevain/cecil#947 but not Preserve original PDB path in Debug header #183.

…ion (jbevain#882) When all other comparisons (return type, parameters, etc.) are the same, treat instance and static methods with the same name as different methods. This corrects a problem we noticed in IL2CPP in Unity.

…vain#879) During `AssemblyReader.ReadCustomAttributes` there is a call to `WindowsRuntimeProjections.Project` ``` if (module.IsWindowsMetadata ()) foreach (var custom_attribute in custom_attributes) WindowsRuntimeProjections.Project (owner, custom_attributes, custom_attribute); ``` `WindowsRuntimeProjections.Project` would call `WindowsRuntimeProjections.HasAttribute`, which would then call `type.CustomAttributes`, which would end up back in `AssemblyReader.ReadCustomAttributes`. This would lead to a StackOverflowException. This wasn't an issue previously. My PR jbevain#843 caused this sequence of calls to start resulting in a StackOverflowException. Prior to my PR, there was a call to `metadata.RemoveCustomAttributeRange (owner);` before the call to `WindowsRuntimeProjections.Project`. This meant that when `WindowsRuntimeProjections.HasAttribute` would call `type.CustomAttributes`, we'd still end up in `AssemblyReader.ReadCustomAttributes`, however, no attributes would be found because the following if would be true and lead to returning an empty collection. ``` if (!metadata.TryGetCustomAttributeRanges (owner, out ranges)) return new Collection<CustomAttribute> (); ``` The old behavior was probably the wrong. Although I'm not certain what the tangible impact was. The fix was pretty easy. `AssemblyReader.ReadCustomAttributes` will now pass in the custom attributes to `WindowsRuntimeProjections.Project` avoiding the need to call `type.CustomAttributes`

… the same (jbevain#885)

* Fix RVA field alignment * Use non-latest images to run tests * Revert windows-2019 change * Don't align code segment This wasn't aligned to begin with. Attempting to align it causes problems due to the way CodeWriter gets the code RVA - it computes the RVA from the CLI Header length, which doesn't take into account the code's alignment requirements. * Revert "Don't align code segment" This reverts commit 9410f1e. * Keep alignment values for code

With MSBuild 7.0.201 and later, building the Mono.Cecil projects used by Unity's IL2CPP causes warning NU1605 to occur, indicating that there is a package version mismatch between the .NET Framework reference assemblies package used. To avoid this, bump the version used by Mono.Cecil and similar project to be the latest, version 1.0.3. Note that MSBuild version 7.0.100 and earlier did not cause this warning to occur.

Co-authored-by: Marek Safar <marek.safar@gmail.com>

When resolving a `GenericInstanceMethod` for a `privatescope` method that has the same signature as other methods, `MetadataResolver.GetMethod` would incorrectly return the first method with the same name. The fix for this seems to be an optimization opportunity as well. Although I have to admit it feels a little to easy. Please make sure I'm not overlooking some reason why this fix is not safe. A added 2 variations of tests. `PrivateScope` - These tests were fine and passed as is. This is because the instruction operands are MethodDefinitions and therefore didn't need to be resolved by `MetadataResolver` `PrivateScopeGeneric` - This test triggered the bug.

* Add new flag for ByRefLike constraints * Allow seems to be the prefered nomenclature

sbomer · 2024-06-05T22:59:35Z

This brings in the latest cecil changes from upstream. Importantly, includes:

Changes for ByRefLike constrains (needed for Validate ILLink support for ref structs within generics runtime#98519)
Updates the version of NUnit that doesn't throw due to unknown framework version in some of our tests.
- See RunTests target hides exception thrown during NUnit fixture setup arcade#14812 for context.
- This is why I was seeing some test failures in Preserve original PDB path in Debug header jbevain/cecil#947 but not Preserve original PDB path in Debug header #183.

…ion (jbevain#882) When all other comparisons (return type, parameters, etc.) are the same, treat instance and static methods with the same name as different methods. This corrects a problem we noticed in IL2CPP in Unity.

…vain#879) During `AssemblyReader.ReadCustomAttributes` there is a call to `WindowsRuntimeProjections.Project` ``` if (module.IsWindowsMetadata ()) foreach (var custom_attribute in custom_attributes) WindowsRuntimeProjections.Project (owner, custom_attributes, custom_attribute); ``` `WindowsRuntimeProjections.Project` would call `WindowsRuntimeProjections.HasAttribute`, which would then call `type.CustomAttributes`, which would end up back in `AssemblyReader.ReadCustomAttributes`. This would lead to a StackOverflowException. This wasn't an issue previously. My PR jbevain#843 caused this sequence of calls to start resulting in a StackOverflowException. Prior to my PR, there was a call to `metadata.RemoveCustomAttributeRange (owner);` before the call to `WindowsRuntimeProjections.Project`. This meant that when `WindowsRuntimeProjections.HasAttribute` would call `type.CustomAttributes`, we'd still end up in `AssemblyReader.ReadCustomAttributes`, however, no attributes would be found because the following if would be true and lead to returning an empty collection. ``` if (!metadata.TryGetCustomAttributeRanges (owner, out ranges)) return new Collection<CustomAttribute> (); ``` The old behavior was probably the wrong. Although I'm not certain what the tangible impact was. The fix was pretty easy. `AssemblyReader.ReadCustomAttributes` will now pass in the custom attributes to `WindowsRuntimeProjections.Project` avoiding the need to call `type.CustomAttributes`

… the same (jbevain#885)

* Fix RVA field alignment * Use non-latest images to run tests * Revert windows-2019 change * Don't align code segment This wasn't aligned to begin with. Attempting to align it causes problems due to the way CodeWriter gets the code RVA - it computes the RVA from the CLI Header length, which doesn't take into account the code's alignment requirements. * Revert "Don't align code segment" This reverts commit 9410f1e. * Keep alignment values for code

With MSBuild 7.0.201 and later, building the Mono.Cecil projects used by Unity's IL2CPP causes warning NU1605 to occur, indicating that there is a package version mismatch between the .NET Framework reference assemblies package used. To avoid this, bump the version used by Mono.Cecil and similar project to be the latest, version 1.0.3. Note that MSBuild version 7.0.100 and earlier did not cause this warning to occur.

Co-authored-by: Marek Safar <marek.safar@gmail.com>

When resolving a `GenericInstanceMethod` for a `privatescope` method that has the same signature as other methods, `MetadataResolver.GetMethod` would incorrectly return the first method with the same name. The fix for this seems to be an optimization opportunity as well. Although I have to admit it feels a little to easy. Please make sure I'm not overlooking some reason why this fix is not safe. A added 2 variations of tests. `PrivateScope` - These tests were fine and passed as is. This is because the instruction operands are MethodDefinitions and therefore didn't need to be resolved by `MetadataResolver` `PrivateScopeGeneric` - This test triggered the bug.

* Add new flag for ByRefLike constraints * Allow seems to be the prefered nomenclature

sbomer · 2024-06-05T22:59:35Z

This brings in the latest cecil changes from upstream. Importantly, includes:

Changes for ByRefLike constrains (needed for Validate ILLink support for ref structs within generics runtime#98519)
Updates the version of NUnit that doesn't throw due to unknown framework version in some of our tests.
- See RunTests target hides exception thrown during NUnit fixture setup arcade#14812 for context.
- This is why I was seeing some test failures in Preserve original PDB path in Debug header jbevain/cecil#947 but not Preserve original PDB path in Debug header #183.

…ion (jbevain#882) When all other comparisons (return type, parameters, etc.) are the same, treat instance and static methods with the same name as different methods. This corrects a problem we noticed in IL2CPP in Unity.

…vain#879) During `AssemblyReader.ReadCustomAttributes` there is a call to `WindowsRuntimeProjections.Project` ``` if (module.IsWindowsMetadata ()) foreach (var custom_attribute in custom_attributes) WindowsRuntimeProjections.Project (owner, custom_attributes, custom_attribute); ``` `WindowsRuntimeProjections.Project` would call `WindowsRuntimeProjections.HasAttribute`, which would then call `type.CustomAttributes`, which would end up back in `AssemblyReader.ReadCustomAttributes`. This would lead to a StackOverflowException. This wasn't an issue previously. My PR jbevain#843 caused this sequence of calls to start resulting in a StackOverflowException. Prior to my PR, there was a call to `metadata.RemoveCustomAttributeRange (owner);` before the call to `WindowsRuntimeProjections.Project`. This meant that when `WindowsRuntimeProjections.HasAttribute` would call `type.CustomAttributes`, we'd still end up in `AssemblyReader.ReadCustomAttributes`, however, no attributes would be found because the following if would be true and lead to returning an empty collection. ``` if (!metadata.TryGetCustomAttributeRanges (owner, out ranges)) return new Collection<CustomAttribute> (); ``` The old behavior was probably the wrong. Although I'm not certain what the tangible impact was. The fix was pretty easy. `AssemblyReader.ReadCustomAttributes` will now pass in the custom attributes to `WindowsRuntimeProjections.Project` avoiding the need to call `type.CustomAttributes`

… the same (jbevain#885)

* Fix RVA field alignment * Use non-latest images to run tests * Revert windows-2019 change * Don't align code segment This wasn't aligned to begin with. Attempting to align it causes problems due to the way CodeWriter gets the code RVA - it computes the RVA from the CLI Header length, which doesn't take into account the code's alignment requirements. * Revert "Don't align code segment" This reverts commit 9410f1e. * Keep alignment values for code

With MSBuild 7.0.201 and later, building the Mono.Cecil projects used by Unity's IL2CPP causes warning NU1605 to occur, indicating that there is a package version mismatch between the .NET Framework reference assemblies package used. To avoid this, bump the version used by Mono.Cecil and similar project to be the latest, version 1.0.3. Note that MSBuild version 7.0.100 and earlier did not cause this warning to occur.

Co-authored-by: Marek Safar <marek.safar@gmail.com>

When resolving a `GenericInstanceMethod` for a `privatescope` method that has the same signature as other methods, `MetadataResolver.GetMethod` would incorrectly return the first method with the same name. The fix for this seems to be an optimization opportunity as well. Although I have to admit it feels a little to easy. Please make sure I'm not overlooking some reason why this fix is not safe. A added 2 variations of tests. `PrivateScope` - These tests were fine and passed as is. This is because the instruction operands are MethodDefinitions and therefore didn't need to be resolved by `MetadataResolver` `PrivateScopeGeneric` - This test triggered the bug.

* Add new flag for ByRefLike constraints * Allow seems to be the prefered nomenclature

sbomer · 2024-06-05T22:59:35Z

This brings in the latest cecil changes from upstream. Importantly, includes:

Changes for ByRefLike constrains (needed for Validate ILLink support for ref structs within generics runtime#98519)
Updates the version of NUnit that doesn't throw due to unknown framework version in some of our tests.
- See RunTests target hides exception thrown during NUnit fixture setup arcade#14812 for context.
- This is why I was seeing some test failures in Preserve original PDB path in Debug header jbevain/cecil#947 but not Preserve original PDB path in Debug header #183.

…ion (jbevain#882) When all other comparisons (return type, parameters, etc.) are the same, treat instance and static methods with the same name as different methods. This corrects a problem we noticed in IL2CPP in Unity.

…vain#879) During `AssemblyReader.ReadCustomAttributes` there is a call to `WindowsRuntimeProjections.Project` ``` if (module.IsWindowsMetadata ()) foreach (var custom_attribute in custom_attributes) WindowsRuntimeProjections.Project (owner, custom_attributes, custom_attribute); ``` `WindowsRuntimeProjections.Project` would call `WindowsRuntimeProjections.HasAttribute`, which would then call `type.CustomAttributes`, which would end up back in `AssemblyReader.ReadCustomAttributes`. This would lead to a StackOverflowException. This wasn't an issue previously. My PR jbevain#843 caused this sequence of calls to start resulting in a StackOverflowException. Prior to my PR, there was a call to `metadata.RemoveCustomAttributeRange (owner);` before the call to `WindowsRuntimeProjections.Project`. This meant that when `WindowsRuntimeProjections.HasAttribute` would call `type.CustomAttributes`, we'd still end up in `AssemblyReader.ReadCustomAttributes`, however, no attributes would be found because the following if would be true and lead to returning an empty collection. ``` if (!metadata.TryGetCustomAttributeRanges (owner, out ranges)) return new Collection<CustomAttribute> (); ``` The old behavior was probably the wrong. Although I'm not certain what the tangible impact was. The fix was pretty easy. `AssemblyReader.ReadCustomAttributes` will now pass in the custom attributes to `WindowsRuntimeProjections.Project` avoiding the need to call `type.CustomAttributes`

… the same (jbevain#885)

* Fix RVA field alignment * Use non-latest images to run tests * Revert windows-2019 change * Don't align code segment This wasn't aligned to begin with. Attempting to align it causes problems due to the way CodeWriter gets the code RVA - it computes the RVA from the CLI Header length, which doesn't take into account the code's alignment requirements. * Revert "Don't align code segment" This reverts commit 9410f1e. * Keep alignment values for code

With MSBuild 7.0.201 and later, building the Mono.Cecil projects used by Unity's IL2CPP causes warning NU1605 to occur, indicating that there is a package version mismatch between the .NET Framework reference assemblies package used. To avoid this, bump the version used by Mono.Cecil and similar project to be the latest, version 1.0.3. Note that MSBuild version 7.0.100 and earlier did not cause this warning to occur.

Co-authored-by: Marek Safar <marek.safar@gmail.com>

When resolving a `GenericInstanceMethod` for a `privatescope` method that has the same signature as other methods, `MetadataResolver.GetMethod` would incorrectly return the first method with the same name. The fix for this seems to be an optimization opportunity as well. Although I have to admit it feels a little to easy. Please make sure I'm not overlooking some reason why this fix is not safe. A added 2 variations of tests. `PrivateScope` - These tests were fine and passed as is. This is because the instruction operands are MethodDefinitions and therefore didn't need to be resolved by `MetadataResolver` `PrivateScopeGeneric` - This test triggered the bug.

* Add new flag for ByRefLike constraints * Allow seems to be the prefered nomenclature

sbomer · 2024-06-05T22:59:35Z

This brings in the latest cecil changes from upstream. Importantly, includes:

Changes for ByRefLike constrains (needed for Validate ILLink support for ref structs within generics runtime#98519)
Updates the version of NUnit that doesn't throw due to unknown framework version in some of our tests.
- See RunTests target hides exception thrown during NUnit fixture setup arcade#14812 for context.
- This is why I was seeing some test failures in Preserve original PDB path in Debug header jbevain/cecil#947 but not Preserve original PDB path in Debug header #183.

…ion (jbevain#882) When all other comparisons (return type, parameters, etc.) are the same, treat instance and static methods with the same name as different methods. This corrects a problem we noticed in IL2CPP in Unity.

…vain#879) During `AssemblyReader.ReadCustomAttributes` there is a call to `WindowsRuntimeProjections.Project` ``` if (module.IsWindowsMetadata ()) foreach (var custom_attribute in custom_attributes) WindowsRuntimeProjections.Project (owner, custom_attributes, custom_attribute); ``` `WindowsRuntimeProjections.Project` would call `WindowsRuntimeProjections.HasAttribute`, which would then call `type.CustomAttributes`, which would end up back in `AssemblyReader.ReadCustomAttributes`. This would lead to a StackOverflowException. This wasn't an issue previously. My PR jbevain#843 caused this sequence of calls to start resulting in a StackOverflowException. Prior to my PR, there was a call to `metadata.RemoveCustomAttributeRange (owner);` before the call to `WindowsRuntimeProjections.Project`. This meant that when `WindowsRuntimeProjections.HasAttribute` would call `type.CustomAttributes`, we'd still end up in `AssemblyReader.ReadCustomAttributes`, however, no attributes would be found because the following if would be true and lead to returning an empty collection. ``` if (!metadata.TryGetCustomAttributeRanges (owner, out ranges)) return new Collection<CustomAttribute> (); ``` The old behavior was probably the wrong. Although I'm not certain what the tangible impact was. The fix was pretty easy. `AssemblyReader.ReadCustomAttributes` will now pass in the custom attributes to `WindowsRuntimeProjections.Project` avoiding the need to call `type.CustomAttributes`

… the same (jbevain#885)

* Fix RVA field alignment * Use non-latest images to run tests * Revert windows-2019 change * Don't align code segment This wasn't aligned to begin with. Attempting to align it causes problems due to the way CodeWriter gets the code RVA - it computes the RVA from the CLI Header length, which doesn't take into account the code's alignment requirements. * Revert "Don't align code segment" This reverts commit 9410f1e. * Keep alignment values for code

With MSBuild 7.0.201 and later, building the Mono.Cecil projects used by Unity's IL2CPP causes warning NU1605 to occur, indicating that there is a package version mismatch between the .NET Framework reference assemblies package used. To avoid this, bump the version used by Mono.Cecil and similar project to be the latest, version 1.0.3. Note that MSBuild version 7.0.100 and earlier did not cause this warning to occur.

Co-authored-by: Marek Safar <marek.safar@gmail.com>

When resolving a `GenericInstanceMethod` for a `privatescope` method that has the same signature as other methods, `MetadataResolver.GetMethod` would incorrectly return the first method with the same name. The fix for this seems to be an optimization opportunity as well. Although I have to admit it feels a little to easy. Please make sure I'm not overlooking some reason why this fix is not safe. A added 2 variations of tests. `PrivateScope` - These tests were fine and passed as is. This is because the instruction operands are MethodDefinitions and therefore didn't need to be resolved by `MetadataResolver` `PrivateScopeGeneric` - This test triggered the bug.

* Add new flag for ByRefLike constraints * Allow seems to be the prefered nomenclature

sbomer · 2024-06-05T22:59:35Z

This brings in the latest cecil changes from upstream. Importantly, includes:

Changes for ByRefLike constrains (needed for Validate ILLink support for ref structs within generics runtime#98519)
Updates the version of NUnit that doesn't throw due to unknown framework version in some of our tests.
- See RunTests target hides exception thrown during NUnit fixture setup arcade#14812 for context.
- This is why I was seeing some test failures in Preserve original PDB path in Debug header jbevain/cecil#947 but not Preserve original PDB path in Debug header #183.

…ion (jbevain#882) When all other comparisons (return type, parameters, etc.) are the same, treat instance and static methods with the same name as different methods. This corrects a problem we noticed in IL2CPP in Unity.

…vain#879) During `AssemblyReader.ReadCustomAttributes` there is a call to `WindowsRuntimeProjections.Project` ``` if (module.IsWindowsMetadata ()) foreach (var custom_attribute in custom_attributes) WindowsRuntimeProjections.Project (owner, custom_attributes, custom_attribute); ``` `WindowsRuntimeProjections.Project` would call `WindowsRuntimeProjections.HasAttribute`, which would then call `type.CustomAttributes`, which would end up back in `AssemblyReader.ReadCustomAttributes`. This would lead to a StackOverflowException. This wasn't an issue previously. My PR jbevain#843 caused this sequence of calls to start resulting in a StackOverflowException. Prior to my PR, there was a call to `metadata.RemoveCustomAttributeRange (owner);` before the call to `WindowsRuntimeProjections.Project`. This meant that when `WindowsRuntimeProjections.HasAttribute` would call `type.CustomAttributes`, we'd still end up in `AssemblyReader.ReadCustomAttributes`, however, no attributes would be found because the following if would be true and lead to returning an empty collection. ``` if (!metadata.TryGetCustomAttributeRanges (owner, out ranges)) return new Collection<CustomAttribute> (); ``` The old behavior was probably the wrong. Although I'm not certain what the tangible impact was. The fix was pretty easy. `AssemblyReader.ReadCustomAttributes` will now pass in the custom attributes to `WindowsRuntimeProjections.Project` avoiding the need to call `type.CustomAttributes`

… the same (jbevain#885)

* Fix RVA field alignment * Use non-latest images to run tests * Revert windows-2019 change * Don't align code segment This wasn't aligned to begin with. Attempting to align it causes problems due to the way CodeWriter gets the code RVA - it computes the RVA from the CLI Header length, which doesn't take into account the code's alignment requirements. * Revert "Don't align code segment" This reverts commit 9410f1e. * Keep alignment values for code

With MSBuild 7.0.201 and later, building the Mono.Cecil projects used by Unity's IL2CPP causes warning NU1605 to occur, indicating that there is a package version mismatch between the .NET Framework reference assemblies package used. To avoid this, bump the version used by Mono.Cecil and similar project to be the latest, version 1.0.3. Note that MSBuild version 7.0.100 and earlier did not cause this warning to occur.

Co-authored-by: Marek Safar <marek.safar@gmail.com>

When resolving a `GenericInstanceMethod` for a `privatescope` method that has the same signature as other methods, `MetadataResolver.GetMethod` would incorrectly return the first method with the same name. The fix for this seems to be an optimization opportunity as well. Although I have to admit it feels a little to easy. Please make sure I'm not overlooking some reason why this fix is not safe. A added 2 variations of tests. `PrivateScope` - These tests were fine and passed as is. This is because the instruction operands are MethodDefinitions and therefore didn't need to be resolved by `MetadataResolver` `PrivateScopeGeneric` - This test triggered the bug.

* Add new flag for ByRefLike constraints * Allow seems to be the prefered nomenclature

sbomer · 2024-06-05T22:59:35Z

This brings in the latest cecil changes from upstream. Importantly, includes:

Changes for ByRefLike constrains (needed for Validate ILLink support for ref structs within generics runtime#98519)
Updates the version of NUnit that doesn't throw due to unknown framework version in some of our tests.
- See RunTests target hides exception thrown during NUnit fixture setup arcade#14812 for context.
- This is why I was seeing some test failures in Preserve original PDB path in Debug header jbevain/cecil#947 but not Preserve original PDB path in Debug header #183.

…ion (jbevain#882) When all other comparisons (return type, parameters, etc.) are the same, treat instance and static methods with the same name as different methods. This corrects a problem we noticed in IL2CPP in Unity.

…vain#879) During `AssemblyReader.ReadCustomAttributes` there is a call to `WindowsRuntimeProjections.Project` ``` if (module.IsWindowsMetadata ()) foreach (var custom_attribute in custom_attributes) WindowsRuntimeProjections.Project (owner, custom_attributes, custom_attribute); ``` `WindowsRuntimeProjections.Project` would call `WindowsRuntimeProjections.HasAttribute`, which would then call `type.CustomAttributes`, which would end up back in `AssemblyReader.ReadCustomAttributes`. This would lead to a StackOverflowException. This wasn't an issue previously. My PR jbevain#843 caused this sequence of calls to start resulting in a StackOverflowException. Prior to my PR, there was a call to `metadata.RemoveCustomAttributeRange (owner);` before the call to `WindowsRuntimeProjections.Project`. This meant that when `WindowsRuntimeProjections.HasAttribute` would call `type.CustomAttributes`, we'd still end up in `AssemblyReader.ReadCustomAttributes`, however, no attributes would be found because the following if would be true and lead to returning an empty collection. ``` if (!metadata.TryGetCustomAttributeRanges (owner, out ranges)) return new Collection<CustomAttribute> (); ``` The old behavior was probably the wrong. Although I'm not certain what the tangible impact was. The fix was pretty easy. `AssemblyReader.ReadCustomAttributes` will now pass in the custom attributes to `WindowsRuntimeProjections.Project` avoiding the need to call `type.CustomAttributes`

… the same (jbevain#885)

* Fix RVA field alignment * Use non-latest images to run tests * Revert windows-2019 change * Don't align code segment This wasn't aligned to begin with. Attempting to align it causes problems due to the way CodeWriter gets the code RVA - it computes the RVA from the CLI Header length, which doesn't take into account the code's alignment requirements. * Revert "Don't align code segment" This reverts commit 9410f1e. * Keep alignment values for code

With MSBuild 7.0.201 and later, building the Mono.Cecil projects used by Unity's IL2CPP causes warning NU1605 to occur, indicating that there is a package version mismatch between the .NET Framework reference assemblies package used. To avoid this, bump the version used by Mono.Cecil and similar project to be the latest, version 1.0.3. Note that MSBuild version 7.0.100 and earlier did not cause this warning to occur.

Co-authored-by: Marek Safar <marek.safar@gmail.com>

When resolving a `GenericInstanceMethod` for a `privatescope` method that has the same signature as other methods, `MetadataResolver.GetMethod` would incorrectly return the first method with the same name. The fix for this seems to be an optimization opportunity as well. Although I have to admit it feels a little to easy. Please make sure I'm not overlooking some reason why this fix is not safe. A added 2 variations of tests. `PrivateScope` - These tests were fine and passed as is. This is because the instruction operands are MethodDefinitions and therefore didn't need to be resolved by `MetadataResolver` `PrivateScopeGeneric` - This test triggered the bug.

* Add new flag for ByRefLike constraints * Allow seems to be the prefered nomenclature

sbomer · 2024-06-05T22:59:35Z

This brings in the latest cecil changes from upstream. Importantly, includes:

Changes for ByRefLike constrains (needed for Validate ILLink support for ref structs within generics runtime#98519)
Updates the version of NUnit that doesn't throw due to unknown framework version in some of our tests.
- See RunTests target hides exception thrown during NUnit fixture setup arcade#14812 for context.
- This is why I was seeing some test failures in Preserve original PDB path in Debug header jbevain/cecil#947 but not Preserve original PDB path in Debug header #183.

…ion (jbevain#882) When all other comparisons (return type, parameters, etc.) are the same, treat instance and static methods with the same name as different methods. This corrects a problem we noticed in IL2CPP in Unity.

…vain#879) During `AssemblyReader.ReadCustomAttributes` there is a call to `WindowsRuntimeProjections.Project` ``` if (module.IsWindowsMetadata ()) foreach (var custom_attribute in custom_attributes) WindowsRuntimeProjections.Project (owner, custom_attributes, custom_attribute); ``` `WindowsRuntimeProjections.Project` would call `WindowsRuntimeProjections.HasAttribute`, which would then call `type.CustomAttributes`, which would end up back in `AssemblyReader.ReadCustomAttributes`. This would lead to a StackOverflowException. This wasn't an issue previously. My PR jbevain#843 caused this sequence of calls to start resulting in a StackOverflowException. Prior to my PR, there was a call to `metadata.RemoveCustomAttributeRange (owner);` before the call to `WindowsRuntimeProjections.Project`. This meant that when `WindowsRuntimeProjections.HasAttribute` would call `type.CustomAttributes`, we'd still end up in `AssemblyReader.ReadCustomAttributes`, however, no attributes would be found because the following if would be true and lead to returning an empty collection. ``` if (!metadata.TryGetCustomAttributeRanges (owner, out ranges)) return new Collection<CustomAttribute> (); ``` The old behavior was probably the wrong. Although I'm not certain what the tangible impact was. The fix was pretty easy. `AssemblyReader.ReadCustomAttributes` will now pass in the custom attributes to `WindowsRuntimeProjections.Project` avoiding the need to call `type.CustomAttributes`

… the same (jbevain#885)

* Fix RVA field alignment * Use non-latest images to run tests * Revert windows-2019 change * Don't align code segment This wasn't aligned to begin with. Attempting to align it causes problems due to the way CodeWriter gets the code RVA - it computes the RVA from the CLI Header length, which doesn't take into account the code's alignment requirements. * Revert "Don't align code segment" This reverts commit 9410f1e. * Keep alignment values for code

With MSBuild 7.0.201 and later, building the Mono.Cecil projects used by Unity's IL2CPP causes warning NU1605 to occur, indicating that there is a package version mismatch between the .NET Framework reference assemblies package used. To avoid this, bump the version used by Mono.Cecil and similar project to be the latest, version 1.0.3. Note that MSBuild version 7.0.100 and earlier did not cause this warning to occur.

Co-authored-by: Marek Safar <marek.safar@gmail.com>

When resolving a `GenericInstanceMethod` for a `privatescope` method that has the same signature as other methods, `MetadataResolver.GetMethod` would incorrectly return the first method with the same name. The fix for this seems to be an optimization opportunity as well. Although I have to admit it feels a little to easy. Please make sure I'm not overlooking some reason why this fix is not safe. A added 2 variations of tests. `PrivateScope` - These tests were fine and passed as is. This is because the instruction operands are MethodDefinitions and therefore didn't need to be resolved by `MetadataResolver` `PrivateScopeGeneric` - This test triggered the bug.

* Add new flag for ByRefLike constraints * Allow seems to be the prefered nomenclature

Joshua Petersonand others added 14 commits November 15, 2022 09:53

Fix that method resolution would consider all function-pointers to be…
4ad9c0f
… the same (jbevain#885)

Bump to 0.11.5
8c123e1

Update README.md to remove lost link.
ed62c02

Add public API for instruction clonning (jbevain#915)
04286ac
Co-authored-by: Marek Safar <marek.safar@gmail.com>

Update to .net 8.0 (jbevain#942)
a840bdb

Import reflection modifiers (jbevain#943)
fec4ee9

Use FIPS compliant SHA1 (jbevain#944)
dff01d9

Add new flag for ByRefLike constraints (jbevain#945)
1da2145
* Add new flag for ByRefLike constraints * Allow seems to be the prefered nomenclature

Merge remote-tracking branch 'upstream/master' into mergeUpstream
fd53fe9

sbomer requested a review from jtschuster June 5, 2024 22:59

marek-safar merged commit 5c9255f into dotnet:mainJun 6, 2024

sbomer mentioned this pull request Jun 17, 2024
Update VersionPrefix #192
Merged

Joshua Petersonand others added 14 commits November 15, 2022 09:53

Fix that method resolution would consider all function-pointers to be…
4ad9c0f
… the same (jbevain#885)

Bump to 0.11.5
8c123e1

Update README.md to remove lost link.
ed62c02

Add public API for instruction clonning (jbevain#915)
04286ac
Co-authored-by: Marek Safar <marek.safar@gmail.com>

Update to .net 8.0 (jbevain#942)
a840bdb

Import reflection modifiers (jbevain#943)
fec4ee9

Use FIPS compliant SHA1 (jbevain#944)
dff01d9

Add new flag for ByRefLike constraints (jbevain#945)
1da2145
* Add new flag for ByRefLike constraints * Allow seems to be the prefered nomenclature

Merge remote-tracking branch 'upstream/master' into mergeUpstream
fd53fe9

sbomer requested a review from jtschuster June 5, 2024 22:59

marek-safar merged commit 5c9255f into dotnet:mainJun 6, 2024

sbomer mentioned this pull request Jun 17, 2024
Update VersionPrefix #192
Merged

Joshua Petersonand others added 14 commits November 15, 2022 09:53

Fix that method resolution would consider all function-pointers to be…
4ad9c0f
… the same (jbevain#885)

Bump to 0.11.5
8c123e1

Update README.md to remove lost link.
ed62c02

Add public API for instruction clonning (jbevain#915)
04286ac
Co-authored-by: Marek Safar <marek.safar@gmail.com>

Update to .net 8.0 (jbevain#942)
a840bdb

Import reflection modifiers (jbevain#943)
fec4ee9

Use FIPS compliant SHA1 (jbevain#944)
dff01d9

Add new flag for ByRefLike constraints (jbevain#945)
1da2145
* Add new flag for ByRefLike constraints * Allow seems to be the prefered nomenclature

Merge remote-tracking branch 'upstream/master' into mergeUpstream
fd53fe9

sbomer requested a review from jtschuster June 5, 2024 22:59

marek-safar merged commit 5c9255f into dotnet:mainJun 6, 2024

sbomer mentioned this pull request Jun 17, 2024
Update VersionPrefix #192
Merged

Joshua Petersonand others added 14 commits November 15, 2022 09:53

Fix that method resolution would consider all function-pointers to be…
4ad9c0f
… the same (jbevain#885)

Bump to 0.11.5
8c123e1

Update README.md to remove lost link.
ed62c02

Add public API for instruction clonning (jbevain#915)
04286ac
Co-authored-by: Marek Safar <marek.safar@gmail.com>

Update to .net 8.0 (jbevain#942)
a840bdb

Import reflection modifiers (jbevain#943)
fec4ee9

Use FIPS compliant SHA1 (jbevain#944)
dff01d9

Add new flag for ByRefLike constraints (jbevain#945)
1da2145
* Add new flag for ByRefLike constraints * Allow seems to be the prefered nomenclature

Merge remote-tracking branch 'upstream/master' into mergeUpstream
fd53fe9

sbomer requested a review from jtschuster June 5, 2024 22:59

marek-safar merged commit 5c9255f into dotnet:mainJun 6, 2024

sbomer mentioned this pull request Jun 17, 2024
Update VersionPrefix #192
Merged

Joshua Petersonand others added 14 commits November 15, 2022 09:53

Fix that method resolution would consider all function-pointers to be…
4ad9c0f
… the same (jbevain#885)

Bump to 0.11.5
8c123e1

Update README.md to remove lost link.
ed62c02

Add public API for instruction clonning (jbevain#915)
04286ac
Co-authored-by: Marek Safar <marek.safar@gmail.com>

Update to .net 8.0 (jbevain#942)
a840bdb

Import reflection modifiers (jbevain#943)
fec4ee9

Use FIPS compliant SHA1 (jbevain#944)
dff01d9

Add new flag for ByRefLike constraints (jbevain#945)
1da2145
* Add new flag for ByRefLike constraints * Allow seems to be the prefered nomenclature

Merge remote-tracking branch 'upstream/master' into mergeUpstream
fd53fe9

sbomer requested a review from jtschuster June 5, 2024 22:59

marek-safar merged commit 5c9255f into dotnet:mainJun 6, 2024

sbomer mentioned this pull request Jun 17, 2024
Update VersionPrefix #192
Merged

Joshua Petersonand others added 14 commits November 15, 2022 09:53

Fix that method resolution would consider all function-pointers to be…
4ad9c0f
… the same (jbevain#885)

Bump to 0.11.5
8c123e1

Update README.md to remove lost link.
ed62c02

Add public API for instruction clonning (jbevain#915)
04286ac
Co-authored-by: Marek Safar <marek.safar@gmail.com>

Update to .net 8.0 (jbevain#942)
a840bdb

Import reflection modifiers (jbevain#943)
fec4ee9

Use FIPS compliant SHA1 (jbevain#944)
dff01d9

Add new flag for ByRefLike constraints (jbevain#945)
1da2145
* Add new flag for ByRefLike constraints * Allow seems to be the prefered nomenclature

Merge remote-tracking branch 'upstream/master' into mergeUpstream
fd53fe9

sbomer requested a review from jtschuster June 5, 2024 22:59

marek-safar merged commit 5c9255f into dotnet:mainJun 6, 2024

sbomer mentioned this pull request Jun 17, 2024
Update VersionPrefix #192
Merged

Joshua Petersonand others added 14 commits November 15, 2022 09:53

Fix that method resolution would consider all function-pointers to be…
4ad9c0f
… the same (jbevain#885)

Bump to 0.11.5
8c123e1

Update README.md to remove lost link.
ed62c02

Add public API for instruction clonning (jbevain#915)
04286ac
Co-authored-by: Marek Safar <marek.safar@gmail.com>

Update to .net 8.0 (jbevain#942)
a840bdb

Import reflection modifiers (jbevain#943)
fec4ee9

Use FIPS compliant SHA1 (jbevain#944)
dff01d9

Add new flag for ByRefLike constraints (jbevain#945)
1da2145
* Add new flag for ByRefLike constraints * Allow seems to be the prefered nomenclature

Merge remote-tracking branch 'upstream/master' into mergeUpstream
fd53fe9

sbomer requested a review from jtschuster June 5, 2024 22:59

marek-safar merged commit 5c9255f into dotnet:mainJun 6, 2024

sbomer mentioned this pull request Jun 17, 2024
Update VersionPrefix #192
Merged

Joshua Petersonand others added 14 commits November 15, 2022 09:53

Fix that method resolution would consider all function-pointers to be…
4ad9c0f
… the same (jbevain#885)

Bump to 0.11.5
8c123e1

Update README.md to remove lost link.
ed62c02

Add public API for instruction clonning (jbevain#915)
04286ac
Co-authored-by: Marek Safar <marek.safar@gmail.com>

Update to .net 8.0 (jbevain#942)
a840bdb

Import reflection modifiers (jbevain#943)
fec4ee9

Use FIPS compliant SHA1 (jbevain#944)
dff01d9

Add new flag for ByRefLike constraints (jbevain#945)
1da2145
* Add new flag for ByRefLike constraints * Allow seems to be the prefered nomenclature

Merge remote-tracking branch 'upstream/master' into mergeUpstream
fd53fe9

sbomer requested a review from jtschuster June 5, 2024 22:59

marek-safar merged commit 5c9255f into dotnet:mainJun 6, 2024

sbomer mentioned this pull request Jun 17, 2024
Update VersionPrefix #192
Merged

Uh oh!

Conversation

sbomer commented Jun 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

sbomer commented Jun 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

sbomer commented Jun 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

sbomer commented Jun 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

sbomer commented Jun 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

sbomer commented Jun 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

sbomer commented Jun 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects