Warnings for intrinsics that throw or are unreachable

[sbomer]

dotnet/linker#2652 added tests that showed a few cases where intrinsics produce warnings for null inputs even when they throw at runtime. The intrinsic handling falls back on the annotated return values, which warn if they flow into methods with requirements that aren't satisfied by the return annotation.

// Warns about the return value of GetType, even though this throws at runtime.
Type.GetType (null).RequiresAll ();

TestType nullInstance = null;
// Warns about the return value of GetType, even though this throws at runtime
nullInstance.GetType ().RequiresAll ();

Type type = null;
// Warns about the return value of BaseType, even though this throws at runtime.
type.BaseType.RequiresPublicMethods ();

// Technically this shouldn't warn because CreateInstanceFrom throws if the assembly file path is null.
// However, our implementation is the same for CreateInstance and CreateInstanceFrom.
Activator.CreateInstanceFrom (null, "Namespace.SomeType");

A similar issue exists for "empty" inputs flowing to intrinsics (the empty inputs represent unimplemented intrinsics in the analyzer, which should not warn, or values produced from intrinsics that are known to throw at runtime). In a few intrinsics, we propagate the "empty" input, while in others, we fall back on annotations.

Type t = null;
string noValue = t.AssemblyQualifiedName;
// Warns about the return value of GetType, even though AssemblyQualifiedName throws at runtime.
Type.GetType (noValue).RequiresAll ();

Type t = null;
Type noValue = Type.GetTypeFromHandle (t.TypeHandle);
// Warns about the return value of GetType, even though the above throws at runtime.
noValue.GetType ().RequiresAll ();

Type t = null;
Type noValue = Type.GetTypeFromHandle (t.TypeHandle);
// Warns about the base type even though the above throws an exception at runtime.
noValue.BaseType.RequiresPublicMethods ();

Type t = null;
Type noValue = Type.GetTypeFromHandle (t.TypeHandle);
// Warns about the base type even though the above throws an exception at runtime.
noValue.BaseType.RequiresPublicMethods ();

My suggestion would be to consistently avoid producing warnings for these cases, and return "empty" from intrinsics that we know would throw, or which receive an "empty" value.

[sbomer]

I found another example:

MethodInfo noValue = typeof (MakeGenericMethod).GetMethod (null);
// Warns even though GetMethod throws at runtime.
noValue.MakeGenericMethod (typeof (TestType));

[sbomer]

dotnet/linker#2694 fixed this for Type.BaseType.

BTW @vitek-karas I realized that another way to think about this is that the BaseType behavior violated monotonicity of the transfer functions (which in general is required for the dataflow analysis to converge). So if x <= y in the lattice, then we should have Intrinsic(x) <= Intrinsic(y). But for BaseType, we had BaseType({All}) = {All} and BaseType({}) = {None} - where {All} <= {}, but it's not true that {All} <= {None}.

[vitek-karas]

That's a good point!

[dotnet-policy-service]

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.