diff --git a/Documentation/coding-guidelines/interop-guidelines.md b/Documentation/coding-guidelines/interop-guidelines.md index 64250da4186b..355b0f5744dd 100644 --- a/Documentation/coding-guidelines/interop-guidelines.md +++ b/Documentation/coding-guidelines/interop-guidelines.md @@ -18,6 +18,11 @@ We have the following goals related to interop code being used in CoreFX: - This is both for good hygiene and to help keep platform-specific code separated from platform-neutral code, which is important for maximizing reusable code above PAL layers. +- Ensure maximal managed code reuse across different OS flavors which have + the same API but not the same ABI. + - This is the case for UNIX and addressing it is a work-in-progress (see issue + #2137 and section on "shims" below.) + ## Approach @@ -244,3 +249,42 @@ rather than the underlying integral type. should be named with the most discoverable name possible; if that name is a concept (e.g. Errors), it can be named using managed naming guidelines. + + +## UNIX shims + +Often, various UNIX flavors offer the same API from the point-of-view of compatibility +with C/C++ source code, but they do not have the same ABI. e.g. Fields can be laid out +differently, constants can have different numeric values, etc. + +This leaves us with a situation where we can't write portable P/Invoke declarations +that will work on all flavors, and writing separate declarations per flavor won't scale. + +To address this, we're moving to a model where all UNIX interop from corefx starts with +a P/Invoke to a C++ lib written specifically for corefx. These libs -- libcorefx_*.so +(aka "shims") -- are intended to be very thin layer over underlying platform libraries. +Generally, they are not there to add any significant abstraction, but to create a +stable ABI such that the same IL assembly can work across UNIX flavors. + +At this time, these shims are compiled in the dotnet/coreclr repository under the corefx +folder. This is temporary (issue #2301) until we add necessary infrastructure to build them +in this repository. + +Guidelines for shim C++ API: + +- Keep them as "thin"/1:1 as possible. + - We want to write the majority of code in C#. +- Never skip the shim and P/Invoke directly to the underlying platform API. It's +easy to assume something is safe/guaranteed when it isn't. +- Don't cheat and take advantage of coincidental agreement between +one flavor's ABI and the shim's ABI. +- Use PascalCase and spell things out in a style closer to Win32 than libc. + - At first, it seemed that we'd want to use 1:1 names for the shims, but it + turns out there are many cases where being strictly 1:1 isn't practical. As such, + the libraries will end up looking more self-consistent if we give them their + own style with which to express themselves. +- Stick to data types which are guaranteed not to vary in size across flavors. (Pointers +and size_t variance across bitness is OK.) + - e.g. use int32_t, int64_t from stdint.h and not int, long. + + diff --git a/src/Common/src/Interop/Unix/Interop.Libraries.cs b/src/Common/src/Interop/Unix/Interop.Libraries.cs index 8f6fb30dc6c9..4a63504c61d8 100644 --- a/src/Common/src/Interop/Unix/Interop.Libraries.cs +++ b/src/Common/src/Interop/Unix/Interop.Libraries.cs @@ -9,6 +9,7 @@ private static partial class Libraries internal const string LibCoreClr= "libcoreclr"; // CoreCLR runtime internal const string LibCrypto = "libcrypto"; // OpenSSL crypto library internal const string Zlib = "libz"; // zlib compression library + internal const string LibCoreFxIO = "libcorefx_io"; // CoreFx IO shims internal const string CryptoInterop = "System.Security.Cryptography.Native"; } } diff --git a/src/Common/src/Interop/Unix/libcorefx_io/Interop.GetFileStats.cs b/src/Common/src/Interop/Unix/libcorefx_io/Interop.GetFileStats.cs new file mode 100644 index 000000000000..0ce744b10acb --- /dev/null +++ b/src/Common/src/Interop/Unix/libcorefx_io/Interop.GetFileStats.cs @@ -0,0 +1,48 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Runtime.InteropServices; + +internal static partial class Interop +{ + internal static partial class libcorefx_io + { + internal struct FileStats + { + private FileStatsFlags Flags; + internal int Mode; + internal int Uid; + internal int Gid; + internal int Size; + internal int AccessTime; + internal int ModificationTime; + internal int StatusChangeTime; + internal int CreationTime; + } + + internal static class FileTypes + { + internal const int S_IFMT = 0xF000; + internal const int S_IFIFO = 0x1000; + internal const int S_IFCHR = 0x2000; + internal const int S_IFDIR = 0x4000; + internal const int S_IFREG = 0x8000; + internal const int S_IFLNK = 0xA000; + } + + [Flags] + internal enum FileStatsFlags + { + None = 0, + HasCreationTime = 1, + } + + + [DllImport(Libraries.LibCoreFxIO, SetLastError = true)] + internal static extern int GetFileStatsFromDescriptor(int fileDescriptor, out FileStats output); + + [DllImport(Libraries.LibCoreFxIO, SetLastError = true)] + internal static extern int GetFileStatsFromPath(string path, out FileStats output); + } +} diff --git a/src/Common/src/Interop/Unix/libcorefx_io/Interop.libcorefx_io.cs b/src/Common/src/Interop/Unix/libcorefx_io/Interop.libcorefx_io.cs new file mode 100644 index 000000000000..abb2f6719573 --- /dev/null +++ b/src/Common/src/Interop/Unix/libcorefx_io/Interop.libcorefx_io.cs @@ -0,0 +1,42 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Runtime.InteropServices; + +internal static partial class Interop +{ + internal static partial class libcorefx_io + { + internal struct FileStats + { + private Flags Flags; + internal int Mode; + internal int Uid; + internal int Gid; + internal int Size; + internal int AccessTime; + internal int ModificationTime; + internal int StatusChangeTime; + internal int CreationTime; + + internal bool HasCreationTime + { + get { return (this.Flags & Flags.HasCreationTime) != 0; } + } + + [Flags] + private enum Flags + { + None = 0, + HasCreationTime = 1, + } + } + + [DllImport(Libraries.LibCoreFxIO, SetLastError = true)] + internal static extern int GetFileStatsFromDescriptor(int fileDescriptor, out FileStats stats); + + [DllImport(Libraries.LibCoreFxIO, CharSet = CharSet.Ansi, SetLastError = true)] + internal static extern int GetFileStatsFromPath(string path, out FileStats stats); + } +} diff --git a/src/Common/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs b/src/Common/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs index f49ae4bd40e3..6f44c0d01348 100644 --- a/src/Common/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs +++ b/src/Common/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs @@ -51,13 +51,13 @@ internal static SafeFileHandle Open(string path, Interop.libc.OpenFlags flags, i // Make sure it's not a directory; we do this after opening it once we have a file descriptor // to avoid race conditions. - Interop.libcoreclr.fileinfo buf; - if (Interop.libcoreclr.GetFileInformationFromFd(fd, out buf) != 0) + Interop.libcorefx_io.FileStats stats; + if (Interop.libcorefx_io.GetFileStatsFromDescriptor(fd, out stats) != 0) { handle.Dispose(); throw Interop.GetExceptionForIoErrno(Marshal.GetLastWin32Error(), path); } - if ((buf.mode & Interop.libcoreclr.FileTypes.S_IFMT) == Interop.libcoreclr.FileTypes.S_IFDIR) + if ((stats.Mode & Interop.libcorefx_io.FileTypes.S_IFMT) == Interop.libcorefx_io.FileTypes.S_IFDIR) { handle.Dispose(); throw Interop.GetExceptionForIoErrno(Interop.Errors.EACCES, path, isDirectory: true); diff --git a/src/System.Console/src/System.Console.csproj b/src/System.Console/src/System.Console.csproj index 813b8d393add..6c20bbf99453 100644 --- a/src/System.Console/src/System.Console.csproj +++ b/src/System.Console/src/System.Console.csproj @@ -135,8 +135,8 @@ Common\Interop\Unix\Interop.strerror.cs - - Common\Interop\Unix\Interop.GetFileInformation.cs" + + Common\Interop\Unix\Interop.GetFileStats.cs" Common\Interop\Unix\Interop.SetConsoleCtrlHandler.cs" diff --git a/src/System.Console/src/System/ConsolePal.Unix.cs b/src/System.Console/src/System/ConsolePal.Unix.cs index afad581dec29..c577e9155138 100644 --- a/src/System.Console/src/System/ConsolePal.Unix.cs +++ b/src/System.Console/src/System/ConsolePal.Unix.cs @@ -91,7 +91,7 @@ private static bool ConsoleOutIsTerminal UnixConsoleStream ucs = sw.BaseStream as UnixConsoleStream; if (ucs != null) { - return ucs._handleType == Interop.libcoreclr.FileTypes.S_IFCHR; + return ucs._handleType == Interop.libcorefx_io.FileTypes.S_IFCHR; } } } @@ -378,11 +378,11 @@ internal UnixConsoleStream(string devPath, FileAccess access) try { _handle.DangerousAddRef(ref gotFd); - Interop.libcoreclr.fileinfo buf; + Interop.libcorefx_io.FileStats buf; _handleType = - Interop.libcoreclr.GetFileInformationFromFd((int)_handle.DangerousGetHandle(), out buf) == 0 ? - (buf.mode & Interop.libcoreclr.FileTypes.S_IFMT) : - Interop.libcoreclr.FileTypes.S_IFREG; // if something goes wrong, don't fail, just say it's a regular file + Interop.libcorefx_io.GetFileStatsFromDescriptor((int)_handle.DangerousGetHandle(), out buf) == 0 ? + (buf.Mode & Interop.libcorefx_io.FileTypes.S_IFMT) : + Interop.libcorefx_io.FileTypes.S_IFREG; // if something goes wrong, don't fail, just say it's a regular file } finally {