Migrate libpsl-native to managed/libc P/Invoke - #27668
#27668Migrate libpsl-native to managed/libc P/Invoke#27668adityapatwardhan wants to merge 14 commits intoPowerShell:masterPowerShell/PowerShell:masterfrom adityapatwardhan:ReplaceLibPSLNativeadityapatwardhan/PowerShell:ReplaceLibPSLNativeCopy head branch name to clipboard
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
| private static partial int Close(int fd); | ||
|
|
||
| [LibraryImport("libc", EntryPoint = "posix_spawn", StringMarshalling = StringMarshalling.Utf8)] | ||
| private static unsafe partial int PosixSpawn(out int pid, string path, IntPtr fileActions, IntPtr attr, byte** argv, byte** envp); |
There was a problem hiding this comment.
Now in .Net 11 we have StartDetached
Implementation is used PosixSpawn
There was a problem hiding this comment.
ProcessStartInfo.StartDetached in .NET 11 fully detaches the process so it survives parent exit, signal, and terminal close. That's semantically different and could interfere with SSH remoting, which needs the parent to actively communicate with the child via the redirected streams.
There was a problem hiding this comment.
If you mean redirecting stdin/stdout/stderr then RedirectStandardInput/RedirectStandardOutput/RedirectStandardError or StandardInputHandle/StandardOutputHandle/StandardErrorHandle are supported.
4079d05 to
92fc34e
Compare
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
This PR continues the migration away from libpsl-native by introducing a managed engine/Interop/Unix surface and switching multiple Unix-specific operations over to direct libc P/Invokes (with a macOS exception for syslog where unified logging semantics require the existing native shim).
Changes:
- Add new
Interop.UnixP/Invokes for filesystem, process, time, stat, user/group, and spawn operations. - Replace SSH remoting process creation from the native
ForkAndExecProcessshim to a managedposix_spawnimplementation. - Switch Linux syslog calls to
libc(while keeping macOS onlibpsl-nativeforos_log-compatible behavior).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/System.Management.Automation/utils/tracing/SysLogProvider.cs | Routes Linux syslog calls to libc while preserving macOS unified logging behavior via libpsl-native. |
| src/System.Management.Automation/engine/remoting/common/RunspaceConnectionInfo.cs | Updates SSH remoting process creation to use Interop.Unix.SpawnProcess instead of the native fork/exec shim. |
| src/System.Management.Automation/CoreCLR/CorePsPlatform.cs | Replaces multiple libpsl-native helpers with managed implementations backed by Interop.Unix. |
| src/System.Management.Automation/engine/Interop/Unix/UserGroup.cs | Adds managed getpwuid_r/getgrgid_r-based lookups for user/group names. |
| src/System.Management.Automation/engine/Interop/Unix/Time.cs | Adds settimeofday P/Invoke with a managed timeval representation. |
| src/System.Management.Automation/engine/Interop/Unix/Stat.cs | Introduces normalized stat retrieval using Linux statx and macOS stat/lstat variants. |
| src/System.Management.Automation/engine/Interop/Unix/Spawn.cs | Adds a managed posix_spawn path with optional stdio redirection and session isolation behavior. |
| src/System.Management.Automation/engine/Interop/Unix/Process.cs | Adds process/thread-related P/Invokes (kill, waitpid, gettid, pthread_threadid_np, macOS proc_pidinfo helpers). |
| src/System.Management.Automation/engine/Interop/Unix/FileSystem.cs | Adds symlink, link, and access P/Invokes for Unix filesystem operations. |
Comments suppressed due to low confidence (1)
src/System.Management.Automation/engine/Interop/Unix/Spawn.cs:181
- The code sets a hard-coded
POSIX_SPAWN_SETSIDflag value viaposix_spawnattr_setflags, butPOSIX_SPAWN_SETSIDis a non-standard extension and the accepted flag bits vary by libc version. On platforms that reject/ignore this flag,SpawnAttrSetFlagscan returnEINVAL, which would currently fail SSH remoting process creation (sinceownSession: trueis always requested). Consider a more portable approach (e.g.POSIX_SPAWN_SETPGROUP+posix_spawnattr_setpgroup, or setting SIGINT handling via spawn attributes) or at least handlingEINVALwith a clearer error/fallback.
if (ownSession)
{
short flags = OperatingSystem.IsMacOS() ? PosixSpawnSetSidMacOS : PosixSpawnSetSidLinux;
if ((rc = SpawnAttrSetFlags(attr, flags)) != 0)
{
throw new Win32Exception(rc);
}
Replace the low-risk libpsl-native functions (those not requiring
stat/passwd struct marshalling) with pure managed code and direct
LibraryImport("libc") calls, following the engine/Interop/Windows
convention with a parallel engine/Interop/Unix folder.
Migrated:
- GetErrorCategory -> managed errno->ErrorCategory table
- CreateSymLink/CreateHardLink -> libc symlink/link
- IsExecutable -> libc access(path, X_OK)
- KillProcess -> libc kill(pid, SIGKILL)
- WaitPid -> libc waitpid
- GetCurrentThreadId -> libc gettid (Linux) / pthread_threadid_np (macOS)
- SetDate -> libc settimeofday (via DateTimeOffset; drops UnixTm struct)
- Syslog Open/SysLog/Close -> libc openlog/syslog/closelog
Interop.Unix is compiled only on non-Windows (per csproj), so the
consuming bodies in CorePsPlatform are guarded with #if UNIX. The
remaining libpsl-native imports (stat, passwd/group, GetUserFromPid,
ForkAndExecProcess) are deferred to later phases.
Verified: Windows build and cross-compiled linux-x64 build both pass.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… managed libc Replace remaining libpsl-native P/Invokes with direct libc calls: - Stat.cs: architecture-independent Linux statx(2) and macOS 64-bit-inode stat/lstat, exposed as a normalized StatInfo. Wires GetCommonStat, GetCommonLStat, GetLinkCount, GetInodeData, IsSameFileSystemItem. - UserGroup.cs: getpwuid_r/getgrgid_r name resolution (GetPwUid, GetGrGid). - Process.cs: macOS proc_pidinfo helpers for GetPPid and GetUserFromPid. - CorePsPlatform.cs: all remaining psLib imports removed; migrated methods guarded with #if UNIX and #else PlatformNotSupportedException stubs so the Windows build (which excludes engine/Interop/Unix) still compiles. Compile-verified on Windows and cross-compiled linux-x64. Runtime testing on Linux/macOS still required (struct offsets, macOS symbol dispatch). Only ForkAndExecProcess (Phase 3) still uses libpsl-native. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Phase 1 syslog migration used POSIX syslog() on all Unix platforms per the
migration plan's simplicity recommendation. On macOS this broke Logging.Tests:
POSIX syslog() does not set the os_log subsystem/category, so 'log show' entries
never match the test's "category -eq $logId" filter and no items are returned.
Restore the native (libpsl-native nativesyslog.cpp) behavior on macOS by logging
through the unified logging system:
- os_log_create("com.microsoft.powershell", ident) sets subsystem + category.
- _os_log_impl(...) is invoked with a hand-built argument buffer equivalent to
what the os_log compiler macro emits for a single "%{public}s" argument.
- Severity is mapped to os_log_type_t (fault/error/debug/default) matching the
native switch. Linux continues to use POSIX syslog().
NOTE: os_log is normally a compiler-macro API; the _os_log_impl + DSO/format-string
path is compile-verified only (host is Windows). macOS CI (Logging.Tests.ps1) must
confirm 'log show' renders the eventMessage.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…uture phase
POSIX syslog() is not a valid substitute for os_log on macOS: Host/Logging.Tests.ps1
reads entries via 'log show --style json' and filters on the os_log category (= logId)
and messageType "Default". syslog() sets no category, so no entries match.
A pure-managed os_log (os_log_create + hand-rolled _os_log_impl) compiles and sets the
category correctly, but 'log show' renders a compose failure because os_log requires the
format string to be a compile-time constant located in a Mach-O __TEXT,__os_log section;
a runtime/heap format string is not resolvable by logd. This is why mature FFI bindings
ship a small C shim.
Interim resolution:
- Linux: managed libc syslog (unchanged).
- macOS: route OpenLog/SysLog/CloseLog back to libpsl-native
Native_OpenLog/Native_SysLog/Native_CloseLog.
A dedicated macOS os_log shim (compiled during the macOS build with a literal
"%{public}s" format) is documented as a future migration phase; it will let the
libpsl-native logging dependency be removed on macOS as well.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…Phase 3)
Replace the libpsl-native ForkAndExecProcess P/Invoke used by SSH based
remoting with a pure-managed implementation over libc posix_spawn(3).
- Add engine/Interop/Unix/Spawn.cs: Interop.Unix.SpawnProcess sets up
close-on-exec pipes for the requested stdin/stdout/stderr redirections,
builds posix_spawn file actions (adddup2, addchdir_np) and attributes,
and spawns the child. posix_spawn is used instead of a hand-rolled
fork/execve because running managed code between fork and exec is unsafe.
- Preserve SUPPRESS_PROCESS_SIGINT semantics via POSIX_SPAWN_SETSID so the
SSH child runs in its own session and terminal Ctrl+C (SIGINT) does not
propagate to it (flag value differs per OS: Linux 0x0080, macOS 0x0400).
- RunspaceConnectionInfo.StartSSHProcess now calls Interop.Unix.SpawnProcess;
remove the now-dead CreateProcess, ForkAndExecProcess DllImport,
AllocNullTerminatedArray, FreeArray and the SUPPRESS_PROCESS_SIGINT const.
This removes the last real [DllImport("libpsl-native")] P/Invoke; only the
macOS os_log syslog path remains (deferred to a future phase).
Compile-verified on Windows and cross-compiled linux-x64. Not runtime-tested;
SSH remoting startup, stream redirection, and Ctrl+C isolation should be
validated on Linux and macOS.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the low-risk libpsl-native functions (those not requiring
stat/passwd struct marshalling) with pure managed code and direct
LibraryImport("libc") calls, following the engine/Interop/Windows
convention with a parallel engine/Interop/Unix folder.
Migrated:
- GetErrorCategory -> managed errno->ErrorCategory table
- CreateSymLink/CreateHardLink -> libc symlink/link
- IsExecutable -> libc access(path, X_OK)
- KillProcess -> libc kill(pid, SIGKILL)
- WaitPid -> libc waitpid
- GetCurrentThreadId -> libc gettid (Linux) / pthread_threadid_np (macOS)
- SetDate -> libc settimeofday (via DateTimeOffset; drops UnixTm struct)
- Syslog Open/SysLog/Close -> libc openlog/syslog/closelog
Interop.Unix is compiled only on non-Windows (per csproj), so the
consuming bodies in CorePsPlatform are guarded with #if UNIX. The
remaining libpsl-native imports (stat, passwd/group, GetUserFromPid,
ForkAndExecProcess) are deferred to later phases.
Verified: Windows build and cross-compiled linux-x64 build both pass.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… managed libc Replace remaining libpsl-native P/Invokes with direct libc calls: - Stat.cs: architecture-independent Linux statx(2) and macOS 64-bit-inode stat/lstat, exposed as a normalized StatInfo. Wires GetCommonStat, GetCommonLStat, GetLinkCount, GetInodeData, IsSameFileSystemItem. - UserGroup.cs: getpwuid_r/getgrgid_r name resolution (GetPwUid, GetGrGid). - Process.cs: macOS proc_pidinfo helpers for GetPPid and GetUserFromPid. - CorePsPlatform.cs: all remaining psLib imports removed; migrated methods guarded with #if UNIX and #else PlatformNotSupportedException stubs so the Windows build (which excludes engine/Interop/Unix) still compiles. Compile-verified on Windows and cross-compiled linux-x64. Runtime testing on Linux/macOS still required (struct offsets, macOS symbol dispatch). Only ForkAndExecProcess (Phase 3) still uses libpsl-native. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Phase 1 syslog migration used POSIX syslog() on all Unix platforms per the
migration plan's simplicity recommendation. On macOS this broke Logging.Tests:
POSIX syslog() does not set the os_log subsystem/category, so 'log show' entries
never match the test's "category -eq $logId" filter and no items are returned.
Restore the native (libpsl-native nativesyslog.cpp) behavior on macOS by logging
through the unified logging system:
- os_log_create("com.microsoft.powershell", ident) sets subsystem + category.
- _os_log_impl(...) is invoked with a hand-built argument buffer equivalent to
what the os_log compiler macro emits for a single "%{public}s" argument.
- Severity is mapped to os_log_type_t (fault/error/debug/default) matching the
native switch. Linux continues to use POSIX syslog().
NOTE: os_log is normally a compiler-macro API; the _os_log_impl + DSO/format-string
path is compile-verified only (host is Windows). macOS CI (Logging.Tests.ps1) must
confirm 'log show' renders the eventMessage.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…uture phase
POSIX syslog() is not a valid substitute for os_log on macOS: Host/Logging.Tests.ps1
reads entries via 'log show --style json' and filters on the os_log category (= logId)
and messageType "Default". syslog() sets no category, so no entries match.
A pure-managed os_log (os_log_create + hand-rolled _os_log_impl) compiles and sets the
category correctly, but 'log show' renders a compose failure because os_log requires the
format string to be a compile-time constant located in a Mach-O __TEXT,__os_log section;
a runtime/heap format string is not resolvable by logd. This is why mature FFI bindings
ship a small C shim.
Interim resolution:
- Linux: managed libc syslog (unchanged).
- macOS: route OpenLog/SysLog/CloseLog back to libpsl-native
Native_OpenLog/Native_SysLog/Native_CloseLog.
A dedicated macOS os_log shim (compiled during the macOS build with a literal
"%{public}s" format) is documented as a future migration phase; it will let the
libpsl-native logging dependency be removed on macOS as well.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
92fc34e to
7a6a9c2
Compare
daxian-dbw
left a comment
There was a problem hiding this comment.
Compatibility Review Summary
| Severity | Issue | Risk |
|---|---|---|
| 🔴 Bug | IsSetUid/IsSetGid/IsSticky mutually exclusive check |
Files with multiple special bits misreported |
| 🟡 Compat | posix_spawn_file_actions_addchdir_np requires glibc >= 2.29 |
Runtime crash on older distros |
| 🟡 Compat | POSIX_SPAWN_SETSID is semantically broader than signal-mask suppression |
Unlikely issue for SSH, but different |
| 🟡 Compat | `LOG_PID | LOG_NDELAY` added to Linux syslog |
Detailed comments are inline below.
daxian-dbw
left a comment
There was a problem hiding this comment.
Additional minor findings (informational - no action required):
The gettid() libc wrapper is only available since glibc 2.30. Older distros (e.g. RHEL 7/8, older Ubuntu LTS) would throw EntryPointNotFoundException. Switch to the raw syscall interface which has been available in libc for decades, with syscall numbers for all supported Linux architectures (x64, arm64, s390x, ppc64le, arm). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…allback The _np extension is not available on musl libc or older glibc (< 2.29). Instead of a static P/Invoke that would throw EntryPointNotFoundException, resolve the symbol dynamically via NativeLibrary.TryGetExport at first use. When the symbol is absent, fall back to chdir() before posix_spawn() and restore the original cwd via fchdir() afterwards. This mirrors the approach used by the .NET runtime itself for Process.Start. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The statx libc wrapper was added in glibc 2.28 and musl 1.1.20. On older baselines the symbol may be absent even though the kernel syscall exists (available since Linux 4.11). Resolve the statx wrapper dynamically via NativeLibrary.TryGetExport. When absent, fall back to the raw syscall(__NR_statx) on x86_64 where the variadic and non-variadic calling conventions are compatible. On other architectures where syscall cannot be safely P/Invoked, throw a clear PlatformNotSupportedException instead of EntryPointNotFoundException. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
If pthread_threadid_np fails, the out parameter is undefined and could return 0 or garbage, breaking thread-id correlation logic. Check the return code and fall back to Environment.CurrentManagedThreadId on failure. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The previous check masked all three special bits (0xE00) then compared against a single bit constant, so IsSetUid was only true when SUID was set AND SGID+Sticky were both clear. A file with mode 6755 (SUID+SGID) would incorrectly report both IsSetUid=0 and IsSetGid=0. Test each bit independently with its own mask instead. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
|
||
| // Maps a Unix errno to the integer value of a PowerShell ErrorCategory. | ||
| // Mirrors the mapping previously provided by libpsl-native's GetErrorCategory. | ||
| internal static int GetErrorCategory(int errno) |
| Interop.Unix.Timeval tv; | ||
| tv.Seconds = new DateTimeOffset(date).ToUnixTimeSeconds(); | ||
| tv.Microseconds = 0; | ||
| return Interop.Unix.SetTimeOfDay(ref tv, IntPtr.Zero); |
There was a problem hiding this comment.
| return Interop.Unix.SetTimeOfDay(ref tv, IntPtr.Zero); | |
| return Interop.Unix.SetTimeOfDay(ref tv, timezone: IntPtr.Zero); |
| [UnmanagedFunctionPointer(CallingConvention.Cdecl, SetLastError = true)] | ||
| private delegate int StatxDelegate(int dirfd, IntPtr pathname, int flags, uint mask, out StatxBuffer buffer); | ||
|
|
||
| private static readonly StatxDelegate? s_statxWrapper = ResolveStatxWrapper(); |
There was a problem hiding this comment.
This will always be called the first time this class is used. Should it be lazy initialized?
| // convention, so we cannot safely call it and must require the libc wrapper. | ||
| if (RuntimeInformation.ProcessArchitecture == Architecture.X64) | ||
| { | ||
| long result = SyscallStatx(SYS_statx_x86_64, dirfd, pathPtr, flags, mask, out buffer); |
There was a problem hiding this comment.
This could be saved in s_statxWrapper too (with a wrapper).
| throw new PlatformNotSupportedException( | ||
| $"The 'statx' libc wrapper was not found and the raw syscall fallback is not " + | ||
| $"available on architecture '{RuntimeInformation.ProcessArchitecture}'. " + | ||
| $"Upgrade to glibc >= 2.28 or musl >= 1.1.20."); |
There was a problem hiding this comment.
I guess this should be thrown only once at s_statxWrapper initialization.
|
|
||
| internal static partial class Interop | ||
| { | ||
| internal static partial class Unix |
There was a problem hiding this comment.
Should it be compiled only on UNIX?
| const int MaxBufLen = 1024 * 1024; | ||
| int bufLen = 1024; | ||
|
|
||
| IntPtr record = Marshal.AllocHGlobal(RecordBufferSize); |
There was a problem hiding this comment.
We could use a strategy from .Net code:
https://github.com/dotnet/runtime/blob/e614b717a9de49446fa0a59c4ce7e7c051d749c6/src/libraries/System.Private.CoreLib/src/System/IO/PersistedFiles.Unix.cs#L63
- use stackalloc for most calls.
- exclude Marshal.AllocHGlobal().
| // is NOT a substitute on macOS: it does not set the os_log category the diagnostics/tests | ||
| // rely on. Linux uses libc syslog directly. | ||
| private const string libpslnative = "libpsl-native"; | ||
|
|
There was a problem hiding this comment.
Here we could replace libpsl-native with p/invokes like:
[DllImport("libsystem.dylib", EntryPoint = "os_log_create", CharSet = CharSet.Ansi)]
private static extern IntPtr os_log_create(string subsystem, string category);
[DllImport("libsystem.dylib", EntryPoint = "_os_log_impl")]
private static extern void _os_log_impl(IntPtr dylib, IntPtr log, byte type, string format, string message);
PR Summary
This pull request refactors Unix platform interop in PowerShell by removing dependencies on the native
libpsl-nativelibrary and replacing them with direct P/Invoke calls to libc and other system libraries. This change simplifies the codebase, improves maintainability, and makes Unix-specific functionality more transparent and easier to modify. The main changes are the introduction of new interop files for filesystem and process operations, and the replacement of native method calls with managed implementations.Key changes include:
Removal of
libpsl-nativedependencies and managed replacementslibpsl-nativeinCorePsPlatform.csare replaced with direct calls to libc or managed code, including file system and process operations such as creating links, checking executability, getting process/user info, and setting dates. Platform-specific logic is implemented directly in managed code. [1] [2]New Unix interop implementations
engine/Interop/Unix/FileSystem.cswith direct P/Invoke declarations forsymlink,link, andaccesssystem calls, allowing managed code to interact directly with the Unix filesystem without a native shim.engine/Interop/Unix/Process.cswith direct P/Invoke declarations forkill,waitpid,gettid, and macOS-specific process information retrieval, replacing previous native implementations.Refactoring and simplification
NonWindowsSetDateto use the new managed implementations, removing the need for unsafe code and custom struct marshaling.These changes modernize the Unix interop layer, make the code easier to maintain, and reduce reliance on external native components.
PR Context
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright header