SecurityTransparent should be removed from Collections.Immutable

[VSadov]

SecurityTransparent is a bit of a vestige nowdays, but is still enforced on desktop runtime.
As such the presence of the attribute is a blocker for the new language/runtime features such as ref returns, readonly references, spans...

Use of readonly references, in particular, makes a lot of sense, since the collections are Immutable

NOTE: Nearly no other library in CoreFx uses SecurityTransparent. ( https://github.com/dotnet/corefx/search?utf8=%E2%9C%93&q=SecurityTransparent&type= )

It is surprising that Collections.Immutable still does.

I think the attribute needs to be removed and soon since it is already causing troubles.
(Example: change dotnet/corefx@fc4b1ad when upgrade to C#7.2 had to be paired with disabling certain optimizations - we are not even using new features yet)

[VSadov]

CC: @AArnott @jaredpar @stephentoub @jkotas

[jkotas]

https://github.com/dotnet/corefx/issues/1512
https://github.com/dotnet/corefx/issues/3293

[jaredpar]

Agree. This attribute is specifically blocking us from making performance wins in Roslyn. It prevents us from using any performance related feature we've done in SCI and that is the core of Roslyn's performance.

Customers who want to continue to use SecurityTransparent can use the older version of this library.

[jaredpar]

@jkotas yep. In both those cases the customer can continue using the older NuGet package. Going forward we should remove this limitation so we can take advantage of all the new performance work we've been doing.

[AArnott]

Using an older version of the package sounds easy -- until a desktop app wants to use a library that 'requires' a newer version of the package. When will this be a problem? If full trust contexts on .NET Framework have no problem with this attribute removed, then I don't object.

[jaredpar]

Requiring that this package continue to load in full trust enviroment is pretty much going to prevent it from being modified in any substantial way going forward. Consider that:

• Moving to Net Standard 2.0 will break verification. All DLLs / EXEs which target Net Standard 2.0 will fail to verify and hence not be usable in full trust. This is due to corlib being named netstandard instead of mscorlib.
• Using C# 7.2 features, such as ref / ref readonly returns, do not pass PEVerify and hence can't be used in a full trust environment.

This is not due to the code being unsafe but rather due to PEVerify simply not being updated anymore to match the evolution of the platform. Other projects like ILVerify are attempting to fill the gap for type safe verification but it will not impact full trust envirnoments which still rely on PEVerify.

This is the only package in the .NET Core SDK which has this attribute and the only one being held back by this scenario. It's also important for performance and platform evolution that we move this package forward.

The solution for existing customers who rely on full trust is to use the existing package. Any library which is requiring them to upgrade is itself likely using a technology like Net Standard 2.0 which prevents full trust usage. Hence no point there in keeping this.

[jkotas]

Requiring that this package continue to load in full trust

I assume you meant to say partial trust.

Moving to Net Standard 2.0 will break verification. All DLLs / EXEs which target Net Standard 2.0 will fail to verify and hence not be usable in full trust.

It will break PEVerify. It will not break partial trust. The checks done by PEVerify are not the same as the checks done when running in particular trust.

I agree with the rest what you are saying. Partial trust is a dead end. Not worth keeping it alive.

[AArnott]

@jaredpar Is it safe to replace every one of your uses of the term "full trust" with "partial trust"? Otherwise I'm thoroughly confused as I would expect everything to always work in full trust environments (the default for .NET Framework). I thought removing this security transparent attribute would break partial trust scenarios, which were never very popular anyway and have been deprecated long ago.

[AArnott]

Also, how is this package being held back? So what if it can't use C# 7.2 features like ref returns? Have we identified a need for it in this package? Or is it preventing consumers of the package from using this new C# feature when dealing with types defined in this library?

[jaredpar]

Is it safe to replace every one of your uses of the term "full trust" with "partial trust"?

Yes. Sorry for the confusion here.

Have we identified a need for it in this package?

There are many places in this library where we would like to take advantage of ref readonly returns. Concretely we can make Roslyn more efficient in places if we have a ref readonly index style method on ImmutableArray<T>. It's not possible to use that until we move to C# 7.2.

[AArnott]

OK. Since it sounds like it works in Full Trust environments on .NET Framework, I don't object.