Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions Documentation/coding-guidelines/interop-guidelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another big one is that exports can be named differently, making the DllImport mechanism itself quite challenging.


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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And is quite fragile.


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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: "thin layer" => "thin layers"

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.


1 change: 1 addition & 0 deletions src/Common/src/Interop/Unix/Interop.Libraries.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}
}
48 changes: 48 additions & 0 deletions src/Common/src/Interop/Unix/libcorefx_io/Interop.GetFileStats.cs
Original file line number Diff line number Diff line change
@@ -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);
}
}
42 changes: 42 additions & 0 deletions src/Common/src/Interop/Unix/libcorefx_io/Interop.libcorefx_io.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright (c) Microsoft. All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have both of these Interop.libcorefx_io.cs and Interop.GetFileStat.cs files?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:)

// 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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/System.Console/src/System.Console.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@
<Compile Include="$(CommonPath)\Interop\Unix\libc\Interop.strerror.cs">
<Link>Common\Interop\Unix\Interop.strerror.cs</Link>
</Compile>
<Compile Include="$(CommonPath)\Interop\Unix\libcoreclr\Interop.GetFileInformation.cs">
<Link>Common\Interop\Unix\Interop.GetFileInformation.cs"</Link>
<Compile Include="$(CommonPath)\Interop\Unix\libcorefx_io\Interop.GetFileStats.cs">
<Link>Common\Interop\Unix\Interop.GetFileStats.cs"</Link>
</Compile>
<Compile Include="$(CommonPath)\Interop\Unix\libcoreclr\Interop.SetConsoleCtrlHandler.cs">
<Link>Common\Interop\Unix\Interop.SetConsoleCtrlHandler.cs"</Link>
Expand Down
10 changes: 5 additions & 5 deletions src/System.Console/src/System/ConsolePal.Unix.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}
Expand Down Expand Up @@ -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
{
Expand Down