Don't use incompatible LOAD_LIBRARY_SEARCH flags when using LOAD_WITH_ALTERED_SEARCH_PATH (#111990)

LoadLibraryEx documents that the `LOAD_WITH_ALTERED_SEARCH_PATH` flag is incompatible with `LOAD_LIBRARY_SEARCH` flags. In cases where we explicitly add this flag (absolute path or looking in p/invoke assembly directory), we could end up specifying an incompatible combination if other `DllImportSearchPath` flags corresponding to the `LOAD_LIBRARY_SEARCH` flags were also specified. This change avoids using those flags in when the runtime specifies loading with an altered search path.

Fixes #111825

Author: elinor-fung

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/NativeLibrary.NativeAot.cs

@@ -67,6 +67,11 @@ internal static IntPtr LoadBySearch(Assembly callingAssembly, bool searchAssembl
             IntPtr ret;
 
             int loadWithAlteredPathFlags = LoadWithAlteredSearchPathFlag;
+            const int loadLibrarySearchFlags = (int)DllImportSearchPath.UseDllDirectoryForDependencies
+                | (int)DllImportSearchPath.ApplicationDirectory
+                | (int)DllImportSearchPath.UserDirectories
+                | (int)DllImportSearchPath.System32
+                | (int)DllImportSearchPath.SafeDirectories;
             bool libNameIsRelativePath = !Path.IsPathFullyQualified(libraryName);
 
             // P/Invokes are often declared with variations on the actual library name.
@@ -80,14 +85,8 @@ internal static IntPtr LoadBySearch(Assembly callingAssembly, bool searchAssembl
 
                 if (!libNameIsRelativePath)
                 {
-                    int flags = loadWithAlteredPathFlags;
-                    if ((dllImportSearchPathFlags & (int)DllImportSearchPath.UseDllDirectoryForDependencies) != 0)
-                    {
-                        // LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR is the only flag affecting absolute path. Don't OR the flags
-                        // unconditionally as all absolute path P/Invokes could then lose LOAD_WITH_ALTERED_SEARCH_PATH.
-                        flags |= dllImportSearchPathFlags;
-                    }
-
+                    // LOAD_WITH_ALTERED_SEARCH_PATH is incompatible with LOAD_LIBRARY_SEARCH flags. Remove those flags if they are set.
+                    int flags = loadWithAlteredPathFlags | (dllImportSearchPathFlags & ~loadLibrarySearchFlags);
                     ret = LoadLibraryHelper(currLibNameVariation, flags, ref errorTracker);
                     if (ret != IntPtr.Zero)
                     {
@@ -96,9 +95,12 @@ internal static IntPtr LoadBySearch(Assembly callingAssembly, bool searchAssembl
                 }
                 else if ((callingAssembly != null) && searchAssemblyDirectory)
                 {
+                    // LOAD_WITH_ALTERED_SEARCH_PATH is incompatible with LOAD_LIBRARY_SEARCH flags. Remove those flags if they are set.
+                    int flags = loadWithAlteredPathFlags | (dllImportSearchPathFlags & ~loadLibrarySearchFlags);
+
                     // Try to load the module alongside the assembly where the PInvoke was declared.
                     // For PInvokes where the DllImportSearchPath.AssemblyDirectory is specified, look next to the application.
-                    ret = LoadLibraryHelper(Path.Combine(AppContext.BaseDirectory, currLibNameVariation), loadWithAlteredPathFlags | dllImportSearchPathFlags, ref errorTracker);
+                    ret = LoadLibraryHelper(Path.Combine(AppContext.BaseDirectory, currLibNameVariation), flags, ref errorTracker);
                     if (ret != IntPtr.Zero)
                     {
                         return ret;

src/coreclr/vm/nativelibrary.cpp

@@ -476,6 +476,7 @@ namespace
         NATIVE_LIBRARY_HANDLE hmod = NULL;
 
         SString path{ pAssembly->GetPEAssembly()->GetPath() };
+        _ASSERTE(!Path::IsRelative(path));
 
         SString::Iterator lastPathSeparatorIter = path.End();
         if (PEAssembly::FindLastPathSeparator(path, lastPathSeparatorIter))
@@ -656,6 +657,14 @@ namespace
 
         AppDomain* pDomain = GetAppDomain();
         DWORD loadWithAlteredPathFlags = GetLoadWithAlteredSearchPathFlag();
+        DWORD loadLibrarySearchFlags = 0;
+#ifdef TARGET_WINDOWS
+        loadLibrarySearchFlags = LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR
+            | LOAD_LIBRARY_SEARCH_APPLICATION_DIR
+            | LOAD_LIBRARY_SEARCH_USER_DIRS
+            | LOAD_LIBRARY_SEARCH_SYSTEM32
+            | LOAD_LIBRARY_SEARCH_DEFAULT_DIRS;
+#endif
         bool libNameIsRelativePath = Path::IsRelative(wszLibName);
 
         // P/Invokes are often declared with variations on the actual library name.
@@ -689,14 +698,8 @@ namespace
 
             if (!libNameIsRelativePath)
             {
-                DWORD flags = loadWithAlteredPathFlags;
-                if ((dllImportSearchPathFlags & LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR) != 0)
-                {
-                    // LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR is the only flag affecting absolute path. Don't OR the flags
-                    // unconditionally as all absolute path P/Invokes could then lose LOAD_WITH_ALTERED_SEARCH_PATH.
-                    flags |= dllImportSearchPathFlags;
-                }
-
+                // LOAD_WITH_ALTERED_SEARCH_PATH is incompatible with LOAD_LIBRARY_SEARCH flags. Remove those flags if they are set.
+                DWORD flags = loadWithAlteredPathFlags | (dllImportSearchPathFlags & ~loadLibrarySearchFlags);
                 hmod = LocalLoadLibraryHelper(currLibNameVariation, flags, pErrorTracker);
                 if (hmod != NULL)
                 {
@@ -705,7 +708,9 @@ namespace
             }
             else if ((callingAssembly != nullptr) && searchAssemblyDirectory)
             {
-                hmod = LoadFromPInvokeAssemblyDirectory(callingAssembly, currLibNameVariation, loadWithAlteredPathFlags | dllImportSearchPathFlags, pErrorTracker);
+                // LOAD_WITH_ALTERED_SEARCH_PATH is incompatible with LOAD_LIBRARY_SEARCH flags. Remove those flags if they are set.
+                DWORD flags = loadWithAlteredPathFlags | (dllImportSearchPathFlags & ~loadLibrarySearchFlags);
+                hmod = LoadFromPInvokeAssemblyDirectory(callingAssembly, currLibNameVariation, flags, pErrorTracker);
                 if (hmod != NULL)
                 {
                     return hmod;

src/mono/mono/metadata/native-library.c

@@ -275,6 +275,22 @@ convert_dllimport_flags (int flags)
 #endif
 }
 
+static int
+add_load_with_altered_search_path_flags (int flags)
+{
+#ifdef HOST_WIN32
+	// LOAD_WITH_ALTERED_SEARCH_PATH is incompatible with LOAD_LIBRARY_SEARCH flags. Remove those flags if they are set.
+	int load_library_search_flags = LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR
+		| LOAD_LIBRARY_SEARCH_APPLICATION_DIR
+		| LOAD_LIBRARY_SEARCH_USER_DIRS
+		| LOAD_LIBRARY_SEARCH_SYSTEM32
+		| LOAD_LIBRARY_SEARCH_DEFAULT_DIRS;
+	return LOAD_WITH_ALTERED_SEARCH_PATH | (flags & ~load_library_search_flags);
+#else
+	return flags;
+#endif
+}
+
 static MonoDl *
 netcore_probe_for_module_variations (const char *mdirname, const char *file_name, int raw_flags, MonoError *error)
 {
@@ -352,7 +368,7 @@ netcore_probe_for_module (MonoImage *image, const char *file_name, int flags, Mo
 		error_init_reuse (error);
 		char *mdirname = g_path_get_dirname (image->filename);
 		if (mdirname)
-			module = netcore_probe_for_module_variations (mdirname, file_name, lflags, error);
+			module = netcore_probe_for_module_variations (mdirname, file_name, add_load_with_altered_search_path_flags(lflags), error);
 		g_free (mdirname);
 	}
 

src/tests/Interop/CMakeLists.txt

@@ -57,6 +57,7 @@ add_subdirectory(MarshalAPI/FunctionPointer)
 add_subdirectory(NativeLibrary/NativeLibraryToLoad)
 add_subdirectory(DllImportAttribute/DllImportPath)
 add_subdirectory(DllImportAttribute/ExactSpelling)
+add_subdirectory(DllImportSearchPaths/NativeLibraryWithDependency)
 add_subdirectory(ICustomMarshaler/ConflictingNames)
 add_subdirectory(ICustomMarshaler/Primitives)
 add_subdirectory(LayoutClass)

src/tests/Interop/DllImportSearchPaths/DllImportSearchPathsTest.cs

@@ -65,6 +65,20 @@ public static void AssemblyDirectory_Fallback_Found()
             Environment.CurrentDirectory = currentDirectory;
         }
     }
+
+    [ConditionalFact(nameof(CanLoadAssemblyInSubdirectory))]
+    [PlatformSpecific(TestPlatforms.Windows)]
+    public static void AssemblyDirectory_SearchFlags_WithDependency_Found()
+    {
+        // Library and its dependency should be found in the assembly directory.
+        var assembly = Assembly.LoadFile(Path.Combine(Subdirectory, $"{nameof(DllImportSearchPathsTest)}.dll"));
+        var type = assembly.GetType(nameof(NativeLibraryWithDependency));
+        var method = type.GetMethod(nameof(NativeLibraryWithDependency.Sum));
+
+        int sum = (int)method.Invoke(null, new object[] { 1, 2 });
+        Assert.Equal(3, sum);
+        Console.WriteLine("NativeLibraryWithDependency.Sum returned {0}", sum);
+    }
 }
 
 public class NativeLibraryPInvoke
@@ -93,3 +107,18 @@ public static int Sum(int a, int b)
     [DefaultDllImportSearchPaths(DllImportSearchPath.AssemblyDirectory | DllImportSearchPath.System32)]
     static extern int NativeSum(int arg1, int arg2);
 }
+
+public class NativeLibraryWithDependency
+{
+    public static int Sum(int a, int b)
+    {
+        return CallDependencySum(a, b);
+    }
+
+    // For LoadLibrary on Windows, search flags, like that represented by System32, are incompatible with
+    // looking at a specific path (per AssemblyDirectory), so we specify both flags to validate that we do
+    // not incorrectly use both when looking in the assembly directory.
+    [DllImport(nameof(NativeLibraryWithDependency))]
+    [DefaultDllImportSearchPaths(DllImportSearchPath.AssemblyDirectory | DllImportSearchPath.System32)]
+    static extern int CallDependencySum(int a, int b);
+}

src/tests/Interop/DllImportSearchPaths/DllImportSearchPathsTest.csproj

@@ -9,6 +9,7 @@
     <Compile Include="*.cs" />
     <Compile Include="../NativeLibrary/NativeLibraryToLoad/NativeLibraryToLoad.cs" />
     <CMakeProjectReference Include="../NativeLibrary/NativeLibraryToLoad/CMakeLists.txt" />
+    <CMakeProjectReference Include="NativeLibraryWithDependency/CMakeLists.txt" />
   </ItemGroup>
 
   <PropertyGroup>
@@ -18,7 +19,9 @@
   <Target Name="SetUpSubdirectoryNative" AfterTargets="CopyNativeProjectBinaries">
     <ItemGroup>
       <NativeLibrariesToMove Include="$(OutDir)/libNativeLibrary.*" />
+      <NativeLibrariesToMove Include="$(OutDir)/libNativeLibraryWithDependency.*" />
       <NativeLibrariesToMove Include="$(OutDir)/NativeLibrary.*" />
+      <NativeLibrariesToMove Include="$(OutDir)/NativeLibraryWithDependency.*" />
     </ItemGroup>
     <Move SourceFiles="@(NativeLibrariesToMove)" DestinationFiles="@(NativeLibrariesToMove -> '$(LibrarySubdirectory)/%(Filename)%(Extension)')"/>
   </Target>

src/tests/Interop/DllImportSearchPaths/NativeLibraryWithDependency/CMakeLists.txt

@@ -0,0 +1,8 @@
+project(NativeLibraryWithDependency)
+include("${CLR_INTEROP_TEST_ROOT}/Interop.cmake")
+
+add_library(Dependency SHARED Dependency.cpp)
+target_link_libraries(Dependency PRIVATE ${LINK_LIBRARIES_ADDITIONAL})
+
+add_library(NativeLibraryWithDependency SHARED NativeLibraryWithDependency.cpp)
+target_link_libraries(NativeLibraryWithDependency PRIVATE Dependency ${LINK_LIBRARIES_ADDITIONAL})

src/tests/Interop/DllImportSearchPaths/NativeLibraryWithDependency/Dependency.cpp

@@ -0,0 +1,10 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+#include <platformdefines.h>
+
+extern "C" DLL_EXPORT int STDMETHODCALLTYPE Sum(int a, int b)
+{
+    return a + b;
+}
+

src/tests/Interop/DllImportSearchPaths/NativeLibraryWithDependency/NativeLibraryWithDependency.cpp

@@ -0,0 +1,12 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+#include <platformdefines.h>
+
+extern "C" int STDMETHODCALLTYPE Sum(int a, int b);
+
+extern "C" DLL_EXPORT int STDMETHODCALLTYPE CallDependencySum(int a, int b)
+{
+    return Sum(a, b);
+}
+