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

Trimmer removes implementation of members of nested types #2185

Closed
yaakov-h opened this issue Aug 1, 2021 · 4 comments
Closed

Trimmer removes implementation of members of nested types #2185

yaakov-h opened this issue Aug 1, 2021 · 4 comments

Comments

@yaakov-h
Copy link
Member

yaakov-h commented Aug 1, 2021

I am trying to update https://github.com/steamre/steamkit to be trim-compatible, which is a heavy ask since it depends on a lot of reflection, particularly to serialize and deserialize messages, which are done through the protobuf-.net library.

I have applied the following attribute in order to ensure that the key parts of message types do not get trimmed:

[DynamicallyAccessedMembersAttribute( DynamicallyAccessedMemberTypes.PublicConstructors |
    DynamicallyAccessedMemberTypes.NonPublicConstructors |
    DynamicallyAccessedMemberTypes.PublicMethods |
    DynamicallyAccessedMemberTypes.NonPublicMethods |
    DynamicallyAccessedMemberTypes.PublicFields |
    DynamicallyAccessedMemberTypes.NonPublicFields |
    DynamicallyAccessedMemberTypes.PublicNestedTypes |
    DynamicallyAccessedMemberTypes.NonPublicNestedTypes |
    DynamicallyAccessedMemberTypes.PublicProperties |
    DynamicallyAccessedMemberTypes.NonPublicProperties
)]

In practice I have made a constant for all these values so I only have to add [DynamicallyAccessedMembersAttribute(Trimming.ForProtobufNet)] to the relevant code, and defined the following centrally:

static class Trimming
{
    public const DynamicallyAccessedMemberTypes ForProtobufNet = DynamicallyAccessedMemberTypes.All;
        DynamicallyAccessedMemberTypes.PublicConstructors |
        DynamicallyAccessedMemberTypes.NonPublicConstructors |
        DynamicallyAccessedMemberTypes.PublicMethods |
        DynamicallyAccessedMemberTypes.NonPublicMethods |
        DynamicallyAccessedMemberTypes.PublicFields |
        DynamicallyAccessedMemberTypes.NonPublicFields |
        DynamicallyAccessedMemberTypes.PublicNestedTypes |
        DynamicallyAccessedMemberTypes.NonPublicNestedTypes |
        DynamicallyAccessedMemberTypes.PublicProperties |
        DynamicallyAccessedMemberTypes.NonPublicProperties;
}

However, a trimmed consuming application crashes with the following error:

System.InvalidOperationException: Cannot apply changes to property SteamKit2.Internal.CMsgClientFriendsList+Friend.ulfriendid
   at ProtoBuf.Internal.Serializers.PropertyDecorator.SanityCheck(PropertyInfo property, IRuntimeProtoSerializerNode tail, Boolean& writeValue, Boolean nonPublic, Boolean allowInternal) in /_/src/protobuf-net/Internal/Serializers/PropertyDecorator.cs:line 40
   at ProtoBuf.Internal.Serializers.PropertyDecorator..ctor(Type forType, PropertyInfo property, IRuntimeProtoSerializerNode tail) in /_/src/protobuf-net/Internal/Serializers/PropertyDecorator.cs:line 24
   at ProtoBuf.Meta.ValueMember.BuildSerializer() in /_/src/protobuf-net/Meta/ValueMember.cs:line 509
   at ProtoBuf.Meta.MetaType.BuildSerializer() in /_/src/protobuf-net/Meta/MetaType.cs:line 492
   at ProtoBuf.Meta.MetaType.get_Serializer() in /_/src/protobuf-net/Meta/MetaType.cs:line 449
   at ProtoBuf.Meta.RuntimeTypeModel.<GetServicesSlow>g__GetServicesImpl|88_0(RuntimeTypeModel model, Type type, CompatibilityLevel ambient) in /_/src/protobuf-net/Meta/RuntimeTypeModel.cs:line 1032
   at ProtoBuf.Meta.RuntimeTypeModel.GetServicesSlow(Type type, CompatibilityLevel ambient) in /_/src/protobuf-net/Meta/RuntimeTypeModel.cs:line 998
   at ProtoBuf.Meta.RuntimeTypeModel.GetSerializerCore[T](CompatibilityLevel ambient) in /_/src/protobuf-net/Meta/RuntimeTypeModel.cs:line 964
   at ProtoBuf.Meta.TypeModel.GetSerializer[T](TypeModel model, CompatibilityLevel ambient) in /_/src/protobuf-net.Core/Meta/TypeModel.cs:line 1413
   at ProtoBuf.Serializers.RepeatedSerializer`2.ReadRepeated(State& state, SerializerFeatures features, TCollection values, ISerializer`1 serializer) in /_/src/protobuf-net.Core/Serializers/RepeatedSerializer.cs:line 275
   at proto_24(State& , CMsgClientFriendsList )
   at ProtoBuf.Internal.Serializers.SimpleCompiledSerializer`1.ProtoBuf.Serializers.ISerializer<T>.Read(State& state, T value) in /_/src/protobuf-net/Internal/Serializers/CompiledSerializer.cs:line 107
   at ProtoBuf.ProtoReader.State.ReadAsRoot[T](T value, ISerializer`1 serializer) in /_/src/protobuf-net.Core/ProtoReader.State.ReadMethods.cs:line 1056
   at ProtoBuf.ProtoReader.State.DeserializeRoot[T](T value, ISerializer`1 serializer) in /_/src/protobuf-net.Core/ProtoReader.State.ReadMethods.cs:line 1036
   at ProtoBuf.Serializer.Deserialize[T](Stream source) in /_/src/protobuf-net/Serializer.Deserialize.cs:line 21
   at SteamKit2.ClientMsgProtobuf`1.Deserialize(Byte[] ) in C:\Users\Yaakov\source\repos\SteamKit\SteamKit2\SteamKit2\Base\ClientMsg.cs:line 226
   at SteamKit2.ClientMsgProtobuf`1..ctor(IPacketMsg ) in C:\Users\Yaakov\source\repos\SteamKit\SteamKit2\SteamKit2\Base\ClientMsg.cs:line 195
   at SteamKit2.SteamFriends.HandleFriendsList(IPacketMsg ) in C:\Users\Yaakov\source\repos\SteamKit\SteamKit2\SteamKit2\Steam\Handlers\SteamFriends\SteamFriends.cs:line 789
   at SteamKit2.SteamFriends.HandleMsg(IPacketMsg ) in C:\Users\Yaakov\source\repos\SteamKit\SteamKit2\SteamKit2\Steam\Handlers\SteamFriends\SteamFriends.cs:line 750
   at SteamKit2.SteamClient.OnClientMsgReceived(IPacketMsg ) in C:\Users\Yaakov\source\repos\SteamKit\SteamKit2\SteamKit2\Steam\SteamClient\SteamClient.cs:line 373

Examining the trimmed version of SteamKit2.dll shows that the nested type still exists, however its members have been trimmed out of existence by replacing their actual implementation with throwing an exception:

ILSpy screenshot showing nested type members have had their IL replaced

The final IL for this type is:

.class nested public auto ansi beforefieldinit Friend
	extends [System.Private.CoreLib]System.Object
	implements ['protobuf-net.Core']ProtoBuf.IExtensible
{
	.custom instance void ['protobuf-net.Core']ProtoBuf.ProtoContractAttribute::.ctor() = (
		01 00 00 00
	)
	// Methods
	.method private final hidebysig newslot virtual 
		instance class ['protobuf-net.Core']ProtoBuf.IExtension 'global::ProtoBuf.IExtensible.GetExtensionObject' (
			bool ''
		) cil managed noinlining 
	{
		.override method instance class ['protobuf-net.Core']ProtoBuf.IExtension ['protobuf-net.Core']ProtoBuf.IExtensible::GetExtensionObject(bool)
		// Method begins at RVA 0x13a32
		// Code size 11 (0xb)
		.maxstack 8

		IL_0000: ldstr "Linked away"
		IL_0005: newobj instance void [System.Private.CoreLib]System.NotSupportedException::.ctor(string)
		IL_000a: throw
	} // end of method Friend::'global::ProtoBuf.IExtensible.GetExtensionObject'

	.method public hidebysig specialname 
		instance uint64 get_ulfriendid () cil managed noinlining 
	{
		// Method begins at RVA 0x13a32
		// Code size 11 (0xb)
		.maxstack 8

		IL_0000: ldstr "Linked away"
		IL_0005: newobj instance void [System.Private.CoreLib]System.NotSupportedException::.ctor(string)
		IL_000a: throw
	} // end of method Friend::get_ulfriendid

	.method public hidebysig specialname 
		instance uint32 get_efriendrelationship () cil managed noinlining 
	{
		// Method begins at RVA 0x13a32
		// Code size 11 (0xb)
		.maxstack 8

		IL_0000: ldstr "Linked away"
		IL_0005: newobj instance void [System.Private.CoreLib]System.NotSupportedException::.ctor(string)
		IL_000a: throw
	} // end of method Friend::get_efriendrelationship

	// Properties
	.property instance uint64 ulfriendid()
	{
		.custom instance void ['protobuf-net.Core']ProtoBuf.ProtoMemberAttribute::.ctor(int32) = (
			01 00 01 00 00 00 01 00 54 55 69 50 72 6f 74 6f
			42 75 66 2e 44 61 74 61 46 6f 72 6d 61 74 2c 20
			70 72 6f 74 6f 62 75 66 2d 6e 65 74 2e 43 6f 72
			65 2c 20 56 65 72 73 69 6f 6e 3d 33 2e 30 2e 30
			2e 30 2c 20 43 75 6c 74 75 72 65 3d 6e 65 75 74
			72 61 6c 2c 20 50 75 62 6c 69 63 4b 65 79 54 6f
			6b 65 6e 3d 32 35 37 62 35 31 64 38 37 64 32 65
			34 64 36 37 0a 44 61 74 61 46 6f 72 6d 61 74 03
			00 00 00
		)
		.get instance uint64 SteamKit2.Internal.CMsgClientFriendsList/Friend::get_ulfriendid()
	}
	.property instance uint32 efriendrelationship()
	{
		.custom instance void ['protobuf-net.Core']ProtoBuf.ProtoMemberAttribute::.ctor(int32) = (
			01 00 02 00 00 00 00 00
		)
		.get instance uint32 SteamKit2.Internal.CMsgClientFriendsList/Friend::get_efriendrelationship()
	}

} // end of class Friend

As a workaround I have switched the attributes to use [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)], which leaves the property implementations intact, but this seems a bit strange.

Surely DynamicallyAccessedMemberTypes.PublicNestedTypes should be enough? There aren't any further member types that I can specify, such as DynamicallyAccessedMemberTypes.PublicNestedTypesPublicProperties etc.

@MichalStrehovsky
Copy link
Member

PublicNestedTypes wasn't very useful because it only kept the hollowed out type without any members. This changed recently in #2133 to align with your expectations (the type will be kept along with all its members).

I'm not sure of the logistics of when this will show up in .NET 6 previews/RC, but we definitely want it to land in .NET 6. You could try grabbing the latest SDK out of the main branch https://github.com/dotnet/installer#installers-and-binaries - maybe it's already included.

@yaakov-h
Copy link
Member Author

yaakov-h commented Aug 1, 2021

It looks like this may already be in the current main, which is version rc.1.21381.5.

@MichalStrehovsky
Copy link
Member

It looks like this may already be in the current main, which is version rc.1.21381.5.

Thanks for checking! Is it okay to close this or is there something else that's not working right for the nested types?

@yaakov-h
Copy link
Member Author

yaakov-h commented Aug 1, 2021

Good to close.

I thought there was one more thing still not working but it turns out the function I was calling doesn't specify either of PublicNestedTypes or NonPublicNestedTypes.

@yaakov-h yaakov-h closed this as completed Aug 1, 2021
yaakov-h added a commit to yaakov-h/protobuf-net that referenced this issue Aug 1, 2021
This prevents nested types - e.g. child objects or arrays of child
objects - from being removed by the linker when an application is
trimmed.

This only works at the moment in current unreleased versions of the
.NET SDK (e.g. .NET 6 rc.1.21381.5). It does not work in any publicly
released preview up to and including .NET 6 Preview 6.

See dotnet/linker#2185 for more information.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants