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
[SR-11336] Array + key-path crasher due to -enable-ownership-stripping-after-serialization
for swiftCore
#53737
Comments
Thanks. |
Just tried with near TOT and failed to reproduce. I am going to try a snapshot and then do a ToT. |
Comment by Parker Schuh (JIRA) Bummer. I am on linux if that makes a difference. I diffed the -emit-sil with and without your change and found differences only in struct_extract -> struct_element_addr
Int.hash(into:) -> Hasher._combine(_:) in what seem to be unrelated functions. I'm building things with: utils/build-script -R |
Comment by Parker Schuh (JIRA) The original problem (that I minimized to not be tensorflow specific) is here in the tensorflow branch: https://ci-external.swift.org/job/swift-PR-TensorFlow-macOS/1908/console also on macos. Unfortunately, it depends on keypathiterable, which is not available upstream so it is not easy to separate out. |
Comment by Parker Schuh (JIRA) I'm now able to reproduce on macos with: ~/swift-source/build/Ninja-ReleaseAssert/swift-macosx-x86_64/bin/swiftc --version
Swift version 5.1-dev (LLVM af1f73e9e9, Swift cfaecfc015)
Target: x86_64-apple-darwin18.6.0 Here are the steps to reproduce. ~/swift-source/build/Ninja-ReleaseAssert/swift-macosx-x86_64/bin/swiftc test.swift && DYLD_INSERT_LIBRARIES=~/swift-source/build/Ninja-ReleaseAssert/swift-macosx-x86_64/lib/swift/macosx/x86_64/libswiftCore.dylib ./test This is probably another bug, but swiftc now always links against /usr/lib/swift/libswiftCore.dylib instead of the dylib in the build directory in macos. This implies that the bug is in libswiftCore.dylib and is just exercised by the example above. |
pschuh (JIRA User) can you provide the file? I am assuming what you posted works with only tensor flow, right? |
Comment by Parker Schuh (JIRA) Hi Michael, everything in this thread except https://ci-external.swift.org/job/swift-PR-TensorFlow-macOS/1908/console (which is just initial proof that the bug can happen on macos) is with respect to master. The original crasher still stands and is reduced to fully not depend on tensorflow. You have to be careful to make sure that the system swift dylib is not used in place of the one containing the bug. |
I can reproduce the issue from $ cat sr-11336.swift
protocol SomeProtocol {}
func test_fn(_ v: Array<Float>) -> [PartialKeyPath<Array<Float>>] {
var result: [PartialKeyPath<Array<Float>>] = []
let tmp: [PartialKeyPath<Array>] = v.indices.map { \Array<Float>[$0] }
for kp in tmp {
result.append(kp)
if let nested = v[keyPath: kp] as? SomeProtocol {
print("never reaches here")
}
}
return result
}
let container = Array<Float>([1, 2, 3])
print(test_fn(container))
$ swift sr-11336.swift
sr-11336.swift:7:12: warning: value 'nested' was defined but never used; consider replacing with boolean test
if let nested = v[keyPath: kp] as? SomeProtocol {
~~~~^~~~~~~~~ ~~~
is
[1] 3762 illegal hardware instruction $SWIFT_BIN/swift sr-11336.swift
$ swiftc sr-11336.swift && ./sr-11336
sr-11336.swift:7:12: warning: value 'nested' was defined but never used; consider replacing with boolean test
if let nested = v[keyPath: kp] as? SomeProtocol {
~~~~^~~~~~~~~ ~~~
is
[Swift.WritableKeyPath<Swift.Array<Swift.Float>, Swift.Float>, Swift.WritableKeyPath<Swift.Array<Swift.Float>, Swift.Float>, Swift.WritableKeyPath<Swift.Array<Swift.Float>, Swift.Float>]
# No crash. This is blocking downstreaming |
I suspect the issue may be related to #26608 by @gottesmm, or related patches. @gottesmm: I wonder if you have time to look into this issue, since it seems related to your patches? Here's a lit test reproducer: // RUN: %target-run-simple-swift
protocol SomeProtocol {}
func test_fn(_ v: Array<Float>) -> [PartialKeyPath<Array<Float>>] {
var result: [PartialKeyPath<Array<Float>>] = []
let tmp: [PartialKeyPath<Array>] = v.indices.map { \Array<Float>[$0] }
for kp in tmp {
result.append(kp)
if let nested = v[keyPath: kp] as? SomeProtocol {
print("never reaches here")
}
}
return result
}
let container = Array<Float>([1, 2, 3])
print(test_fn(container)) |
Attempting to narrow down the cause with git bisect start
git bisect good c907dab46d0c47ce712253776c1cf5bad744cc3f # ~pschuh's patch reverting https://github.com/apple/swift/pull/26608
git bisect bad 9a7c32a5a86d0cad43d045dce83d1cb6a57bebd4 # apple/swift:tensorflow-merge HEAD
git bisect run ./script.sh # script.sh:
#!/usr/bin/env bash
set -e
cd ..
ninja -C build/Ninja-ReleaseAssert+stdlib-Release/swift-linux-x86_64 swift swift-stdlib
# Use `status` variable to work around "git bisect run failed: exit code 139 is < 0 or >= 128".
# https://stackoverflow.com/a/22592593
status=0
$SWIFT_BIN/swift foo.swift || status=$?
if [ "$status" -eq 125 ] || [ "$status" -gt 127 ]; then
status=1
fi
exit "$status" EDIT: # good: [c907dab46d0c47ce712253776c1cf5bad744cc3f] Revert "Merge pull request #​26608 from gottesmm/ownership-onone-stdlibcore"
git bisect good c907dab46d0c47ce712253776c1cf5bad744cc3f
# bad: [9a7c32a5a86d0cad43d045dce83d1cb6a57bebd4] [Sema] Fix `Differentiable.TangentVector` derived conformances.
git bisect bad 9a7c32a5a86d0cad43d045dce83d1cb6a57bebd4
# bad: [c773364c7ccfc612ea4a296ca19b6206a20b5487] Merge pull request #​26842 from gottesmm/pr-30ea01bc2b3b5696b7ebd88b6c149bd1e7abcfe7
git bisect bad c773364c7ccfc612ea4a296ca19b6206a20b5487
# bad: [673728ed1f5484d5704e68047b4561bd10ea620c] Merge pull request #​21388 from xiaobai/remote_buildtree_exports
git bisect bad 673728ed1f5484d5704e68047b4561bd10ea620c
# bad: [746cad564ffe6886e3f7b50c13abf9bda646b799] Merge pull request #​26459 from xedin/diag-conversion-to-specified-type
git bisect bad 746cad564ffe6886e3f7b50c13abf9bda646b799
# bad: [529bec17f583db334a2410bba5fc70b3ffa155c8] Merge pull request #​26633 from compnerd/windows-test-regression
git bisect bad 529bec17f583db334a2410bba5fc70b3ffa155c8
# bad: [075e92b1828037985fc30501e359a8d2c33240a1] Merge pull request #​26576 from LucianoPAlmeida/SR-10597-name-lookup-failure-diagnostics
git bisect bad 075e92b1828037985fc30501e359a8d2c33240a1
# bad: [eb1a752ea7cd7f138a42248818ffde207cbe9809] Merge pull request #​26623 from owenv/add_newlines
git bisect bad eb1a752ea7cd7f138a42248818ffde207cbe9809
# bad: [817753a937d771d2ec80cd88350a282cfd70ad30] Merge pull request #​26554 from DougGregor/connected-components-cleanup
git bisect bad 817753a937d771d2ec80cd88350a282cfd70ad30
# good: [ab38be128d73c64d512b3634834bfab400f8d0e6] [Constraint graph] Handle orphaned constraints within connected components
git bisect good ab38be128d73c64d512b3634834bfab400f8d0e6
# bad: [8fc26349c61646833898b335c1fd619847122b5c] Merge pull request #​26615 from rintaro/ide-completion-rdar54215016
git bisect bad 8fc26349c61646833898b335c1fd619847122b5c
# good: [4353a55587dbb8dfe950e27c3963fd7b5bfca1f9] [CodeCompletion] Fix a crash in collectPossibleCalleesByQualifiedLookup
git bisect good 4353a55587dbb8dfe950e27c3963fd7b5bfca1f9
# first bad commit: [8fc26349c61646833898b335c1fd619847122b5c] Merge pull request #​26615 from rintaro/ide-completion-rdar54215016
# The "first bad commit" is a red herring.
# https://github.com/apple/swift/commit/4ecab471909a8bb101afe6f7c40f5ae7f492f17c is the true first bad commit, as known. |
I found the true cause for SR-11336: enabling This was done in 4ecab47 (for We will need to revert enabling |
This issue appears to have been fixed recently! Exact commit unknown. |
Additional Detail from JIRA
md5: 6c4c6e52724c440ca710405a852fb670
blocks:
Issue Description:
The cause is enabling
-Xfrontend -enable-ownership-stripping-after-serialization
forswiftCore
.This was done in 4ecab47 (for swiftCore), then later in 447f008 (for swiftCore and overlays).
Reproduce:
Weirdly,
swiftc sr-11336.swift && ./sr-11336
doesn't crash.The text was updated successfully, but these errors were encountered: