Skip to content

[android] A few tweaks for native compilation and to get more tests working #27167

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.

Sign up for GitHub

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
CodaFi merged 1 commit into from
Dec 6, 2019
Merged

[android] A few tweaks for native compilation and to get more tests working #27167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 1 addition & 8 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -601,13 +601,7 @@ if(SWIFT_HOST_VARIANT_SDK)
set(SWIFT_HOST_VARIANT_SDK_default "${SWIFT_HOST_VARIANT_SDK}")
else()
if("${CMAKE_SYSTEM_NAME}" STREQUAL "Linux")
# CMake on an Android host sets this to Linux, so check for the ANDROID_DATA
# environment variable to see if we're building on Android.
if(NOT "$ENV{ANDROID_DATA}" STREQUAL "")
set(SWIFT_HOST_VARIANT_SDK_default "ANDROID")
else()
set(SWIFT_HOST_VARIANT_SDK_default "LINUX")
endif()
set(SWIFT_HOST_VARIANT_SDK_default "LINUX")
elseif("${CMAKE_SYSTEM_NAME}" STREQUAL "FreeBSD")
set(SWIFT_HOST_VARIANT_SDK_default "FREEBSD")
elseif("${CMAKE_SYSTEM_NAME}" STREQUAL "CYGWIN")
Expand All @@ -617,7 +611,6 @@ else()
elseif("${CMAKE_SYSTEM_NAME}" STREQUAL "Haiku")
set(SWIFT_HOST_VARIANT_SDK_default "HAIKU")
elseif("${CMAKE_SYSTEM_NAME}" STREQUAL "Android")
# CMAKE_SYSTEM_NAME might be set this way when cross-compiling to Android.
set(SWIFT_HOST_VARIANT_SDK_default "ANDROID")
elseif("${CMAKE_SYSTEM_NAME}" STREQUAL "Darwin")
set(SWIFT_HOST_VARIANT_SDK_default "OSX")
Expand Down
10 changes: 7 additions & 3 deletions cmake/modules/AddSwift.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1105,9 +1105,13 @@ function(_add_swift_library_single target name)
PROPERTIES
INSTALL_RPATH "$ORIGIN:/usr/lib/swift/cygwin")
elseif("${SWIFTLIB_SINGLE_SDK}" STREQUAL "ANDROID")
# CMake generates incorrect rule `$SONAME_FLAG $INSTALLNAME_DIR$SONAME` for Android build on macOS cross-compile host.
# Proper linker flags constructed manually. See below variable `swiftlib_link_flags_all`.
set_target_properties("${target}" PROPERTIES NO_SONAME TRUE)
# CMake generates an incorrect rule `$SONAME_FLAG $INSTALLNAME_DIR$SONAME`
# for an Android cross-build from a macOS host. Construct the proper linker
# flags manually in add_swift_target_library instead, see there with
# variable `swiftlib_link_flags_all`.
if(SWIFTLIB_SINGLE_TARGET_LIBRARY)
set_target_properties("${target}" PROPERTIES NO_SONAME TRUE)
Copy link
Member Author

@finagolfin finagolfin Sep 13, 2019

Choose a reason for hiding this comment

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

Turns out this caused the Syntax/Parser issues because swift-syntax-parser-test no longer linked properly, as lld would try to place a dependency on lib/lib_InternalSwiftSyntaxParser.so if that host library's soname wasn't set. This fixes that by only removing the soname for target libraries on Android.

Copy link
Member Author

@finagolfin finagolfin Oct 23, 2019

Choose a reason for hiding this comment

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

@vgorloff, you added this, look good?

Copy link
Contributor

@vgorloff vgorloff Oct 23, 2019

Choose a reason for hiding this comment

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

Not 100% sure. I see that difference in junction if(SWIFTLIB_SINGLE_TARGET_LIBRARY) .... The fix I made in past also was addressed link issues when building single targets, like libswiftCore.so, libswiftSwiftOnoneSupport.so, etc. Seems should be OK.

endif()
# Only set the install RPATH if cross-compiling the host tools, in which
# case both the NDK and Sysroot paths must be set.
if(NOT "${SWIFT_ANDROID_NDK_PATH}" STREQUAL "" AND
Expand Down
2 changes: 2 additions & 0 deletions cmake/modules/SwiftConfigureSDK.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,8 @@ macro(configure_sdk_unix name architectures)
set(_swift_android_prebuilt_build linux-x86_64)
elseif(CMAKE_HOST_SYSTEM_NAME STREQUAL Windows)
set(_swift_android_prebuilt_build Windows-x86_64)
elseif(CMAKE_HOST_SYSTEM_NAME STREQUAL Android)
# When building natively on an Android host, there's no NDK or prebuilt suffix.
Copy link
Member Author

@finagolfin finagolfin Sep 13, 2019

Choose a reason for hiding this comment

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

Didn't hit this before because it was set to Linux, before I patched CMake to set it to Android on an Android host.

else()
message(SEND_ERROR "cannot cross-compile to android from ${CMAKE_HOST_SYSTEM_NAME}")
endif()
Expand Down
3 changes: 2 additions & 1 deletion lib/Driver/UnixToolChains.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ std::string
toolchains::GenericUnix::sanitizerRuntimeLibName(StringRef Sanitizer,
bool shared) const {
return (Twine("libclang_rt.") + Sanitizer + "-" +
this->getTriple().getArchName() + ".a")
this->getTriple().getArchName() +
(this->getTriple().isAndroid() ? "-android" : "") + ".a")
Copy link
Member Author

@finagolfin finagolfin Dec 4, 2019

Choose a reason for hiding this comment

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

Needed similar to clang.

.str();
}

Expand Down
4 changes: 4 additions & 0 deletions stdlib/public/Platform/bionic.modulemap.gyb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,10 @@ module SwiftGlibc [system] {
module sys {
export *

module cdefs {
header "${GLIBC_ARCH_INCLUDE_PATH}/sys/cdefs.h"
export *
}
Copy link
Member Author

@finagolfin finagolfin Dec 4, 2019

Choose a reason for hiding this comment

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

Starting with Android 6, features.h was emptied out and everything moved into sys/cdefs.h instead. This is why I had the earlier __BEGIN_DECLS and other ClangImporter issues with Foundation, which have since repeated with other modulemaps and are now fixed with this header.

Copy link
Contributor

@drodriguez drodriguez Dec 6, 2019

Choose a reason for hiding this comment

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

I would not mind accepting this patch as a standalone. If you create a new PR, mention me and I will accept.

module file {
header "${GLIBC_ARCH_INCLUDE_PATH}/sys/file.h"
export *
Expand Down
3 changes: 2 additions & 1 deletion test/Driver/multi-threaded.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
// RUN: %target-swiftc_driver -driver-print-jobs -module-name=ThisModule -wmo -num-threads 4 %S/Inputs/main.swift %s -c | %FileCheck -check-prefix=OBJECT %s
// RUN: cd %t && %target-swiftc_driver -parseable-output -module-name=ThisModule -wmo -num-threads 4 %S/Inputs/main.swift %s -c 2> %t/parseable-output
// RUN: cat %t/parseable-output | %FileCheck -check-prefix=PARSEABLE %s
// RUN: cd %t && env TMPDIR=/tmp %swiftc_driver -driver-print-jobs -module-name=ThisModule -wmo -num-threads 4 %S/Inputs/main.swift %s -o a.out | %FileCheck -check-prefix=EXEC %s
// RUN: %empty-directory(%t/tmp)
// RUN: cd %t && env TMPDIR=%t/tmp/ %swiftc_driver -driver-print-jobs -module-name=ThisModule -wmo -num-threads 4 %S/Inputs/main.swift %s -o a.out | %FileCheck -check-prefix=EXEC %s
Copy link
Contributor

@drodriguez drodriguez Dec 6, 2019

Choose a reason for hiding this comment

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

I would not mind accepting this patch as a standalone. If you create a new PR, mention me and I will accept. It is also better for other platforms than Android.

// RUN: echo "{\"%/s\": {\"llvm-bc\": \"%/t/multi-threaded.bc\", \"object\": \"%/t/multi-threaded.o\"}, \"%/S/Inputs/main.swift\": {\"llvm-bc\": \"%/t/main.bc\", \"object\": \"%/t/main.o\"}}" > %t/ofmo.json
// RUN: %target-swiftc_driver -module-name=ThisModule -wmo -num-threads 4 %S/Inputs/main.swift %s -emit-dependencies -output-file-map %t/ofmo.json -c
// RUN: cat %t/*.d | %FileCheck -check-prefix=DEPENDENCIES %s
Expand Down
7 changes: 5 additions & 2 deletions test/lit.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1021,6 +1021,9 @@ elif (run_os in ['linux-gnu', 'linux-gnueabihf', 'freebsd', 'windows-cygnus', 'w
config.target_shared_library_prefix = 'lib'
config.target_shared_library_suffix = ".so"
config.target_sdk_name = "android"
# Needed by several ParseableInterface/swift_build_sdk_interfaces tests on
# Android
config.environment['ANDROID_DATA'] = os.environ['ANDROID_DATA']
else:
lit_config.note("Testing Linux " + config.variant_triple)
config.target_object_format = "elf"
Expand Down Expand Up @@ -1239,7 +1242,7 @@ ENV_VAR_PREFIXES = {
'appletvsimulator': SIMULATOR_ENV_PREFIX,
'android': 'ANDROID_CHILD_'
}
TARGET_ENV_PREFIX = ENV_VAR_PREFIXES.get(config.target_sdk_name, "")
TARGET_ENV_PREFIX = ENV_VAR_PREFIXES.get(config.target_sdk_name, "") if not kIsAndroid else ""
Copy link
Member Author

@finagolfin finagolfin Sep 13, 2019

Choose a reason for hiding this comment

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

This prefix was why two hashing tests were failing before, as this prefix is removed by the adb script for a remote Android test run.


if 'remote_run_host' in lit_config.params:
if 'remote_run_tmpdir' not in lit_config.params:
Expand Down Expand Up @@ -1351,7 +1354,7 @@ def source_compiler_rt_libs(path):
and config.compiler_rt_platform in lib])

compiler_rt_dir = make_path(test_resource_dir, 'clang', 'lib',
platform.system().lower())
platform.system().lower() if not kIsAndroid else 'android')
Copy link
Member Author

@finagolfin finagolfin Sep 13, 2019

Choose a reason for hiding this comment

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

Several sanitizer tests' shared libraries in the compiler-rt build couldn't be found if this wasn't properly set.

source_compiler_rt_libs(compiler_rt_dir)

def check_runtime_libs(features_to_check):
Expand Down
7 changes: 7 additions & 0 deletions tools/SourceKit/cmake/modules/AddSwiftSourceKit.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,13 @@ macro(add_sourcekit_library name)
endif()
endif()

if("${CMAKE_SYSTEM_NAME}" STREQUAL "Android")
if(SOURCEKITLIB_SHARED)
set_target_properties(${name} PROPERTIES BUILD_WITH_INSTALL_RPATH TRUE)
set_target_properties(${name} PROPERTIES INSTALL_RPATH "$ORIGIN/../lib/swift/android")
endif()
endif()

if("${SOURCEKITLIB_INSTALL_IN_COMPONENT}" STREQUAL "")
if(SOURCEKITLIB_SHARED)
set(SOURCEKITLIB_INSTALL_IN_COMPONENT tools)
Expand Down
14 changes: 13 additions & 1 deletion utils/build-script-impl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -681,6 +681,18 @@ function set_build_options_for_host() {
-DSWIFT_DARWIN_DEPLOYMENT_VERSION_WATCHOS="${DARWIN_DEPLOYMENT_VERSION_WATCHOS}"
)
;;
android-*)
SWIFT_HOST_VARIANT="android"
SWIFT_HOST_VARIANT_SDK="ANDROID"
case ${host} in
android-armv7)
SWIFT_HOST_VARIANT_ARCH="armv7"
;;
android-aarch64)
SWIFT_HOST_VARIANT_ARCH="aarch64"
;;
esac
;;
*)
echo "Unknown host tools target: ${host}"
exit 1
Expand Down Expand Up @@ -2933,7 +2945,7 @@ for host in "${ALL_HOSTS[@]}"; do
fi

case ${host} in
linux-*|freebsd-*|cygwin-*|haiku-*) ;;
linux-*|freebsd-*|cygwin-*|haiku-*|android-*) ;;
*)
echo "error: --install-xctest is not supported on this platform"
exit 1
Expand Down
15 changes: 12 additions & 3 deletions utils/swift_build_sdk_interfaces.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,9 +402,18 @@ def main():
xfails = json.load(xfails_file)

make_dirs_if_needed(args.output_dir, args.dry_run)
shared_output_lock = multiprocessing.Lock()
pool = multiprocessing.Pool(args.jobs, set_up_child,
(args, shared_output_lock))
if 'ANDROID_DATA' not in os.environ:
shared_output_lock = multiprocessing.Lock()
pool = multiprocessing.Pool(args.jobs, set_up_child,
(args, shared_output_lock))
else:
# Android doesn't support Python's multiprocessing as it doesn't have
# sem_open, so switch to a ThreadPool instead.
import threading
shared_output_lock = threading.Lock()
from multiprocessing.pool import ThreadPool
pool = ThreadPool(args.jobs, set_up_child,
(args, shared_output_lock))

interface_framework_dirs = (args.interface_framework_dirs or
DEFAULT_FRAMEWORK_INTERFACE_SEARCH_PATHS)
Expand Down
8 changes: 8 additions & 0 deletions utils/swift_build_support/swift_build_support/targets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,14 @@ def host_target():
machine = platform.machine()

if system == 'Linux':
if 'ANDROID_DATA' in os.environ:
if machine.startswith('armv7'):
return StdlibDeploymentTarget.Android.armv7
elif machine == 'aarch64':
return StdlibDeploymentTarget.Android.aarch64
raise NotImplementedError('Android System with architecture '
'"%s" is not supported' % machine)

if machine == 'x86_64':
return StdlibDeploymentTarget.Linux.x86_64
elif machine == 'i686':
Expand Down