Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.
Merged
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,10 @@ 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 +248,43 @@ 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
Copy link
Member

Choose a reason for hiding this comment

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

I think it'd be worth noting explicitly that this isn't just about Linux vs OS X vs FreeBSD vs... etc. It's also about 32-bit vs 64-bit, processor architecture, etc.

with C/C++ source code, but they do not have the same ABI. e.g. Fields can be laid out
differently, constants cn have different numeric values, exports can
be named differently, etc. There are not only differences between operating systems
(Mac OS X vs. Ubuntu vs. FreeBSD), but also differences related to the underlying
processor architecture (x64 vs. x86 vs. ARM).

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 is quite
fragile and 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 -- System.*.Native.so
(aka "shims") -- are intended to be very thin layers 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.

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 in a style closer to Win32 than libc.
- If an export point has a 1:1 correspondence to the platform API, then name
it after the platform API in PascalCase (e.g. stat -> Stat, fstat -> FStat).
- If an export is not 1:1, then spell things out as we typically would in
CoreFX code (i.e. don't use abbreviations unless they come from the underlying
API.
- At first, it seemed that we'd want to use 1:1 names throughout, but it
turns out there are many cases where being strictly 1:1 isn't practical.
- Stick to data types which are guaranteed not to vary in size across flavors.
- e.g. use int32_t, int64_t from stdint.h and not int, long.

6 changes: 6 additions & 0 deletions run-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@ create_test_overlay()
exit 1
fi
find $CoreFxBins -name '*.dll' -exec cp '{}' "$OverlayDir" ";"

# Then the native CoreFX binaries
#
# TODO: Currently, CI does not build the native CoreFX components so build them here
# in the test phase for now.
( $ProjectRoot/src/Native/build.sh && cp $ProjectRoot/bin/$OS.x64.$Configuration/Native/* $OverlayDir ) || exit 1
}

copy_test_overlay()
Expand Down
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 IOInterop = "System.IO.Native";
internal const string CryptoInterop = "System.Security.Cryptography.Native";
}
}
47 changes: 47 additions & 0 deletions src/Common/src/Interop/Unix/System.IO.Native/Interop.NativeIO.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// 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 NativeIO
{
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.IOInterop, SetLastError = true)]
internal static extern int FStat(int fileDescriptor, out FileStats output);

[DllImport(Libraries.IOInterop, SetLastError = true)]
internal static extern int Stat(string path, out FileStats output);
}
}
26 changes: 26 additions & 0 deletions src/Native/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
cmake_minimum_required(VERSION 2.8.12)
project(CoreFX)

set(CMAKE_INSTALL_PREFIX $ENV{__CMakeBinDir})
set(CMAKE_INCLUDE_CURRENT_DIR ON)
set(CMAKE_C_FLAGS "-std=c11")
set(CMAKE_CXX_FLAGS "-std=c++11")
set(CMAKE_SHARED_LIBRARY_PREFIX "")
add_compile_options(-Wall -Werror -fPIC)

if (CMAKE_SYSTEM_PROCESSOR STREQUAL x86_64 OR CMAKE_SYSTEM_PROCESSOR STREQUAL amd64)
add_definitions(-DBIT64=1)
endif ()

string(TOUPPER ${CMAKE_BUILD_TYPE} UPPERCASE_CMAKE_BUILD_TYPE)
if (UPPERCASE_CMAKE_BUILD_TYPE STREQUAL DEBUG)
add_compile_options(-g -O0)
add_definitions(-DDEBUG)
elseif (UPPERCASE_CMAKE_BUILD_TYPE STREQUAL RELEASE)
add_compile_options (-O3)
add_definitions(-DNDEBUG)
else ()
message(FATAL_ERROR "Unknown build type. Set CMAKE_BUILD_TYPE to DEBUG or RELEASE.")
endif ()

add_subdirectory(System.IO.Native)
15 changes: 15 additions & 0 deletions src/Native/System.IO.Native/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@

project(System.IO.Native)

set(NATIVEIO_SOURCES
nativeio.h
nativeio.cpp
)

add_library(System.IO.Native
SHARED
${NATIVEIO_SOURCES}
)

install (TARGETS System.IO.Native DESTINATION .)

61 changes: 61 additions & 0 deletions src/Native/System.IO.Native/nativeio.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
//
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
//

#include "nativeio.h"
#include <sys/stat.h>

#if HAVE_STAT64 && !(defined(__APPLE__) && defined(_AMD64_))
# define stat_ stat64
# define fstat_ fstat64
#else
# define stat_ stat
# define fstat_ fstat
#endif

static void ConvertFileStats(const struct stat_& src, FileStats* dst)
{
dst->Flags = FILESTATS_FLAGS_NONE;
dst->Mode = src.st_mode;
Copy link
Member

Choose a reason for hiding this comment

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

Add a TODO to ensure that the mode values are appropriately converted as needed?

Copy link
Member

Choose a reason for hiding this comment

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

We should also file a issue tracking any TODO's.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@weshaggard I'm not closing #2137 until these TODOs are done. I expect a lot of churn as this comes up and want to keep it moving forward incrementally. Separate issues for everything is going to be a bit painful.

I've just added a comment to #2137 where I'll gather TODOs as they come up and make sure to drive the TODOs to zero before closing out the issue. Does that approach work for you?

Copy link
Member

Choose a reason for hiding this comment

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

Yes that works for me thanks.

dst->Uid = src.st_uid;
dst->Gid = src.st_gid;
dst->Size = src.st_size;
dst->AccessTime = src.st_atime;
dst->ModificationTime = src.st_mtime;
dst->StatusChangeTime = src.st_ctime;

#if HAVE_STAT_BIRTHTIME
dst->CreationTime = src->st_birthtime;
dst->Flags |= FILESTATS_FLAGS_HAS_CREATION_TIME;
#endif
}

extern "C"
{
int32_t Stat(const char* path, struct FileStats* output)
{
struct stat_ result;
int ret = stat_(path, &result);

if (ret == 0)
{
ConvertFileStats(result, output);
}

Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a TODO for later about errno conversion?

return ret; // TODO: errno conversion
}

int32_t FStat(int32_t fileDescriptor, FileStats* output)
{
struct stat_ result;
int ret = fstat_(fileDescriptor, &result);

if (ret == 0)
{
ConvertFileStats(result, output);
}

return ret; // TODO: errno conversion
}
}
45 changes: 45 additions & 0 deletions src/Native/System.IO.Native/nativeio.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
//
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
//

#pragma once

#include <stdint.h>

extern "C"
{
struct FileStats
{
int32_t Flags; // flags for testing if some members are present (see values below)
int32_t Mode; // protection
int32_t Uid; // user ID of owner
int32_t Gid; // group ID of owner
int64_t Size; // total size, in bytes
int64_t AccessTime; // time of last access (atime)
int64_t ModificationTime; // time of last modification (mtime)
int64_t StatusChangeTime; // time of last status change (ctime)
int64_t CreationTime; // time the file was created (birthtime)
};

enum
{
FILESTATS_FLAGS_NONE = 0,
FILESTATS_FLAGS_HAS_CREATION_TIME = 1,
};

/**
* Get file stats from a decriptor. Implemented as shim to fstat(2).
Copy link
Member

Choose a reason for hiding this comment

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

Cool, a comment like that is what I was alluding to earlier. Thanks.

*
* Returns 0 for success, -1 for failure. Sets errno on failure.
*/
int32_t FStat(int32_t fileDescriptor, FileStats* output);

/**
* Get file stats from a full path. Implemented as shim to stat(2).
*
* Returns 0 for success, -1 for failure. Sets errno on failure.
*/
int32_t Stat(const char* path, FileStats* output);
}

Loading