Fix nullability warnings for NativeAOT CoreLib (#65137)

* Fix nullability warnings for NatveAOT CoreLib Enable checks for CS8600 Contributes to dotnet/runtimelab#106 * Bunch of CS8602 * Apply PR feedback * Fix compile error * Fix errors * Use notnull constraint
dotnet · Apr 18, 2022 · 778b522 · 778b522
1 parent 70612ee
commit 778b522
Show file tree

Hide file tree

Showing 18 changed files with 51 additions and 23 deletions.
diff --git a/src/coreclr/nativeaot/Common/src/Internal/Runtime/CompilerHelpers/StartupCodeHelpers.cs b/src/coreclr/nativeaot/Common/src/Internal/Runtime/CompilerHelpers/StartupCodeHelpers.cs
@@ -88,7 +88,7 @@ internal static int GetLoadedModules(TypeManagerHandle[] outputModules)
 
         private static void AddModule(TypeManagerHandle newModuleHandle)
         {
-            if (s_modules == null || s_moduleCount >= s_modules.Length)
+            if (s_moduleCount >= s_modules!.Length)
             {
                 // Reallocate logical module array
                 int newModuleLength = 2 * s_moduleCount;

diff --git a/src/coreclr/nativeaot/Common/src/Internal/Runtime/CompilerHelpers/StartupDebug.cs b/src/coreclr/nativeaot/Common/src/Internal/Runtime/CompilerHelpers/StartupDebug.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
 
 namespace Internal.Runtime.CompilerHelpers
 {
@@ -13,7 +14,7 @@ namespace Internal.Runtime.CompilerHelpers
     internal static class StartupDebug
     {
         [Conditional("DEBUG")]
-        public static void Assert(bool condition)
+        public static void Assert([DoesNotReturnIf(false)] bool condition)
         {
             if (!condition)
                 unsafe { *(int*)0 = 0; }

diff --git a/src/coreclr/nativeaot/Directory.Build.props b/src/coreclr/nativeaot/Directory.Build.props
@@ -31,7 +31,7 @@
     <NoWarn>$(NoWarn),0419,0649,CA2249,CA1830</NoWarn>
 
     <!-- Disable nullability-related warnings -->
-    <NoWarn>$(NoWarn);CS8600;CS8602;CS8603;CS8604;CS8618;CS8625;CS8632;CS8765</NoWarn>
+    <NoWarn>$(NoWarn);CS8602;CS8603;CS8604;CS8618;CS8625;CS8632;CS8765</NoWarn>
 
     <!-- we should just fix -->
     <NoWarn>$(NoWarn);CA1810;CA1823;CA1825;CA2208;SA1129;SA1205;SA1400;SA1517</NoWarn>

diff --git a/...nativeaot/System.Private.CoreLib/src/Internal/IntrinsicSupport/EqualityComparerHelpers.cs b/...nativeaot/System.Private.CoreLib/src/Internal/IntrinsicSupport/EqualityComparerHelpers.cs
@@ -156,6 +156,7 @@ internal static EqualityComparer<T> GetComparerForReferenceTypesOnly<T>()
         }
 
         private static bool StructOnlyNormalEquals<T>(T left, T right)
+            where T : notnull
         {
             return left.Equals(right);
         }

diff --git a/...ate.CoreLib/src/Internal/Reflection/Extensions/NonPortable/CustomAttributeInstantiator.cs b/...ate.CoreLib/src/Internal/Reflection/Extensions/NonPortable/CustomAttributeInstantiator.cs
@@ -68,7 +68,7 @@ public static Attribute Instantiate(this CustomAttributeData cad)
             //
             // Found the right constructor. Instantiate the Attribute.
             //
-            int arity = matchingParameters.Length;
+            int arity = matchingParameters!.Length;
             object?[] invokeArguments = new object[arity];
             for (int i = 0; i < arity; i++)
             {

diff --git a/...coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/Augments/RuntimeAugments.cs b/...coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/Augments/RuntimeAugments.cs
@@ -143,7 +143,7 @@ public static unsafe Array NewMultiDimArray(RuntimeTypeHandle typeHandleForArray
             {
                 // We just checked above that all lower bounds are zero. In that case, we should actually allocate
                 // a new SzArray instead.
-                Type elementType = Type.GetTypeFromHandle(new RuntimeTypeHandle(typeHandleForArrayType.ToEETypePtr().ArrayElementType));
+                Type elementType = Type.GetTypeFromHandle(new RuntimeTypeHandle(typeHandleForArrayType.ToEETypePtr().ArrayElementType))!;
                 return RuntimeImports.RhNewArray(elementType.MakeArrayType().TypeHandle.ToEETypePtr(), lengths[0]);
             }
 

diff --git a/...clr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/ArrayHelpers.cs b/...clr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/ArrayHelpers.cs
@@ -64,7 +64,7 @@ public static unsafe Array NewObjArray(IntPtr pEEType, int nDimensions, int* pDi
                 {
                     // Multidimensional array of rank 1 with 0 lower bounds gets actually allocated
                     // as an SzArray. SzArray is castable to MdArray rank 1.
-                    Type elementType = Type.GetTypeFromHandle(new RuntimeTypeHandle(eeType.ArrayElementType));
+                    Type elementType = Type.GetTypeFromHandle(new RuntimeTypeHandle(eeType.ArrayElementType))!;
                     return RuntimeImports.RhNewArray(elementType.MakeArrayType().TypeHandle.ToEETypePtr(), pDimensions[0]);
                 }
 

diff --git a/...r/nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/InteropHelpers.cs b/...r/nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/InteropHelpers.cs
@@ -341,7 +341,7 @@ internal static unsafe void FixupModuleCell(ModuleFixupCell* pCell)
                 if (hModule == IntPtr.Zero)
                 {
                     // Built-in rules didn't resolve the library. Use AssemblyLoadContext as a last chance attempt.
-                    AssemblyLoadContext? loadContext = AssemblyLoadContext.GetLoadContext(callingAssembly);
+                    AssemblyLoadContext loadContext = AssemblyLoadContext.GetLoadContext(callingAssembly)!;
                     hModule = loadContext.GetResolvedUnmanagedDll(callingAssembly, moduleName);
                 }
 

diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Environment.CoreRT.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Environment.CoreRT.cs
@@ -65,6 +65,10 @@ internal static void ShutdownCore()
 
         public static int TickCount => (int)TickCount64;
 
-        public static string[] GetCommandLineArgs() => (string[])s_commandLineArgs.Clone();
+        public static string[] GetCommandLineArgs()
+        {
+            Debug.Assert(s_commandLineArgs != null, "VM did not properly setup application.");
+            return (string[])s_commandLineArgs.Clone();
+        }
     }
 }
diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/InvokeUtils.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/InvokeUtils.cs
@@ -96,7 +96,7 @@ internal enum CheckArgumentSemantics
                         return srcObject;
                 }
 
-                object dstObject;
+                object? dstObject;
                 Exception exception = ConvertOrWidenPrimitivesEnumsAndPointersIfPossible(srcObject, srcEEType, dstEEType, semantics, out dstObject);
                 if (exception == null)
                     return dstObject;
@@ -127,7 +127,7 @@ internal enum CheckArgumentSemantics
         }
 
         // Special coersion rules for primitives, enums and pointer.
-        private static Exception ConvertOrWidenPrimitivesEnumsAndPointersIfPossible(object srcObject, EETypePtr srcEEType, EETypePtr dstEEType, CheckArgumentSemantics semantics, out object dstObject)
+        private static Exception ConvertOrWidenPrimitivesEnumsAndPointersIfPossible(object srcObject, EETypePtr srcEEType, EETypePtr dstEEType, CheckArgumentSemantics semantics, out object? dstObject)
         {
             if (semantics == CheckArgumentSemantics.SetFieldDirect && (srcEEType.IsEnum || dstEEType.IsEnum))
             {
@@ -509,9 +509,9 @@ internal static ref IntPtr DynamicInvokeParamHelperRef(ref ArgSetupState argSetu
             }
         }
 
-        internal static object DynamicInvokeBoxedValuetypeReturn(out DynamicInvokeParamLookupType paramLookupType, object? boxedValuetype, object?[]? parameters, int index, RuntimeTypeHandle type, DynamicInvokeParamType paramType, ref object[] nullableCopyBackObjects)
+        internal static object DynamicInvokeBoxedValuetypeReturn(out DynamicInvokeParamLookupType paramLookupType, object? boxedValuetype, object?[] parameters, int index, RuntimeTypeHandle type, DynamicInvokeParamType paramType, ref object[] nullableCopyBackObjects)
         {
-            object finalObjectToReturn = boxedValuetype;
+            object? finalObjectToReturn = boxedValuetype;
             EETypePtr eeType = type.ToEETypePtr();
             bool nullable = eeType.IsNullable;
 
@@ -548,7 +548,7 @@ internal static object DynamicInvokeBoxedValuetypeReturn(out DynamicInvokeParamL
             return finalObjectToReturn;
         }
 
-        internal static object DynamicInvokeUnmanagedPointerReturn(out DynamicInvokeParamLookupType paramLookupType, object? boxedPointerType, int index, RuntimeTypeHandle type, DynamicInvokeParamType paramType)
+        internal static object DynamicInvokeUnmanagedPointerReturn(out DynamicInvokeParamLookupType paramLookupType, object boxedPointerType, int index, RuntimeTypeHandle type, DynamicInvokeParamType paramType)
         {
             object finalObjectToReturn = boxedPointerType;
 

diff --git a/...nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.CoreRT.cs b/...nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.CoreRT.cs
@@ -330,7 +330,7 @@ public unsafe IntPtr GetOrCreateComInterfaceForObject(object instance, CreateCom
             if (instance == null)
                 throw new ArgumentNullException(nameof(instance));
 
-            ManagedObjectWrapperHolder ccwValue;
+            ManagedObjectWrapperHolder? ccwValue;
             if (_ccwTable.TryGetValue(instance, out ccwValue))
             {
                 return ccwValue.ComIp;

diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/ManagedThreadId.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/ManagedThreadId.cs
@@ -209,6 +209,8 @@ public static int AllocateId()
             if (s_idDispenser == null)
                 Interlocked.CompareExchange(ref s_idDispenser, ImmutableIdDispenser.Empty, null);
 
+            Debug.Assert(s_idDispenser != null);
+
             int id;
 
             var priorIdDispenser = Volatile.Read(ref s_idDispenser);

diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Overlapped.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Overlapped.cs
@@ -158,7 +158,7 @@ private void FreeNativeOverlapped()
         internal static unsafe OverlappedData GetOverlappedFromNative(NativeOverlapped* pNativeOverlapped)
         {
             GCHandle handle = *(GCHandle*)(pNativeOverlapped + 1);
-            return (OverlappedData)handle.Target;
+            return (OverlappedData)handle.Target!;
         }
     }
 

diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.CoreRT.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.CoreRT.cs
@@ -376,7 +376,7 @@ private void StartCore()
         private static void StartThread(IntPtr parameter)
         {
             GCHandle threadHandle = (GCHandle)parameter;
-            Thread thread = (Thread)threadHandle.Target;
+            Thread thread = (Thread)threadHandle.Target!;
 
             try
             {

diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/ThreadPool.Windows.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/ThreadPool.Windows.cs
@@ -59,7 +59,7 @@ internal static void RegisteredWaitCallback(IntPtr instance, IntPtr context, Int
         {
             var wrapper = ThreadPoolCallbackWrapper.Enter();
             GCHandle handle = (GCHandle)context;
-            RegisteredWaitHandle registeredWaitHandle = (RegisteredWaitHandle)handle.Target;
+            RegisteredWaitHandle registeredWaitHandle = (RegisteredWaitHandle)handle.Target!;
             Debug.Assert((handle == registeredWaitHandle._gcHandle) && (wait == registeredWaitHandle._tpWait));
 
             bool timedOut = (waitResult == (uint)Interop.Kernel32.WAIT_TIMEOUT);
@@ -138,7 +138,7 @@ public bool Unregister(WaitHandle waitObject)
                     Interop.Kernel32.SetThreadpoolWait(_tpWait, IntPtr.Zero, IntPtr.Zero);
 
                     // Should we wait for callbacks synchronously? Note that we treat the zero handle as the asynchronous case.
-                    SafeWaitHandle safeWaitHandle = waitObject?.SafeWaitHandle;
+                    SafeWaitHandle? safeWaitHandle = waitObject?.SafeWaitHandle;
                     bool blocking = ((safeWaitHandle != null) && (safeWaitHandle.DangerousGetHandle() == new IntPtr(-1)));
 
                     if (blocking)

diff --git a/.../nativeaot/System.Private.CoreLib/src/System/Threading/Win32ThreadPoolNativeOverlapped.cs b/.../nativeaot/System.Private.CoreLib/src/System/Threading/Win32ThreadPoolNativeOverlapped.cs
@@ -75,7 +75,7 @@ internal OverlappedData Data
 
             while (true)
             {
-                OverlappedData[] dataArray = Volatile.Read(ref s_dataArray);
+                OverlappedData[]? dataArray = Volatile.Read(ref s_dataArray);
                 int currentLength = dataArray == null ? 0 : dataArray.Length;
 
                 // If the current array is too small, create a new, larger one.
@@ -87,7 +87,7 @@ internal OverlappedData Data
                     while (newLength <= dataIndex)
                         newLength = (newLength * 3) / 2;
 
-                    OverlappedData[] newDataArray = dataArray;
+                    OverlappedData[]? newDataArray = dataArray;
                     Array.Resize(ref newDataArray, newLength);
 
                     if (Interlocked.CompareExchange(ref s_dataArray, newDataArray, dataArray) != dataArray)
@@ -101,12 +101,12 @@ internal OverlappedData Data
                 if (s_dataArray[dataIndex] == null)
                 {
                     // Full fence so this write can't move past subsequent reads.
-                    Interlocked.Exchange(ref dataArray[dataIndex], data);
+                    Interlocked.Exchange(ref dataArray![dataIndex], data);
                     continue;
                 }
 
                 // We're already in the array, so we're done.
-                Debug.Assert(dataArray[dataIndex] == data);
+                Debug.Assert(dataArray![dataIndex] == data);
                 overlapped->_dataIndex = dataIndex;
                 return overlapped;
             }
@@ -129,7 +129,7 @@ private void SetData(IOCompletionCallback callback, object state, object pinData
             //
             if (pinData != null)
             {
-                object[] objArray = pinData as object[];
+                object[]? objArray = pinData as object[];
                 if (objArray != null && objArray.GetType() == typeof(object[]))
                 {
                     if (data._pinnedData == null || data._pinnedData.Length < objArray.Length)
@@ -192,6 +192,7 @@ internal static unsafe void CompleteWithCallback(uint errorCode, uint bytesWritt
 
             if (data._executionContext == null)
             {
+                Debug.Assert(data._callback != null, "Does CompleteWithCallback called after Reset?");
                 data._callback(errorCode, bytesWritten, ToNativeOverlapped(overlapped));
                 return;
             }
@@ -229,6 +230,7 @@ private static unsafe void OnExecutionContextCallback(object? state)
             args._data = null;
             t_executionContextCallbackArgs = args;
 
+            Debug.Assert(data._callback != null, "Does OnExecutionContextCallback called after Reset?");
             data._callback(errorCode, bytesWritten, ToNativeOverlapped(overlapped));
         }
 

diff --git a/...lr/nativeaot/Test.CoreLib/src/System/Diagnostics/CodeAnalysis/DoesNotReturnIfAttribute.cs b/...lr/nativeaot/Test.CoreLib/src/System/Diagnostics/CodeAnalysis/DoesNotReturnIfAttribute.cs
@@ -0,0 +1,17 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace System.Diagnostics.CodeAnalysis;
+
+public sealed class DoesNotReturnIfAttribute : Attribute
+{
+    /// <summary>Initializes the attribute with the specified parameter value.</summary>
+    /// <param name="parameterValue">
+    /// The condition parameter value. Code after the method will be considered unreachable by diagnostics if the argument to
+    /// the associated parameter matches this value.
+    /// </param>
+    public DoesNotReturnIfAttribute(bool parameterValue) => ParameterValue = parameterValue;
+
+    /// <summary>Gets the condition parameter value.</summary>
+    public bool ParameterValue { get; }
+}
diff --git a/src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj b/src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj
@@ -224,6 +224,7 @@
     <Compile Include="System\Runtime\CompilerServices\ClassConstructorRunner.cs" />
     <Compile Include="System\Runtime\CompilerServices\StaticClassConstructionContext.cs" />
     <Compile Include="System\Runtime\InteropServices\InAttribute.cs" />
+    <Compile Include="System\Diagnostics\CodeAnalysis\DoesNotReturnIfAttribute.cs" />
     <Compile Include="System\Runtime\RuntimeImports.cs" />
     <Compile Include="System\Runtime\RuntimeHelpers.cs" />
     <Compile Include="System\Runtime\InitializeFinalizerThread.cs" />