diff --git a/eng/Common.globalconfig b/eng/Common.globalconfig index c75fbec9493..7f557b4c7ba 100644 --- a/eng/Common.globalconfig +++ b/eng/Common.globalconfig @@ -319,7 +319,7 @@ dotnet_diagnostic.CA1836.severity = warning dotnet_diagnostic.CA1837.severity = suggestion # Avoid 'StringBuilder' parameters for P/Invokes -dotnet_diagnostic.CA1838.severity = suggestion +dotnet_diagnostic.CA1838.severity = warning # Dispose objects before losing scope dotnet_diagnostic.CA2000.severity = none diff --git a/src/Framework/NativeMethods.cs b/src/Framework/NativeMethods.cs index 9c3a8dbaec5..bf6714d12ee 100644 --- a/src/Framework/NativeMethods.cs +++ b/src/Framework/NativeMethods.cs @@ -48,7 +48,6 @@ internal static class NativeMethods internal const int MAX_PATH = 260; private const string kernel32Dll = "kernel32.dll"; - private const string mscoreeDLL = "mscoree.dll"; private const string WINDOWS_FILE_SYSTEM_REGISTRY_KEY = @"SYSTEM\CurrentControlSet\Control\FileSystem"; private const string WINDOWS_LONG_PATHS_ENABLED_VALUE_NAME = "LongPathsEnabled"; @@ -950,13 +949,13 @@ internal static string GetShortFilePath(string path) if (length > 0) { - StringBuilder fullPathBuffer = new StringBuilder(length); + char[] fullPathBuffer = new char[length]; length = GetShortPathName(path, fullPathBuffer, length); errorCode = Marshal.GetLastWin32Error(); if (length > 0) { - string fullPath = fullPathBuffer.ToString(); + string fullPath = new(fullPathBuffer, 0, length); path = fullPath; } } @@ -989,13 +988,13 @@ internal static string GetLongFilePath(string path) if (length > 0) { - StringBuilder fullPathBuffer = new StringBuilder(length); + char[] fullPathBuffer = new char[length]; length = GetLongPathName(path, fullPathBuffer, length); errorCode = Marshal.GetLastWin32Error(); if (length > 0) { - string fullPath = fullPathBuffer.ToString(); + string fullPath = new(fullPathBuffer, 0, length); path = fullPath; } } @@ -1084,7 +1083,7 @@ DateTime LastWriteFileUtcTime(string path) if (success && (data.fileAttributes & NativeMethods.FILE_ATTRIBUTE_DIRECTORY) == 0) { - long dt = ((long) (data.ftLastWriteTimeHigh) << 32) | ((long) data.ftLastWriteTimeLow); + long dt = ((long)(data.ftLastWriteTimeHigh) << 32) | ((long)data.ftLastWriteTimeLow); fileModifiedTime = DateTime.FromFileTimeUtc(dt); // If file is a symlink _and_ we're not instructed to do the wrong thing, get a more accurate timestamp. @@ -1468,17 +1467,6 @@ internal static void VerifyThrowWin32Result(int result) [return: MarshalAs(UnmanagedType.Bool)] internal static extern bool GetFileAttributesEx(String name, int fileInfoLevel, ref WIN32_FILE_ATTRIBUTE_DATA lpFileInformation); - [DllImport(kernel32Dll, SetLastError = true, CharSet = CharSet.Unicode)] - private static extern uint SearchPath - ( - string path, - string fileName, - string extension, - int numBufferChars, - [Out] StringBuilder buffer, - int[] filePart - ); - [DllImport("kernel32.dll", PreserveSig = true, SetLastError = true)] [return: MarshalAs(UnmanagedType.Bool)] internal static extern bool FreeLibrary([In] IntPtr module); @@ -1489,26 +1477,14 @@ int[] filePart [DllImport("kernel32.dll", CharSet = CharSet.Unicode, PreserveSig = true, SetLastError = true)] internal static extern IntPtr LoadLibrary(string fileName); - [DllImport(mscoreeDLL, SetLastError = true, CharSet = CharSet.Unicode)] - internal static extern uint GetRequestedRuntimeInfo(String pExe, - String pwszVersion, - String pConfigurationFile, - uint startupFlags, - uint runtimeInfoFlags, - [Out] StringBuilder pDirectory, - int dwDirectory, - out uint dwDirectoryLength, - [Out] StringBuilder pVersion, - int cchBuffer, - out uint dwlength); - /// - /// Gets the fully qualified filename of the currently executing .exe + /// Gets the fully qualified filename of the currently executing .exe. /// + /// of the module for which we are finding the file name. + /// The character buffer used to return the file name. + /// The length of the buffer. [DllImport(kernel32Dll, SetLastError = true, CharSet = CharSet.Unicode)] - internal static extern int GetModuleFileName( - HandleRef hModule, - [Out] StringBuilder buffer, int length); + internal static extern int GetModuleFileName(HandleRef hModule, [Out] char[] buffer, int length); [DllImport("kernel32.dll")] internal static extern IntPtr GetStdHandle(int nStdHandle); @@ -1557,10 +1533,10 @@ internal static bool SetCurrentDirectory(string path) private static extern bool GlobalMemoryStatusEx([In, Out] MemoryStatus lpBuffer); [DllImport("kernel32.dll", CharSet = CharSet.Unicode, BestFitMapping = false)] - internal static extern int GetShortPathName(string path, [Out] StringBuilder fullpath, [In] int length); + internal static extern int GetShortPathName(string path, [Out] char[] fullpath, [In] int length); [DllImport("kernel32.dll", CharSet = CharSet.Unicode, BestFitMapping = false)] - internal static extern int GetLongPathName([In] string path, [Out] StringBuilder fullpath, [In] int length); + internal static extern int GetLongPathName([In] string path, [Out] char[] fullpath, [In] int length); [DllImport("kernel32.dll", CharSet = CharSet.Auto, SetLastError = true)] internal static extern bool CreatePipe(out SafeFileHandle hReadPipe, out SafeFileHandle hWritePipe, SecurityAttributes lpPipeAttributes, int nSize); @@ -1648,7 +1624,7 @@ internal static bool MsgWaitOne(this WaitHandle handle, int timeout) if (!(returnValue == 0 || ((uint)returnValue == RPC_S_CALLPENDING && timeout != Timeout.Infinite))) { - throw new InternalErrorException($"Received {returnValue} from CoWaitForMultipleHandles, but expected 0 (S_OK)"); + throw new InternalErrorException($"Received {returnValue} from CoWaitForMultipleHandles, but expected 0 (S_OK)"); } return returnValue == 0; diff --git a/src/Tasks/AssemblyDependency/AssemblyInformation.cs b/src/Tasks/AssemblyDependency/AssemblyInformation.cs index 574040bdeac..5e5f39a1656 100644 --- a/src/Tasks/AssemblyDependency/AssemblyInformation.cs +++ b/src/Tasks/AssemblyDependency/AssemblyInformation.cs @@ -569,30 +569,36 @@ internal static string GetRuntimeVersion(string path) #if FEATURE_MSCOREE if (NativeMethodsShared.IsWindows) { - StringBuilder runtimeVersion; - uint hresult; #if DEBUG - // Just to make sure and exercise the code that doubles the size - // every time GetRequestedRuntimeInfo fails due to insufficient buffer size. + // Just to make sure and exercise the code that uses dwLength to allocate the buffer + // when GetRequestedRuntimeInfo fails due to insufficient buffer size. int bufferLength = 1; #else int bufferLength = 11; // 11 is the length of a runtime version and null terminator v2.0.50727/0 #endif - do - { - runtimeVersion = new StringBuilder(bufferLength); - hresult = NativeMethods.GetFileVersion(path, runtimeVersion, bufferLength, out _); - bufferLength *= 2; - } while (hresult == NativeMethodsShared.ERROR_INSUFFICIENT_BUFFER); - if (hresult == NativeMethodsShared.S_OK) + unsafe { - return runtimeVersion.ToString(); - } - else - { - return String.Empty; - } + // Allocate an initial buffer + char* runtimeVersion = stackalloc char[bufferLength]; + + // Run GetFileVersion, this should succeed using the initial buffer. + // It also returns the dwLength which is used if there is insufficient buffer. + uint hresult = NativeMethods.GetFileVersion(path, runtimeVersion, bufferLength, out int dwLength); + + if (hresult == NativeMethodsShared.ERROR_INSUFFICIENT_BUFFER) + { + // Allocate new buffer based on the returned length. + char* runtimeVersion2 = stackalloc char[dwLength]; + runtimeVersion = runtimeVersion2; + + // Get the RuntimeVersion in this second call. + bufferLength = dwLength; + hresult = NativeMethods.GetFileVersion(path, runtimeVersion, bufferLength, out dwLength); + } + + return hresult == NativeMethodsShared.S_OK ? new string(runtimeVersion, 0, dwLength - 1) : string.Empty; + } } else { @@ -603,7 +609,6 @@ internal static string GetRuntimeVersion(string path) #endif } - /// /// Import assembly dependencies. /// @@ -790,7 +795,7 @@ private static AssemblyNameExtension ConstructAssemblyName(IntPtr asmMetaPtr, ch // Construct the assembly name. (Note asmNameLength should/must be > 0.) var assemblyName = new AssemblyName { - Name = new string(asmNameBuf, 0, (int) asmNameLength - 1), + Name = new string(asmNameBuf, 0, (int)asmNameLength - 1), Version = new Version( asmMeta.usMajorVersion, asmMeta.usMinorVersion, @@ -911,7 +916,7 @@ public static string GetRuntimeVersion(string path) // Read the PE header signature sr.BaseStream.Position = peHeaderOffset; - if (!ReadBytes(sr, (byte) 'P', (byte) 'E', 0, 0)) + if (!ReadBytes(sr, (byte)'P', (byte)'E', 0, 0)) { return string.Empty; } diff --git a/src/Tasks/AssemblyDependency/GlobalAssemblyCache.cs b/src/Tasks/AssemblyDependency/GlobalAssemblyCache.cs index b7df962c76f..7611767ca12 100644 --- a/src/Tasks/AssemblyDependency/GlobalAssemblyCache.cs +++ b/src/Tasks/AssemblyDependency/GlobalAssemblyCache.cs @@ -31,6 +31,16 @@ internal static class GlobalAssemblyCache /// internal static readonly GetGacEnumerator gacEnumerator = GetGacNativeEnumerator; + /// + /// Lazy loaded cached root path of the GAC. + /// + private static readonly Lazy _gacPath = new(() => GetGacPath()); + + /// + /// Gets the root path of the GAC. + /// + internal static string GacPath => _gacPath.Value; + /// /// Given a strong name, find its path in the GAC. /// @@ -367,16 +377,18 @@ bool specificVersion } /// - /// Return the root path of the GAC + /// Return the root path of the GAC. /// internal static string GetGacPath() { int gacPathLength = 0; - NativeMethods.GetCachePath(AssemblyCacheFlags.GAC, null, ref gacPathLength); - StringBuilder gacPath = new StringBuilder(gacPathLength); - NativeMethods.GetCachePath(AssemblyCacheFlags.GAC, gacPath, ref gacPathLength); - - return gacPath.ToString(); + unsafe + { + NativeMethods.GetCachePath(AssemblyCacheFlags.GAC, null, ref gacPathLength); + char* gacPath = stackalloc char[gacPathLength]; + NativeMethods.GetCachePath(AssemblyCacheFlags.GAC, gacPath, ref gacPathLength); + return new string(gacPath, 0, gacPathLength - 1); + } } } } diff --git a/src/Tasks/ComReference.cs b/src/Tasks/ComReference.cs index cea831d5798..eaf732c4ec4 100644 --- a/src/Tasks/ComReference.cs +++ b/src/Tasks/ComReference.cs @@ -406,22 +406,33 @@ internal static string StripTypeLibNumberFromPath(string typeLibPath, FileExists private static string GetModuleFileName(IntPtr handle) { - bool success = false; - var buffer = new StringBuilder(); + char[] buffer = null; // Try increased buffer sizes if on longpath-enabled Windows - for (int bufferSize = NativeMethodsShared.MAX_PATH; !success && bufferSize <= NativeMethodsShared.MaxPath; bufferSize *= 2) + for (int bufferSize = NativeMethodsShared.MAX_PATH; bufferSize <= NativeMethodsShared.MaxPath; bufferSize *= 2) { - buffer.EnsureCapacity(bufferSize); + buffer = System.Buffers.ArrayPool.Shared.Rent(bufferSize); + try + { + var handleRef = new System.Runtime.InteropServices.HandleRef(buffer, handle); + int pathLength = NativeMethodsShared.GetModuleFileName(handleRef, buffer, bufferSize); - var handleRef = new System.Runtime.InteropServices.HandleRef(buffer, handle); - int pathLength = NativeMethodsShared.GetModuleFileName(handleRef, buffer, buffer.Capacity); + bool isBufferTooSmall = (uint)Marshal.GetLastWin32Error() == NativeMethodsShared.ERROR_INSUFFICIENT_BUFFER; + if (pathLength != 0 && !isBufferTooSmall) + { + return new string(buffer, 0, pathLength); + } + } + finally + { + System.Buffers.ArrayPool.Shared.Return(buffer); + } - bool isBufferTooSmall = ((uint)Marshal.GetLastWin32Error() == NativeMethodsShared.ERROR_INSUFFICIENT_BUFFER); - success = pathLength != 0 && !isBufferTooSmall; + // Double check that the buffer is not insanely big + ErrorUtilities.VerifyThrow(bufferSize <= int.MaxValue / 2, "Buffer size approaching int.MaxValue"); } - return success ? buffer.ToString() : string.Empty; + return string.Empty; } /// diff --git a/src/Tasks/LockCheck.cs b/src/Tasks/LockCheck.cs index fcd8a41a300..9ef5cdd62d8 100644 --- a/src/Tasks/LockCheck.cs +++ b/src/Tasks/LockCheck.cs @@ -55,10 +55,57 @@ internal enum ApplicationType uint nServices, string[] rgsServiceNames); + /// + /// Starts a new Restart Manager session. + /// A maximum of 64 Restart Manager sessions per user session + /// can be open on the system at the same time. When this + /// function starts a session, it returns a session handle + /// and session key that can be used in subsequent calls to + /// the Restart Manager API. + /// + /// + /// A pointer to the handle of a Restart Manager session. + /// The session handle can be passed in subsequent calls + /// to the Restart Manager API. + /// + /// + /// Reserved. This parameter should be 0. + /// + /// + /// A null-terminated string that contains the session key + /// to the new session. The string must be allocated before + /// calling the RmStartSession function. + /// + /// System error codes that are defined in Winerror.h. + /// + /// The Rm­­StartSession function doesn’t properly null-terminate + /// the session key, even though the function is documented as + /// returning a null-terminated string. To work around this bug, + /// we pre-fill the buffer with null characters so that whatever + /// ends gets written will have a null terminator (namely, one of + /// the null characters we placed ahead of time). + /// + /// see . + /// + /// [DllImport(RestartManagerDll, CharSet = CharSet.Unicode)] - private static extern int RmStartSession(out uint pSessionHandle, - int dwSessionFlags, StringBuilder strSessionKey); - + private static extern unsafe int RmStartSession( + out uint pSessionHandle, + int dwSessionFlags, + char* strSessionKey); + + /// + /// Ends the Restart Manager session. + /// This function should be called by the primary installer that + /// has previously started the session by calling the + /// function. The RmEndSession function can be called by a secondary installer + /// that is joined to the session once no more resources need to be registered + /// by the secondary installer. + /// + /// A handle to an existing Restart Manager session. + /// + /// The function can return one of the system error codes that are defined in Winerror.h. + /// [DllImport(RestartManagerDll)] private static extern int RmEndSession(uint pSessionHandle); @@ -207,11 +254,16 @@ internal static IEnumerable GetLockingProcessInfos(params string[] } const int maxRetries = 6; + uint handle; + int res; - // See http://blogs.msdn.com/b/oldnewthing/archive/2012/02/17/10268840.aspx. - var key = new StringBuilder(new string('\0', CCH_RM_SESSION_KEY + 1)); + unsafe + { + // See http://blogs.msdn.com/b/oldnewthing/archive/2012/02/17/10268840.aspx. + char* key = stackalloc char[CCH_RM_SESSION_KEY + 1]; + res = RmStartSession(out handle, 0, key); + } - int res = RmStartSession(out uint handle, 0, key); if (res != 0) { throw GetException(res, "RmStartSession", "Failed to begin restart manager session."); diff --git a/src/Tasks/NativeMethods.cs b/src/Tasks/NativeMethods.cs index 45bc7d9faad..54b21a7a904 100644 --- a/src/Tasks/NativeMethods.cs +++ b/src/Tasks/NativeMethods.cs @@ -1053,13 +1053,15 @@ internal static bool AllDrivesMapped() /// /// GetCachePath from fusion.dll. - /// Using StringBuilder here is a way to pass a preallocated buffer of characters to (native) functions that require it. /// A common design pattern in unmanaged C++ is calling a function twice, once to determine the length of the string - /// and then again to pass the client-allocated character buffer. StringBuilder is the most straightforward way - /// to allocate a mutable buffer of characters and pass it around. + /// and then again to pass the client-allocated character buffer. /// + /// Value that indicates the source of the cached assembly. + /// The returned pointer to the path. + /// The requested maximum length of CachePath, and upon return, the actual length of CachePath. + /// [DllImport("fusion.dll", CharSet = CharSet.Unicode)] - internal static extern int GetCachePath(AssemblyCacheFlags cacheFlags, StringBuilder cachePath, ref int pcchPath); + internal static extern unsafe int GetCachePath(AssemblyCacheFlags cacheFlags, [Out] char* cachePath, ref int pcchPath); #endif //------------------------------------------------------------------------------ @@ -1125,15 +1127,15 @@ internal static bool AllDrivesMapped() #if FEATURE_MSCOREE /// - /// Get the runtime version for a given file + /// Get the runtime version for a given file. /// - /// The path of the file to be examined + /// The path of the file to be examined. /// The buffer allocated for the version information that is returned. - /// The size, in wide characters, of szBuffer + /// The size, in wide characters, of szBuffer. /// The size, in bytes, of the returned szBuffer. - /// HResult + /// HResult. [DllImport(MscoreeDLL, SetLastError = true, CharSet = CharSet.Unicode)] - internal static extern uint GetFileVersion(String szFullPath, StringBuilder szBuffer, int cchBuffer, out uint dwLength); + internal static extern unsafe uint GetFileVersion([MarshalAs(UnmanagedType.LPWStr)] string szFileName, [Out] char* szBuffer, int cchBuffer, out int dwLength); #endif #endregion diff --git a/src/Tasks/ResolveComReference.cs b/src/Tasks/ResolveComReference.cs index afa2a75cb88..911d84afb0b 100644 --- a/src/Tasks/ResolveComReference.cs +++ b/src/Tasks/ResolveComReference.cs @@ -432,7 +432,7 @@ public override bool Execute() } } - SetCopyLocalToFalseOnGacOrNoPIAAssemblies(resolvedReferenceList, GlobalAssemblyCache.GetGacPath()); + SetCopyLocalToFalseOnGacOrNoPIAAssemblies(resolvedReferenceList, GlobalAssemblyCache.GacPath); ResolvedModules = moduleList.ToArray(); ResolvedFiles = resolvedReferenceList.ToArray();