Skip to content

FreeBSD compatibility patch #4804

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 40 commits into from
Jan 10, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
014963f
add define for FreeBSD getline() compat
kstaring Sep 13, 2016
f11d057
CMAKE_SDK apparenty isn't always filled when cmake runs; always use /…
kstaring Sep 13, 2016
6428295
Two fixes for FreeBSD build
kstaring Sep 14, 2016
0492374
Merge branch 'master' of https://github.com/apple/swift
kstaring Sep 14, 2016
33d2dec
Merge branch 'master' of https://github.com/apple/swift
kstaring Sep 15, 2016
afc95a5
disable handy pthread wrappers on FreeBSD since they don't compile
kstaring Sep 13, 2016
29d5de1
Merge branch 'master' of https://github.com/apple/swift
kstaring Sep 15, 2016
4936a39
Merge branch 'master' of https://github.com/apple/swift
kstaring Sep 15, 2016
4c84a33
remove extraneous extern getline
kstaring Sep 15, 2016
4dbf4de
don't try to link "dl" on FreeBSD
kstaring Sep 16, 2016
7e51593
Merge branch 'master' of https://github.com/apple/swift
kstaring Sep 16, 2016
b267fda
Merge branch 'master' of https://github.com/apple/swift
kstaring Sep 18, 2016
98a8b58
add //FIXME for FreeBSD outcommented code, add sys/event.h for kqueue…
kstaring Sep 20, 2016
7c50ccd
fix cut/paste typo 'CMAKE_SDK != "FreeBSD"' to 'CMAKE_SDK in ["FreeBS…
kstaring Sep 20, 2016
7edc310
fix unbalanced '% if'/'% end', found by gribozavr
kstaring Sep 22, 2016
47b4c7e
fixed the pthread compilation errors by following Sangjin Han's advice,
kstaring Sep 29, 2016
69085ae
Merge branch 'master' of https://github.com/apple/swift
kstaring Sep 29, 2016
ede70f2
rolled back FreeBSD /proc changes and rewrote as per @landonf suggest…
kstaring Oct 4, 2016
1a148c6
Merge branch 'master' of https://github.com/apple/swift
kstaring Oct 13, 2016
8bda672
Merge branch 'master' of https://github.com/apple/swift
Oct 15, 2016
07166c0
revert outcommented code-- the code was outcommented due to the pthre…
kstaring Oct 21, 2016
419c832
default ld.gold on FreeBSD for as long as the ld linker problem exist…
kstaring Oct 21, 2016
db8878e
incoorporate @landonf and @dcci's suggestions wrt dynamic sysct KERN_…
kstaring Oct 21, 2016
22c0b4a
fix ICU include paths. Please check this commit and verify that the p…
kstaring Oct 21, 2016
f3b612a
Merge branch 'master' of https://github.com/apple/swift
kstaring Oct 21, 2016
fb70a1b
incoorporate @jeremyandrews' suggestion
kstaring Oct 22, 2016
c1676a1
Merge branch 'master' of https://github.com/apple/swift
kstaring Oct 22, 2016
0b30970
Merge branch 'master' of https://github.com/kstaring/swift
kstaring Oct 24, 2016
97d3fba
change some variables to make it more clear - when comparing with swi…
kstaring Oct 24, 2016
5384630
don't require ICU on Darwin as per jrose-apple's comment
kstaring Oct 25, 2016
7cebd11
Merge branch 'master' of https://github.com/kstaring/swift
kstaring Oct 25, 2016
6613f07
Update RaceTest.swift
kstaring Oct 25, 2016
197b64f
re-instate newline?
kstaring Oct 25, 2016
134bd2e
don't hard-force ld.gold as per @jrose-apple 's suggestion
kstaring Oct 25, 2016
536c1ac
Merge branch 'master' of https://github.com/apple/swift
kstaring Oct 31, 2016
0039860
add cast to convey to the compiler which overload to use
kstaring Nov 17, 2016
5462fb2
different solution for detecting icu (not needed on Darwin, required …
kstaring Nov 17, 2016
6a121fb
Merge branch 'master' of https://github.com/apple/swift
kstaring Nov 17, 2016
9c53bb2
fix stupid outcomment which shouldn't have been committed :-/
kstaring Nov 17, 2016
cd52ed6
fix erroneous logic pointed and commas out by @jrose-apple and @llvm-…
kstaring Dec 13, 2016
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
2 changes: 1 addition & 1 deletion cmake/modules/FindICU.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ foreach(MODULE ${ICU_FIND_COMPONENTS})

find_path(ICU_${MODULE}_INCLUDE_DIR unicode
HINTS ${PC_ICU_${MODULE}_INCLUDEDIR} ${PC_ICU_${MODULE}_INCLUDE_DIRS})
set(ICU_${MODULE}_INCLUDE_DIR ${ICU_${MODULE}_INCLUDE_DIR})
set(ICU_${MODULE}_INCLUDE_DIRS ${ICU_${MODULE}_INCLUDE_DIR})

find_library(ICU_${MODULE}_LIBRARY NAMES icu${module}
HINTS ${PC_ICU_${MODULE}_LIBDIR} ${PC_ICU_${MODULE}_LIBRARY_DIRS})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,13 @@ public var _stdlib_PTHREAD_BARRIER_SERIAL_THREAD: CInt {
}

public struct _stdlib_pthread_barrier_t {
#if CYGWIN || os(FreeBSD)
var mutex: UnsafeMutablePointer<pthread_mutex_t?>?
var cond: UnsafeMutablePointer<pthread_cond_t?>?
#else
var mutex: UnsafeMutablePointer<pthread_mutex_t>?
var cond: UnsafeMutablePointer<pthread_cond_t>?
#endif

/// The number of threads to synchronize.
var count: CUnsignedInt = 0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,15 @@ internal func invokeBlockContext(
return context.run()
}

#if CYGWIN || os(FreeBSD)
public typealias _stdlib_pthread_attr_t = UnsafePointer<pthread_attr_t?>
#else
public typealias _stdlib_pthread_attr_t = UnsafePointer<pthread_attr_t>
#endif

/// Block-based wrapper for `pthread_create`.
public func _stdlib_pthread_create_block<Argument, Result>(
_ attr: UnsafePointer<pthread_attr_t>?,
_ attr: _stdlib_pthread_attr_t?,
_ start_routine: @escaping (Argument) -> Result,
_ arg: Argument
) -> (CInt, pthread_t?) {
Expand Down
22 changes: 22 additions & 0 deletions stdlib/public/Platform/glibc.modulemap.gyb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ module SwiftGlibc [system] {
link "util"
% end

% if CMAKE_SDK != "FREEBSD":
link "dl"
% end

// C standard library
module C {
Expand All @@ -35,6 +37,8 @@ module SwiftGlibc [system] {
header "${GLIBC_INCLUDE_PATH}/complex.h"
export *
}
% end
% if CMAKE_SDK in ["LINUX", "CYGWIN"]:
module pty {
header "${GLIBC_INCLUDE_PATH}/pty.h"

Choose a reason for hiding this comment

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

How about including libutil.h instead?
(And for utmp.h instead including utmpx.h?)

export *
Expand All @@ -44,6 +48,16 @@ module SwiftGlibc [system] {
export *
}
% end
% if CMAKE_SDK == "FREEBSD":
module pty {
header "${GLIBC_INCLUDE_PATH}/libutil.h"
export *
}
module utmp {
header "${GLIBC_INCLUDE_PATH}/utmpx.h"
export *
}
% end

% if CMAKE_SDK in ["LINUX", "ANDROID", "CYGWIN"]:
module features {
Expand Down Expand Up @@ -355,10 +369,12 @@ module SwiftGlibc [system] {
header "${GLIBC_ARCH_INCLUDE_PATH}/sys/select.h"
export *
}
% if CMAKE_SDK != "FREEBSD":
module sendfile {
header "${GLIBC_ARCH_INCLUDE_PATH}/sys/sendfile.h"
export *
}
% end
module socket {
header "${GLIBC_ARCH_INCLUDE_PATH}/sys/socket.h"
export *
Expand All @@ -379,6 +395,12 @@ module SwiftGlibc [system] {
header "${GLIBC_ARCH_INCLUDE_PATH}/sys/types.h"
export *
}
% if CMAKE_SDK in ["FREEBSD"]:
module event {
header "${GLIBC_ARCH_INCLUDE_PATH}/sys/event.h"
export *
}
% end
module uio {
header "${GLIBC_ARCH_INCLUDE_PATH}/sys/uio.h"
export *
Expand Down
12 changes: 12 additions & 0 deletions stdlib/public/stubs/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,18 @@ set(LLVM_OPTIONAL_SOURCES
${swift_stubs_objc_sources}
${swift_stubs_unicode_normalization_sources})

# ICU isn't required on Darwin, but is on every other platform.
# Now in case we're cross-compiling from Darwin for another platform,
# the find_package should still be executed.
if(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
set(icu_required "")
else()
set(icu_required "REQUIRED")
endif()

find_package(ICU ${icu_required} COMPONENTS uc)
set(ICU_UC_LIBRARY "")
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems at odds with what Android and the newer port-to-Windows work (cc @modocache again and @hughbe). I don't understand what the 'REQUIRED' is doing, either—the code doesn't match the comment, which doesn't seem to be the desired behavior anyway.

(Also, CMake doesn't use commas.)

Copy link
Contributor

Choose a reason for hiding this comment

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

The way I'm doing it on Windows is a bit different, as we don't have a build-script so I had to make things simple.
Finding ICU currently requires pkgconfig which is not readily available on Windows.
Instead, you pass -DSWIFT_WINDOWS_ICU and CMake finds the lib and include directory according to your architecture.

See this: https://github.com/apple/swift/pull/5904/files#diff-af3b638bc2a3e6c650974192a53c7291R594

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The intention of the "REQUIRED" if/else was to make find_package() not fail in case we're compiling on Darwin (since it isn't needed on that platform anyway), but fail on other platforms when no ICU library is found.
Thanks for the references @jrose-apple and @hughbe, I'll try to get something better pushed in a day.

Copy link

Choose a reason for hiding this comment

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

@kstaring politely poke, any progress👀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having looked at the Android and Windows solution, it's difficult for me to find the proper way forward.

In other words, both examples pointed to don't seem to provide solutions for the problem on FreeBSD (or other platforms for which the compiler doesn't have a default include path set where ICU can be found). find_package() provides the correct include paths on FreeBSD, but the include flags aren't used for Android and Windows.

It would be far easiest to take the Android solution route and simply add -I/usr/local/include somewhere (e.g. in AddSwift.cmake), but that doesn't really feel like a good solution given that find_package() would provide the correct path and not some hardcoded path...

So, if someone could comment (@jrose-apple ?) on the route that is deemed most correct, that would be greatly appreciated.

Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, the reason the find_package() is present in the stubs/CMakeLists.txt is exactly because the include flags are needed at that point.

Copy link

Choose a reason for hiding this comment

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

@kstaring Glad you have the BSD sprite to get it right, no worries, please take your time

Copy link
Contributor

Choose a reason for hiding this comment

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

Deferring to @llvm-beanz and maybe @modocache.

Copy link
Contributor

Choose a reason for hiding this comment

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

The logic here WRT "REQUIRES" is opposite the comment and the existing code in the standard library.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason I was totally looking over the problem pointed to by @jrose-apple .. It took @llvm-beanz 's comments to make me realise the problematic REQUIRED logic..
I was under the impression that the whole find_package() function call was deemed inappropriate, hence my comments and misunderstanding.
@jrose-apple , @llvm-beanz : can you please look at commit 6c355d74138c1adeebb6d7ed33e38601b2a79160 ? It fixes the problematic logic.


set(swift_stubs_c_compile_flags ${SWIFT_RUNTIME_CORE_CXX_FLAGS})
list(APPEND swift_stubs_c_compile_flags -DswiftCore_EXPORTS)

Expand Down
53 changes: 52 additions & 1 deletion stdlib/public/stubs/CommandLine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ extern "C" char ** _swift_stdlib_getUnsafeArgvArgc(int *outArgLen) {
*outArgLen = *_NSGetArgc();
return *_NSGetArgv();
}
#elif defined(__linux__) || defined(__CYGWIN__) || defined(__FreeBSD__)
#elif defined(__linux__) || defined(__CYGWIN__)
SWIFT_RUNTIME_STDLIB_INTERFACE
extern "C" char ** _swift_stdlib_getUnsafeArgvArgc(int *outArgLen) {
assert(outArgLen != nullptr);
Expand Down Expand Up @@ -104,6 +104,57 @@ extern "C" char ** _swift_stdlib_getUnsafeArgvArgc(int *outArgLen) {
*outArgLen = __argc;
return __argv;
}
#elif defined(__FreeBSD__)
#include <errno.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/param.h>
#include <sys/sysctl.h>

SWIFT_RUNTIME_STDLIB_INTERFACE
extern "C" char ** _swift_stdlib_getUnsafeArgvArgc(int *outArgLen) {
assert(outArgLen != nullptr);

if (_swift_stdlib_ProcessOverrideUnsafeArgv) {
*outArgLen = _swift_stdlib_ProcessOverrideUnsafeArgc;
return _swift_stdlib_ProcessOverrideUnsafeArgv;
}

char *argPtr = NULL; // or use ARG_MAX? 8192 is used in LLDB though..
int mib[4] = { CTL_KERN, KERN_PROC, KERN_PROC_ARGS, getpid() };
size_t argPtrSize = 0;
for (int i = 0; i < 3 && !argPtr; i++) { // give up after 3 tries
if (sysctl(mib, 4, NULL, &argPtrSize, NULL, 0) != -1) {
argPtr = (char *)malloc(argPtrSize);
if (sysctl(mib, 4, argPtr, &argPtrSize, NULL, 0) == -1) {
free(argPtr);
argPtr = NULL;
if (errno != ENOMEM)
break;
}
} else {
break;
}
}
if (!argPtr)
swift::fatalError(0, "fatal error: could not retrieve commandline "
"arguments: sysctl: %s.\n", strerror(errno));

char *curPtr = argPtr;
char *endPtr = argPtr + argPtrSize;

std::vector<char *> argvec;
for (; curPtr < endPtr; curPtr += strlen(curPtr) + 1)
argvec.push_back(strdup(curPtr));
*outArgLen = argvec.size();
char **outBuf = (char **)calloc(argvec.size() + 1, sizeof(char *));
std::copy(argvec.begin(), argvec.end(), outBuf);
outBuf[argvec.size()] = nullptr;

free(argPtr);

return outBuf;
}
#else // __ANDROID__; Add your favorite arch's command line arg grabber here.
SWIFT_RUNTIME_STDLIB_INTERFACE
extern "C" char ** _swift_stdlib_getUnsafeArgvArgc(int *outArgLen) {
Expand Down
4 changes: 4 additions & 0 deletions tools/swift-demangle/swift-demangle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
//
//===----------------------------------------------------------------------===//

#if defined(__FreeBSD__)
#define _WITH_GETLINE
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

Changes to this file LGTM, feel free to submit as a separate PR if you would like them to be merged immediately.

Copy link
Member

Choose a reason for hiding this comment

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

This particular bit LGTM too.


#include "swift/Basic/DemangleWrappers.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/Support/CommandLine.h"
Expand Down
1 change: 1 addition & 0 deletions utils/build-script-impl
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,7 @@ function set_build_options_for_host() {
SWIFT_HOST_VARIANT="freebsd"
SWIFT_HOST_VARIANT_SDK="FREEBSD"
SWIFT_HOST_VARIANT_ARCH="x86_64"
USE_GOLD_LINKER=1
;;
cygwin-x86_64)
SWIFT_HOST_VARIANT="windows"
Expand Down