[SR-7441] ppc64le build started breaking for swift compiler. #49984
Comments
|
cc @milseman |
|
So tracing through the logic, this is a little weird. asowani (JIRA User) can you post the git hash where you have the stack trace from? Can you verify what is in /root/latest/swift-source/swift/stdlib/public/core/Builtin.swift, line 466? If you can't finish a full Debug build, can you try with `build-script -r`, which will keep some debug info around and assertions too? If the same as master at https://github.com/apple/swift/blob/master/stdlib/public/core/Builtin.swift#L466, then it seems like it's seeing a tagged bit pattern. This may be an issue on platforms where our runtime doesn't have support for tagged patterns in bridge objects, which we're leveraging to represent String literals and small strings. CC @jckarter and @gparker42. |
|
That seems likely. We only specifically use tagged patterns on platforms we've opted into them, like x86_64 and arm64, and the default target info without any customization would not assume anything about the address space and not use high tag bits at all. Maybe we just need to add or customize a |
|
We'd like to use the same pattern for all 64-bit ABIs. Should we just change the value of SWIFT_ABI_DEFAULT_OBJC_RESERVED_BITS_MASK based on 64-bit or something like that? What do you think is cleanest? |
|
Making it consistent would be cleanest for sure, but I don't know what the constraints if any on PPC64 platforms in the wild are. We have some guarantees that Apple ARM64 and x86_64 platforms won't use more than the low 56 bits of address space for user mode, but I don't know whether ppc platforms make any similar promises. |
|
asowani (JIRA User), do you know what ppc64le's guarantees concerning user mode address spaces are? |
|
Comment by Atul Sowani (JIRA) I re-built swift using "-r" option.When the build crashed, I ran it through gdb which yielded following: warning: Could not find DWO CU /root/latest/swift-source/buildbot_incremental/swift-linux-powerpc64le/module-cache/6HHA0BVHXTL9/SwiftShims-1IPGKDU80LD57.pcm(0x8748742808ee24fd) referenced by CU at offset 0x2e5388 [in module /root/latest/swift-source/buildbot_incremental/swift-linux-powerpc64le/lib/swift/linux/libswiftCore.so] |
|
Comment by Atul Sowani (JIRA) @milseman I am using master branch for build. /root/latest/swift-source/swift/stdlib/public/core/Builtin.swift:466 is the first _sanityCheck statement in the following code snippet: @inline(__always) |
|
Comment by Atul Sowani (JIRA) BTW, would it help if I setup a CI for Swift on ppc64le? |
|
I think that would be immensely helpful. I don't know if there's currently a policy for new CI platforms. CC @tkremenek and najacque (JIRA User). |
|
Comment by Nicole Jacque (JIRA) asowani (JIRA User) yes please! I will send you an email to discuss. |
|
Comment by Atul Sowani (JIRA) @jckarter @milseman What I learned from Power Linux kernel developers is that Power supports 4PB of user address space which is equivalent to 52 bits. If you need any specific information, please let me know. Since I am not an expert in kernel, is it possible for you/Swift community to interact with Power Linux kernel experts on a public forum? If yes, I can initiate the dialog there and provide details to both communities. Thanks! |
|
Are they able to comment on a thread at forums.swift.org? For the address space, I think we should do something like the following: This puts Swift's generic 64-bit ABI at up to 56 bits of address space (needed for String to have same representation across these platforms). Swift's generic ABI also assumes that the first 4kb of addresses are outside of user-space address space (which is unchanged). Just to double check, is this ok for PPC64? We should also check with the s390x people. Anyone know who to contact for that? |
|
Opened: #16425 |
|
Comment by Sarvesh Tamba (JIRA) Hi, We are trying to build Apple Swift on ppc64le Ubuntu16.04. We built it on ppc64le using a build script at the following link:- With this we were able to run the 'swiftc' and REPL environment for some basic "Hello World" type of code. However for complex codes, like import Foundation, it fails(probably due to Package manager issues). When we use "import Foundation" with swift, compile error occurs as below is seen: (swift) import Foundation <REPL Input>:1:8: error: no such module 'Foundation' import Foundation ^ We are trying to build Swift 4.2(since Swift v4.1 has been released now and no further development is happening on this branch.) on Power8/LE (ppc64le) using the build-toolchain, in order to build the Swift package manager and other tools. We are seeing a crash which is traceable till following function in HeapObject.cpp file: static HeapObject *swift_retain(HeapObject *object) { SWIFT_RT_TRACK_INVOCATION(object, swift_retain); if (isValidPointerForNativeRetain(object)) object->refCounts.increment(1); return object; } Beyond this debugging using simple print statements is tricky and the code flow is unclear. Breakpoints cannot be set since building code in debug mode is not possible as it runs out of resources and hangs. Any help here would be greatly appreciated. Any more outputs, errors can be shared. Looking forward to port Apple Swift on Ubuntu16.04. Regards, Sarvesh Tamba |
|
Comment by Sarvesh Tamba (JIRA) I tried to build Swift 4.2 in debug mode. I applied the patch mentioned in "#16425" for "stdlib/public/SwiftShims/System.h" as well. However, I am facing similar issues as reported above in "https://bugs.swift.org/browse/SR-7441" :- — bootstrap: note: building runtime v4 target: PackageDescription4: |
|
Comment by Sarvesh Tamba (JIRA) Hi Team, any insights on the above errors? |
|
Comment by Sarvesh Tamba (JIRA) On closer investigation, realised that the string implemetation has changed in Swift 4.2 from that in Swift 4.1 to a large extent. In the StringGuts.swift file there are comments such as "// FIXME: what about ppc64 and s390x?" indicating that the support for PowerPC(ppc64) might still be incomplete. Can we confirm that the new string implementation in Swift 4.2 has complete support for PowerPC(ppc64)? |
|
No to throw a wrench in the works, but String's implementation changed again for Swift 5 and master, where it will be ABI stable and not significantly changed in the future. |
|
Comment by Sarvesh Tamba (JIRA) Thanks Michael for replying, I suppose the 'No to throw a wrench in the works' in your reply is a confirmation that the new string implementation in Swift 4.2 isn't supported for PowerPC(ppc64le) yet. Please correct me if that's not the case. |
|
Comment by Sarvesh Tamba (JIRA) @milseman, I tried cherry-picking 9a96666 on master and built both debug & release builds on PowerPC - ppc64le. Both builds completed successfully. However the swift binaries crash in both the instances. Attached are the core dumps and gdb backtraces for your reference. |
So a crash trying to retain the BridgeObject. That BridgeObject has the high bit set, so it should early exit in the runtime. Instead, the runtime has this code: /// Return true iff the given BridgeObject is a tagged value.
static bool isBridgeObjectTaggedPointer(void *object) {
return (uintptr_t(object) & heap_object_abi::BridgeObjectTagBitsMask) != 0;
}
#endif
// Mask out the spare bits in a bridgeObject, returning the object it
// encodes.
///
/// Precondition: object does not encode a tagged pointer
static void* toPlainObject_unTagged_bridgeObject(void *object) {
return (void*)(uintptr_t(object) & ~unTaggedNonNativeBridgeObjectBits);
}
void *swift::swift_bridgeObjectRetain(void *object) {
#if SWIFT_OBJC_INTEROP
if (isObjCTaggedPointer(object) || isBridgeObjectTaggedPointer(object))
return object;
#endif
auto const objectRef = toPlainObject_unTagged_bridgeObject(object);
#if SWIFT_OBJC_INTEROP
if (!isNonNative_unTagged_bridgeObject(object)) {
swift_retain(static_cast<HeapObject *>(objectRef));
return object;
}
objc_retain(static_cast<id>(objectRef));
return object;
#else
swift_retain(static_cast<HeapObject *>(objectRef));
return object;
#endif
}@mikeash @jckarter, the bridge object tagged check only happens on platforms with ObjC interop. If that's the case, how in the world is Linux working with small strings or even string literals, both of which use tagged bridge objects? (minor note: we should assert it's not tagged in |
|
The only thing that happens after that on Linux is a call to swift_retain, which has its own early exit check for zero or negative values on 64-bit platforms. If we're deciding that the high half of address space is reserved on every 64-bit platform, we'll want to change the condition in isValidPointerForNativeRetain too. |
|
So, IIRC, we have some options: 1. Change isValidPointerForNativeRetain/// Returns true if the pointer passed to a native retain or release is valid.
/// If false, the operation should immediately return.
static inline bool isValidPointerForNativeRetain(const void *p) {
#if defined(__x86_64__) || defined(__arm64__) || defined(__aarch64__) || defined(_M_ARM64) || defined(__s390x__)
// On these platforms, except s390x, the upper half of address space is reserved for the
// kernel, so we can assume that pointer values in this range are invalid.
// On s390x it is theoretically possible to have high bit set but in practice
// it is unlikely.
return (intptr_t)p > 0;
#else
return p != nullptr;
#endif
}To just be the greater-than-zero check. 2. Change bridgeObject_retainPut in the early-exit even on platforms without ObjC interop Is my understanding that you'd prefer #1? The downside is that we'd limit ourselves to platforms that don't use the high bit (at least for user space), which is probably fine. We can always pursue #2 in the unlikely event that matters. |
|
#1 sounds good, with the caveat that we need to keep the I suspect the cavalcade of architectures in the |
|
The condition we need is "64-bit platforms where the top bit is reserved". So far that's all the ones we support, but the explicit list does force us to continue checking. Maybe we should have an |
|
That sounds good. Since this is so architecture-specific I wonder if we should just explicitly list all the 32-bit archs as well, then |
|
I thought the conclusion of #16425 was that we were unilaterally assuming that 56 bits ought to be enough for any 64-bit platform we care about, which should mean that we can always reserve the top bit in `isValidPointerForNativeRetain` on a 64-bit target. I would say we should do both #1 and #2, though, since it's non-obvious to follow the net behavior of the code otherwise. |
|
If it's possible to unify the conditions under which we use SWIFT_ABI_DEFAULT_64BIT_SPARE_BITS_MASK and reserve negative retainable pointer values, that'd be nice at minimum. |
|
Comment by Sarvesh Tamba (JIRA) Thanks a lot guys for the inputs. Here is what I tried:- Updated the platform check with following changes in "/swift/stdlib/public/runtime/HeapObject.cpp" in "static inline bool isValidPointerForNativeRetain(const void *p)" function along with cherry pick 9a96666(I guess this is already merged in the master branch by now):- #if defined(x86_64) || defined(arm64) || defined(aarch64) || defined(M_ARM64) || defined(s390x) || defined(powerpc64) || defined(ppc) || defined(PPC) || defined(LITTLE_ENDIAN) || defined(powerpc64le) || defined(ppc64le) || defined(PPC64LE_) (Note:- Need to revisit this check and keep only the appropriate ones, put all of the above to save on the build times, please excuse 🙂 ) With this the release build got successfully built and swift was able to compile simple swift code correctly:- root@swift42-debugub1604:~# ll /root/swift-source/build/Ninja-ReleaseAssert/swift-linux-powerpc64le/bin/swift root@swift42-debugub1604:~# ll /root/swift-source/build/Ninja-ReleaseAssert/swift-linux-powerpc64le/bin/swiftc root@swift42-debugub1604:~# /root/swift-source/build/Ninja-ReleaseAssert/swift-linux-powerpc64le/bin/swift
root@swift42-debugub1604:~# cat testswift.swift print(myString) root@swift42-debugub1604:~# /root/swift-source/build/Ninja-ReleaseAssert/swift-linux-powerpc64le/bin/swift testswift.swift Investigating if complex code like import Foundation or other packages work with this. Also trying to build the tool chain. |
|
Comment by Sarvesh Tamba (JIRA) Hi All, I was finally able to build the swift toolchain successfully on PowerPC(ppc64le) using "utils/build-toolchain" on master branch. root@swift42-debugub1604:~# cat testfoundation.swift import Foundation var str = "Hello, playground" Following are the final set of changes: Would highly appreciate if someone can review and confirm these changes before I can formally raise a PR for them. Thanks once again! |
|
I think PRs are the best way for us to review changes. |
|
Comment by Sarvesh Tamba (JIRA) Raised the following PRs:- swift:- swift-llvm:- swift-package-manager(swiftpm):- |
|
Comment by Sarvesh Tamba (JIRA) I also invoked the build-toolchain to run the tests as well :- Following is the current status of the tests which are failing so far:- Expected Passes : 10116 Attached are the failure details of each of the failing tests. swift-test-failures-ppc64le.txt Can anyone confirm if these test cases are supported for PowerPC64LE? |
|
Comment by Sarvesh Tamba (JIRA) Around 5 tests fail with "FileCheck error: '-' is empty." |
|
Is this still an issue? I don't think this should be on me. |
Attachment: Download
Environment
ppc64le running Ubuntu 16.04 distribution.
Additional Detail from JIRA
md5: e8e5748ebca1db8526bf68baa425ac9a
Issue Description:
I was rebuilding swift to validate my ppc64le related changes done in Swift and related repositories. I am observing crashes during swift build. The crashes are happening in swift/stdlib/public/core/Builtin.swift file and I was able to trace the cause to a new addition made under
commit 3be2faf which seems to be a new implementation for 64-bit StringGuts.
H
ere is the stack trace I am seeing in the console:
— bootstrap: note: building runtime v4 target: PackageDescription4:
Compile Swift Module ‘PackageDescription’ (9 sources)
Link PackageDescription
— bootstrap: note: building self-hosted ‘swift-build’: env SWIFT_EXEC=/root/latest/swift-source/buildbot_incremental/swiftpm-linux-powerpc64le/powerpc64le-unknown-linux/release/swiftc SWIFTPM_BUILD_DIR=/root/latest/swift-source/buildbot_incremental/swiftpm-linux-powerpc64le /root/latest/swift-source/buildbot_incremental/swiftpm-linux-powerpc64le/powerpc64le-unknown-linux/release/swift-build-stage1 --disable-sandbox -Xlinker -rpath -Xlinker $ORIGIN/…/lib/swift/linux -Xlinker -L -Xlinker /root/latest/swift-source/buildbot_incremental/foundation-linux-powerpc64le/Foundation -Xlinker -rpath -Xlinker /root/latest/swift-source/buildbot_incremental/foundation-linux-powerpc64le/Foundation -Xswiftc -I/root/latest/swift-source/buildbot_incremental/foundation-linux-powerpc64le/Foundation -Xswiftc -I/root/latest/swift-source/buildbot_incremental/foundation-linux-powerpc64le/Foundation/usr/lib/swift -Xlinker -L -Xlinker /root/latest/swift-source/buildbot_incremental/xctest-linux-powerpc64le -Xlinker -rpath -Xlinker /root/latest/swift-source/buildbot_incremental/xctest-linux-powerpc64le -Xswiftc -I/root/latest/swift-source/buildbot_incremental/xctest-linux-powerpc64le -Xlinker -L/root/latest/swift-source/buildbot_incremental/libdispatch-linux-powerpc64le/src/.libs -Xswiftc -I/root/latest/swift-source/buildbot_incremental/libdispatch-linux-powerpc64le/src -Xswiftc -I/root/latest/swift-source/buildbot_incremental/libdispatch-linux-powerpc64le/src/swift -Xswiftc -I/root/latest/swift-source/swift-corelibs-libdispatch -Xcc -fblocks -Xswiftc -enable-testing --configuration release --build-tests
Fatal error: file /root/latest/swift-source/swift/stdlib/public/core/Builtin.swift, line 466
Current stack trace:
0 libswiftCore.so 0x00003fff83344f90 swift_stdlib_reportFatalErrorInFile + 276
1 libswiftCore.so 0x00003fff83288b28 + 5933864
2 libswiftCore.so 0x00003fff8328859c + 5932444
3 libswiftCore.so 0x00003fff82e944f8 + 1787128
4 libswiftCore.so 0x00003fff832888b8 + 5933240
5 libswiftCore.so 0x00003fff832884f4 + 5932276
6 libswiftCore.so 0x00003fff82e944f8 + 1787128
7 libswiftCore.so 0x00003fff831cd3b4 + 5166004
8 libswiftCore.so 0x00003fff831cd214 + 5165588
9 libswiftCore.so 0x00003fff82e944f8 + 1787128
10 libswiftCore.so 0x00003fff82e936c0 fatalErrorMessage(__::file:line🎏) + 212
11 libswiftCore.so 0x00003fff8317b790 + 4831120
12 libswiftCore.so 0x00003fff83193840 + 4929600
13 libswiftCore.so 0x00003fff8312e308 + 4514568
14 libswiftCore.so 0x00003fff832f794c + 6388044
15 libstdc++.so.6 0x00003fff81bc2f00 __once_proxy + 52
16 libpthread.so.0 0x00003fff81e322f4 + 74484
17 libswiftCore.so 0x00003fff832f7850 swift_once + 120
18 libswiftCore.so 0x00003fff8312e380 static CommandLine.arguments.getter + 52
19 swift-build-stage1 0x00000000258dd360 + 316256
20 libc.so.6 0x00003fff8181309c + 143516
21 libc.so.6 0x00003fff818131e0 __libc_start_main + 184
— bootstrap: error: build failed with exit status -5
utils/build-script: fatal error: command terminated with a non-zero exit status 1, aborting
I tried to start swift build in debug mode, but my VM does not have enough resources to complete the build task.
The text was updated successfully, but these errors were encountered: