[android] Switch to new single-header modulemap for Bionic too #35707

finagolfin · 2021-02-02T14:48:09Z

This pull gets all the same tests to pass in the Swift compiler test and validation suite when natively run on Android in the Termux app, but also gets 38 C++ interopability tests that were previously failing to pass, as was the intent of #32404. However, building libFoundationNetworking.so from the corelibs then fails with these errors:

<unknown>:0: error: fatal error encountered while reading from module 'Foundation'; please submit a bug report (https://swift.org/contributing/#reporting-bugs) and include the project
<unknown>:0: note: module 'Foundation' full misc version is '5.4(5.4)/Swift version 5.4-dev (LLVM e2976fe639d1f50, Swift f9be289b9e04b3d)'

*** DESERIALIZATION FAILURE (please include this section in any bug report) ***
top-level value not found
Cross-reference to module 'CDispatch'
... memcmp
... with type (UnsafeRawPointer, UnsafeRawPointer, Int) -> Int32
--------
1.      Swift version 5.4-dev (LLVM e2976fe639d1f50, Swift f9be289b9e04b3d)
2.      While evaluating request ExecuteSILPipelineRequest(Run pipelines { PrepareOptimizationPasses, EarlyModulePasses, HighLevel,Function+EarlyLoopOpt, HighLevel,Module+StackPromote, Serialize, MidLevel,Function, ClosureSpecialize, LowLevel,Function, LateLoopOpt, SIL Debug Info Generator } on SIL for FoundationNetworking.FoundationNetworking)
3.      While running pass #1974 SILModuleTransform "PerformanceSILLinker".
4.      While deserializing SIL function "$s10Foundation4DataV2eeoiySbAC_ACtFZ"
5.      While deserializing SIL function "memcmp"
<unknown>:0: error: unable to execute command: Aborted

I am not familiar with this error, so maybe a Swift compiler dev can chime in. I can reproduce when cross-compiling the corelibs for Android with #33724 and a couple other related pulls.

@CodaFi, @drodriguez, @hlopko, any input would be appreciated.

finagolfin · 2021-02-02T14:49:11Z

stdlib/public/Platform/SwiftGlibc.h.gyb

@@ -28,7 +28,6 @@ headers = [
  'tgmath.h',
  'time.h',
  'utmp.h',
-  'utmpx.h',


This header is included twice, so removed one.

Change the original UNSUPPORTED to XFAIL, to get signal if the test starts working. PR #35707 should allow this test to work properly.

finagolfin · 2021-05-10T18:05:36Z

I just tried this pull again with the May 3rd trunk source snapshot and got the following error this time:

<unknown>:0: error: fatal error encountered while reading from module 'Foundation'; please submit a bug report (https://swift.org/contributing/#reporting-bugs) and include the project
<unknown>:0: note: module 'Foundation' full misc version is '5.5(5.5)/Swift version 5.5-dev (LLVM 6337fa4dc365079, Swift a6915e7bfef8173)'

*** DESERIALIZATION FAILURE (please include this section in any bug report) ***
top-level value not found
Cross-reference to module 'CDispatch'
... memcmp
... with type (UnsafeRawPointer, UnsafeRawPointer, Int) -> Int32
Notes:
* 'memcmp' in module 'SwiftShims' was filtered out.
* 'memcmp' in module 'Foundation' was filtered out.
* 'memcmp' in module 'CDispatch' was filtered out.
* 'memcmp' in module 'Glibc' was filtered out.
* 'memcmp' in module 'SwiftGlibc' was filtered out.
* 'memcmp' in module 'SwiftOverlayShims' was filtered out.
* 'memcmp' in module 'CoreFoundation' was filtered out.
* 'memcmp' in module 'CFURLSessionInterface' was filtered out.
----------
1.	Swift version 5.5-dev (LLVM 6337fa4dc365079, Swift a6915e7bfef8173)
2.	While evaluating request ExecuteSILPipelineRequest(Run pipelines { PrepareOptimizationPasses, EarlyModulePasses, HighLevel,Function+EarlyLoopOpt, HighLevel,Module+StackPromote, Serialize, MidLevel,Function, ClosureSpecialize, LowLevel,Function, LateLoopOpt, SIL Debug Info Generator } on SIL for FoundationNetworking.FoundationNetworking)
3.	While running pass #1640 SILModuleTransform "PerformanceSILLinker".
4.	While deserializing SIL function "$s10Foundation4DataV2eeoiySbAC_ACtFZ"
5.	While deserializing SIL function "memcmp"
<unknown>:0: error: unable to execute command: Aborted

@xymus, you added these new messages about Glibc being wrongly filtered out in #36431, any clue what might be going on or how to fix it?

xymus · 2021-05-10T18:24:57Z

We addressed this issue for macOS with the adequately numbered PR #36666. The problem here is that memcmp is sometimes defined with _Nullable parameters and other times with non-nullable parameters via the default behavior. The error message above means that all of these module know about a memcmp but the signatures don't fit.

You can probably extend the workaround from #36666 to Android to get rid of this issue.

finagolfin · 2021-05-10T18:35:11Z

Alright, I will try that.

finagolfin · 2021-05-10T19:53:27Z

Applying that fix got the corelibs to build again and for all the same corelibs tests to pass, thanks Alexis. I've kicked off a run of the Swift compiler unit test and validation suite natively on Android, which takes much longer.

@drodriguez, do you want to try applying this updated pull to the cross-compilation Android CI too, now that it seems to work natively on Android?

finagolfin · 2021-05-10T21:42:22Z

This pull now gets 48 C++ Interop tests that were failing to pass, with no regressions in the validation suite. However, I now get a similar compiler error when building the Swift library wrapper for llbuild, this time with deserializing malloc. I will look into it.

finagolfin · 2021-12-14T04:15:39Z

Rebased and that last malloc issue appears to have gone away. Applying this patch to the Nov. 19 trunk source snapshot got 50 C++ Interop tests that were failing to pass natively on Android, while natively compiling the rest of the toolchain now works. I then applied this patch to cross-compiling the last Dec. 6 trunk snapshot for Android, finagolfin/swift-android-sdk@e174127, and was able to cross-compile the SDK and 11 Swift packages that passed their tests on the Android x86_64 emulator.

I had to patch Swift NIO for some Bionic constants in that commit, but that's it. Hopefully, I won't need to revert the C++ interoperability that @zoecarver added to libswift any longer once this pull is in.

@MaxDesiatov, would you run the CI?

@xymus, would you review?

@drodriguez, you may want to try this on your local setup that reproduces the Android CI before this is merged, though I doubt it will break that CI.

MaxDesiatov · 2021-12-14T09:13:02Z

@swift-ci please smoke test

vgorloff

PR changes working on my side, when applied on top of toolchain https://github.com/vgorloff/swift-everywhere-toolchain/releases/tag/1.0.77.

The only thing I changed – is maintained C/C++ header search paths (passed to swift build command).

# Before
-Xcc -I/usr/local/ndk/21.4.7075529/.../include/c++/v1

# After
-Xcxx -I/usr/local/ndk/21.4.7075529/.../include/c++/v1.

gottesmm

If it works for all of you, LGTM

finagolfin · 2021-12-19T17:00:25Z

@drodriguez, I just ran the compiler validation suite on linux x86_64 with a trunk commit from today, after applying this pull and #40625, for both AArch64 and armv7 and the same tests passed as on the Android CI, ie only the 13 C++ Interop tests that are currently broken on the Android CI failed (I was hoping this patch would fix those 13 tests, but no such luck).

I think you can merge this now, shouldn't break anything.

…om NIOCore Motivation: An upcoming pull, swiftlang/swift#35707, moves Android to the same single-header modulemap for Bionic as used on linux, but that doesn't work unless this casting is changed. This approach works both with the current release toolchain and with that new modulemap. Modifications: - Change how these constants are cast. - Remove them from NIOCore. Result: This repo compiles for Android and the tests pass with both modulemap approaches.

Motivation: An upcoming pull, swiftlang/swift#35707, moves Android to the same single-header modulemap for Bionic as used on linux, but that doesn't work unless this casting is changed. This approach works both with the current release toolchain and with that new modulemap. Modifications: Change how these constants are cast. Result: This repo compiles for Android and the tests pass with both modulemap approaches.

Motivation: An upcoming pull, swiftlang/swift#35707, moves Android to the same single-header modulemap for Bionic as used on linux, but that doesn't work unless this is changed. This approach works both with the current release toolchain and with that new modulemap. Modifications: Change how these Bionic constants are imported. Result: This repo compiles for Android and the tests pass with both modulemap approaches.

Motivation: An upcoming pull, swiftlang/swift#35707, moves Android to the same single-header modulemap for Bionic as used on linux, but that doesn't work unless this is changed. This approach works both with the current release toolchain and with that new modulemap. Modifications: Change how these Bionic constants are imported. Result: This repo compiles for Android and the tests pass with both modulemap approaches. Co-authored-by: Cory Benfield <[email protected]>

finagolfin · 2022-01-13T08:09:55Z

@artemcm, would you run the CI again? It hasn't been re-run since the holiday break.

artemcm · 2022-01-13T16:50:44Z

@swift-ci please test

finagolfin · 2022-01-27T05:34:08Z

Ping, @drodriguez, just waiting for you to merge this.

finagolfin · 2022-02-01T07:17:31Z

@CodaFi, would you merge? This will unbreak the build on the Android CI, and if @drodriguez has any changes he wants made, we can always add those whenever he gets back.

finagolfin · 2022-02-09T22:45:59Z

@drodriguez, can we get this in?

finagolfin · 2022-02-11T06:35:27Z

Alright, this fixed the build on the Android CI and it's able to run the host tests from the compiler validation suite again. @drodriguez, if you have any input, let us know, going to submit this for the 5.6 branch next.

finagolfin commented Feb 2, 2021

View reviewed changes

finagolfin force-pushed the droid branch from 07d5be3 to 8d2e372 Compare February 2, 2021 15:33

finagolfin mentioned this pull request Feb 4, 2021

[android] Move to the NDK's unified sysroot #34491

Merged

drodriguez mentioned this pull request Feb 7, 2021

[android][test] XFAIL memory-layout-silgen in Android ARMv7 #35816

Merged

drodriguez added a commit that referenced this pull request Feb 8, 2021

[android][test] XFAIL memory-layout-silgen in Android ARMv7 (#35816)

182a011

Change the original UNSUPPORTED to XFAIL, to get signal if the test starts working. PR #35707 should allow this test to work properly.

finagolfin force-pushed the droid branch from 8d2e372 to 4eacfea Compare May 10, 2021 19:48

finagolfin mentioned this pull request Jul 10, 2021

Fix build on FreeBSD #38335

Open

finagolfin force-pushed the droid branch from 4eacfea to f363172 Compare December 13, 2021 10:59

[android] Switch to new single-header modulemap for Bionic too

5eb29d9

finagolfin force-pushed the droid branch from f363172 to 5eb29d9 Compare December 13, 2021 17:55

finagolfin mentioned this pull request Dec 16, 2021

[stdlib] Use the single-header Glibc modulemap. #40557

Merged

vgorloff approved these changes Dec 17, 2021

View reviewed changes

gottesmm approved these changes Dec 17, 2021

View reviewed changes

finagolfin mentioned this pull request Dec 19, 2021

[android] Remove ICU build flags since that requirement was dropped in #40340 #40625

Merged

finagolfin mentioned this pull request Jan 5, 2022

Get Android constants used by Channel from Bionic instead apple/swift-nio#2026

Merged

finagolfin mentioned this pull request Jan 8, 2022

[build] Remove SWIFT_SDK_${OS}_ARCH_${ARCH}_LIBC_ARCHITECTURE_INCLUDE_DIRECTORY CMake variable #40774

Merged

CodaFi merged commit f5bc40b into swiftlang:main Feb 10, 2022

finagolfin deleted the droid branch February 11, 2022 05:45

finagolfin mentioned this pull request Feb 22, 2022

[5.6][stdlib] Switch to new single-header modulemap for Android and OpenBSD too #41510

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[android] Switch to new single-header modulemap for Bionic too #35707

[android] Switch to new single-header modulemap for Bionic too #35707

finagolfin commented Feb 2, 2021

finagolfin Feb 2, 2021

finagolfin commented May 10, 2021

xymus commented May 10, 2021

finagolfin commented May 10, 2021

finagolfin commented May 10, 2021

finagolfin commented May 10, 2021

finagolfin commented Dec 14, 2021

MaxDesiatov commented Dec 14, 2021

vgorloff left a comment •

edited

Loading

gottesmm left a comment

finagolfin commented Dec 19, 2021 •

edited

Loading

finagolfin commented Jan 13, 2022

artemcm commented Jan 13, 2022

finagolfin commented Jan 27, 2022

finagolfin commented Feb 1, 2022

finagolfin commented Feb 9, 2022

finagolfin commented Feb 11, 2022

[android] Switch to new single-header modulemap for Bionic too #35707

[android] Switch to new single-header modulemap for Bionic too #35707

Conversation

finagolfin commented Feb 2, 2021

finagolfin Feb 2, 2021

Choose a reason for hiding this comment

finagolfin commented May 10, 2021

xymus commented May 10, 2021

finagolfin commented May 10, 2021

finagolfin commented May 10, 2021

finagolfin commented May 10, 2021

finagolfin commented Dec 14, 2021

MaxDesiatov commented Dec 14, 2021

vgorloff left a comment • edited Loading

Choose a reason for hiding this comment

gottesmm left a comment

Choose a reason for hiding this comment

finagolfin commented Dec 19, 2021 • edited Loading

finagolfin commented Jan 13, 2022

artemcm commented Jan 13, 2022

finagolfin commented Jan 27, 2022

finagolfin commented Feb 1, 2022

finagolfin commented Feb 9, 2022

finagolfin commented Feb 11, 2022

vgorloff left a comment •

edited

Loading

finagolfin commented Dec 19, 2021 •

edited

Loading