From d86cd4da6761e175a5cdc95725f8c7fdb0ba1410 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Wed, 29 Jan 2025 20:00:17 -0800 Subject: [PATCH 1/3] Don't use incompatible LOAD_LIBRARY_SEARCH flags when using LOAD_WITH_ALTERED_SEARCH_PATH --- .../NativeLibrary.NativeAot.cs | 20 ++++++++-------- src/coreclr/vm/nativelibrary.cpp | 23 +++++++++++-------- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/NativeLibrary.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/NativeLibrary.NativeAot.cs index 3abe04a60385e6..8037acd3f65ed8 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/NativeLibrary.NativeAot.cs +++ b/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; diff --git a/src/coreclr/vm/nativelibrary.cpp b/src/coreclr/vm/nativelibrary.cpp index 0f429c49a2951e..ab97367aa678c1 100644 --- a/src/coreclr/vm/nativelibrary.cpp +++ b/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; From e612989e09217f0f6d74180bfbbcd61d4f88232b Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Wed, 29 Jan 2025 20:17:25 -0800 Subject: [PATCH 2/3] Add test --- src/tests/Interop/CMakeLists.txt | 1 + .../DllImportSearchPathsTest.cs | 29 +++++++++++++++++++ .../DllImportSearchPathsTest.csproj | 3 ++ .../CMakeLists.txt | 8 +++++ .../Dependency.cpp | 10 +++++++ .../NativeLibraryWithDependency.cpp | 12 ++++++++ 6 files changed, 63 insertions(+) create mode 100644 src/tests/Interop/DllImportSearchPaths/NativeLibraryWithDependency/CMakeLists.txt create mode 100644 src/tests/Interop/DllImportSearchPaths/NativeLibraryWithDependency/Dependency.cpp create mode 100644 src/tests/Interop/DllImportSearchPaths/NativeLibraryWithDependency/NativeLibraryWithDependency.cpp diff --git a/src/tests/Interop/CMakeLists.txt b/src/tests/Interop/CMakeLists.txt index 4137b5ba16df62..55b4a35bd80e57 100644 --- a/src/tests/Interop/CMakeLists.txt +++ b/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) diff --git a/src/tests/Interop/DllImportSearchPaths/DllImportSearchPathsTest.cs b/src/tests/Interop/DllImportSearchPaths/DllImportSearchPathsTest.cs index 00c7b375a7ed77..a956143b424aca 100644 --- a/src/tests/Interop/DllImportSearchPaths/DllImportSearchPathsTest.cs +++ b/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); +} \ No newline at end of file diff --git a/src/tests/Interop/DllImportSearchPaths/DllImportSearchPathsTest.csproj b/src/tests/Interop/DllImportSearchPaths/DllImportSearchPathsTest.csproj index b98a1734c250b1..3595b4a6a25527 100644 --- a/src/tests/Interop/DllImportSearchPaths/DllImportSearchPathsTest.csproj +++ b/src/tests/Interop/DllImportSearchPaths/DllImportSearchPathsTest.csproj @@ -9,6 +9,7 @@ + @@ -18,7 +19,9 @@ + + diff --git a/src/tests/Interop/DllImportSearchPaths/NativeLibraryWithDependency/CMakeLists.txt b/src/tests/Interop/DllImportSearchPaths/NativeLibraryWithDependency/CMakeLists.txt new file mode 100644 index 00000000000000..4c0d01e49efa45 --- /dev/null +++ b/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}) diff --git a/src/tests/Interop/DllImportSearchPaths/NativeLibraryWithDependency/Dependency.cpp b/src/tests/Interop/DllImportSearchPaths/NativeLibraryWithDependency/Dependency.cpp new file mode 100644 index 00000000000000..df3376374bb7fa --- /dev/null +++ b/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 + +extern "C" DLL_EXPORT int STDMETHODCALLTYPE Sum(int a, int b) +{ + return a + b; +} + diff --git a/src/tests/Interop/DllImportSearchPaths/NativeLibraryWithDependency/NativeLibraryWithDependency.cpp b/src/tests/Interop/DllImportSearchPaths/NativeLibraryWithDependency/NativeLibraryWithDependency.cpp new file mode 100644 index 00000000000000..983c945a7ebad9 --- /dev/null +++ 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 + +extern "C" int STDMETHODCALLTYPE Sum(int a, int b); + +extern "C" DLL_EXPORT int STDMETHODCALLTYPE CallDependencySum(int a, int b) +{ + return Sum(a, b); +} + From ed38aa46b7e042e8c5f5078561d1377982d36a56 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Thu, 30 Jan 2025 10:30:19 -0800 Subject: [PATCH 3/3] Update mono --- src/mono/mono/metadata/native-library.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/mono/mono/metadata/native-library.c b/src/mono/mono/metadata/native-library.c index 1ab165b6f95cc9..b3412cbb4eeb76 100644 --- a/src/mono/mono/metadata/native-library.c +++ b/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); }