Fix native components build for Android #32800

am11 · 2020-02-25T17:26:30Z

With these fixes, native components of coreclr, installer and libraries build on Android rootfs.

Depends on: #32746 and dotnet/arcade#4915.

am11 · 2020-02-25T17:34:48Z

@janvorli, @jkotas, this is the full patch for testing. I will rebase this branch when other PRs are merged (and arcade is updated).

git clone https://github.com/am11/runtime --single-branch --branch feature/android-build --depth 1
cd runtime

# create rootfs
./eng/common/cross/build-android-rootfs.sh arm64 # or arm

# build coreclr, libraries and installer
ROOTFS_DIR=$(realpath .tools/android-rootfs/android-ndk-r21/sysroot) ./build.sh \
    --cross --arch arm64 --subsetCategory coreclr
ROOTFS_DIR=$(realpath .tools/android-rootfs/android-ndk-r21/sysroot) ./build.sh \
    --cross --arch arm64 --subsetCategory libraries
ROOTFS_DIR=$(realpath .tools/android-rootfs/android-ndk-r21/sysroot) ./build.sh \
    --cross --arch arm64 --subsetCategory installer

janvorli · 2020-02-27T15:47:33Z

eng/common/cross/toolchain.cmake

-set(CMAKE_C_COMPILER_EXTERNAL_TOOLCHAIN "${CROSS_ROOTFS}/usr")
-set(CMAKE_CXX_COMPILER_EXTERNAL_TOOLCHAIN "${CROSS_ROOTFS}/usr")
-set(CMAKE_ASM_COMPILER_EXTERNAL_TOOLCHAIN "${CROSS_ROOTFS}/usr")
+if("$ENV{__DistroRid}" MATCHES "android.*")


Would CMAKE_SYSTEM_NAME work too (and CMAKE_SYSTEM_VERSION few lines below)? I would like to try to avoid using env vars as much as possible in the CMake scripts.

Ah, I see, these things are set in this script, so it doesn't make sense. Hmm.

I wonder - isn't this information stored somewhere in the Android rootfs so that we could extract it from there?

CMAKE_SYSTEM_NAME

At this stage, the value of this variable is Linux. I have checked it by dumpping all cmake variables one line above (followed by a line message(FATAL_ERROR ">>>> ${CMAKE_SYSTEM_NAME}" to fail fast) and the ones resembling android are related to our build directory artifacts/obj.. etc.

am11 · 2020-03-01T21:16:51Z

eng/common/cross/toolchain.cmake

+    endif()
+
+    # extract platform number required by the NDK's toolchain
+    string(REGEX REPLACE ".*\\.([0-9]+)-.*" "\\1" ANDROID_PLATFORM "$ENV{__DistroRid}")


@janvorli, ANDROID_PLATFORM number is required by the NDK which expects caller to specify it before calling android.toolchain.cmake. We have two options, echo the value --cmakeargs -DANDROID_PLATFORM=$__ApiLevel it in the last line of rootfs script: https://github.com/dotnet/runtime/blob/05d7a5b9d2c014dd9c16b1fb29be29986c0e58ca/eng/common/cross/build-android-rootfs.sh#L117-L120

or use this parse-number-from-RID approach, which I have used since it is self-contained and we are using RID in other parts of the build that we have written in $__ToolchainDir/sysroot/android_platform.

am11 · 2020-03-02T00:47:57Z

src/coreclr/src/dlls/mscoree/coreclr/CMakeLists.txt

@@ -57,6 +57,10 @@ else(CLR_CMAKE_HOST_WIN32)
        set(EXPORTS_LINKER_OPTION -Wl,-exported_symbols_list,${EXPORTS_FILE})
    endif(CLR_CMAKE_TARGET_DARWIN)

+    if(CLR_CMAKE_TARGET_ANDROID AND CLR_CMAKE_HOST_ARCH_ARM)
+        set(EXPORTS_LINKER_OPTION "${EXPORTS_LINKER_OPTION} -Wl,--no-warn-shared-textrel")
+    endif()


https://android-developers.googleblog.com/2016/06/android-changes-for-ndk-developers.html - NDK picked up some shared textrels during the arm (32-bit) build of libcoreclr. Looking at some of your coreclr PRs, @janvorli, I understood that coreclr intends to comply with hardened kernels (at least in 64-bit). The (warning-as) error message was:

Warning: shared library text segment is not shareable

Maybe it could be investigated, as the rootcause might not be Android specific?

am11 · 2020-03-02T03:18:18Z

While testing these changes with #31701, I realized that I was only trying coreclr build with CLR_CROSS_COMPONENTS_BUILD=1 (moved that line from build.sh to build-commons.sh for testing then reverted, but didn't rebuild coreclr after the revert). Unsetting that variable revealed that there was more work to be done for the main coreclr product build. Rebased and pushed the additional changes (net delta: 047d29c +/- some cosmetic adjustments during the rebasing).

This does not affect libraries' native+managed and installer's native builds, which continue to succeed. The only part which is currently failing (with #31701 changes) is installer managed build, where ngen step picks up the wrong RID: #31701 (comment).

am11 · 2020-03-04T11:59:14Z

runtime (Test crossgen-comparison Linux arm checked) failure is due to 404

@janvorli, @jkotas, the arcade PR is ready for the review. Please take a look. Thanks.

am11 · 2020-03-12T06:02:56Z

The dependency PRs were merged and arcade has been updated. I have rebased my branch on top of master.
cc @janvorli, @jkotas please take a look.

There was a minor bug found in rootfs script in the docker container testing, which is fixed by dotnet/arcade#5003. The current workaround is to create an empty .tools/android-rootfs directory before calling eng/common/cross/build-android-rootfs.sh from clean clone. That can also be reviewed and merged without blocking this, the dotnetbot will sync the change seamlessly.

Thanks!

src/coreclr/src/debug/createdump/createdump.h

src/coreclr/src/pal/inc/pal_mstypes.h

src/libraries/Native/Unix/System.Native/pal_process.c

jkotas · 2020-03-13T03:46:17Z

src/libraries/Native/Unix/System.Native/pal_interfaceaddresses.c

@@ -114,7 +114,7 @@ int32_t SystemNative_EnumerateInterfaceAddresses(IPv4AddressFound onIpv4Found,
            freeifaddrs(headAddr);


@dotnet/ncl Could you please review the networking related changes?

__u32 ethtool_cmd_speed() function is not defined in NDK's header, but struct ethtool_cmd has __u16 speed and __u16 speed_hi defined there.

I have ifdef'd it for Android and just used .speed, ignoring the .speed_hi as done by Linux distros https://github.com/torvalds/linux/blob/bd2463a/include/uapi/linux/ethtool.h#L125. Not sure if there is any down side to this approach on Android.

That's the main change in networking code for Android.

jkotas · 2020-03-17T03:54:50Z

src/coreclr/src/vm/gctoclreventsink.cpp

@@ -25,6 +25,7 @@ void GCToCLREventSink::FireDynamicEvent(const char* eventName, void* payload, ui

 void GCToCLREventSink::FireGCStart_V2(uint32_t count, uint32_t depth, uint32_t reason, uint32_t type)
 {
+#ifndef TARGET_ANDROID


Why is this ifdefed out?

Changed to #ifdef FEATURE_EVENT_TRACE, as we have this feature disabled for Android (lttng-ust is not available).

This was referenced Feb 25, 2020

Fix build without FEATURE_EVENT_TRACE #32746

Merged

NTLM support in Android #32680

Closed

Move MSBuild projects out of the native build scripts #31701

Merged

janvorli reviewed Feb 27, 2020

View reviewed changes

danmoseley added area-Infrastructure os-android labels Feb 28, 2020

am11 commented Mar 1, 2020

View reviewed changes

am11 commented Mar 2, 2020

View reviewed changes

am11 marked this pull request as ready for review March 12, 2020 05:54

Fix native components build for Android

00206f8

am11 mentioned this pull request Mar 12, 2020

Add iOS build configurations #33292

Merged

jkotas reviewed Mar 12, 2020

View reviewed changes

src/coreclr/src/debug/createdump/createdump.h Outdated Show resolved Hide resolved

jkotas reviewed Mar 12, 2020

View reviewed changes

src/coreclr/src/pal/inc/pal_mstypes.h Outdated Show resolved Hide resolved

jkotas reviewed Mar 13, 2020

View reviewed changes

src/libraries/Native/Unix/System.Native/pal_process.c Outdated Show resolved Hide resolved

jkotas reviewed Mar 13, 2020

View reviewed changes

am11 added 4 commits March 13, 2020 04:28

Add cmake introspection for pthread_setcancelstate

1050d47

Address CR feedback

11cc8d5

Merge remote-tracking branch 'dotnet/master' into feature/android-build

6323e8d

Use calculated eth speed for Android

0c23d95

jkotas reviewed Mar 17, 2020

View reviewed changes

Use #ifdef FEATURE_EVENT_TRACE

b113784

jkotas approved these changes Mar 17, 2020

View reviewed changes

jkotas merged commit 91f1418 into dotnet:master Mar 17, 2020

am11 deleted the feature/android-build branch March 18, 2020 01:22

janvorli mentioned this pull request Mar 23, 2020

Rename TARGET_DARWIN to TARGET_OSX #33959

Merged

ghost locked as resolved and limited conversation to collaborators Dec 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix native components build for Android #32800

Fix native components build for Android #32800

am11 commented Feb 25, 2020

am11 commented Feb 25, 2020 •

edited

Loading

janvorli Feb 27, 2020

janvorli Feb 27, 2020

janvorli Feb 27, 2020

am11 Feb 29, 2020

am11 Mar 1, 2020

am11 Mar 2, 2020

am11 commented Mar 2, 2020

am11 commented Mar 4, 2020

am11 commented Mar 12, 2020 •

edited

Loading

jkotas Mar 13, 2020

am11 Mar 17, 2020

jkotas Mar 17, 2020

am11 Mar 17, 2020 •

edited

Loading

		@@ -114,7 +114,7 @@ int32_t SystemNative_EnumerateInterfaceAddresses(IPv4AddressFound onIpv4Found,
		freeifaddrs(headAddr);

Fix native components build for Android #32800

Fix native components build for Android #32800

Conversation

am11 commented Feb 25, 2020

am11 commented Feb 25, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

am11 commented Mar 2, 2020

am11 commented Mar 4, 2020

am11 commented Mar 12, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

am11 Mar 17, 2020 • edited Loading

Choose a reason for hiding this comment

am11 commented Feb 25, 2020 •

edited

Loading

am11 commented Mar 12, 2020 •

edited

Loading

am11 Mar 17, 2020 •

edited

Loading