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-4393] Miscompilation: object modified after being freed (Swift 3.1 regression) #46972
Comments
@swift-ci create |
Looks like that what cause the crash is something related with return (parameter, item.value ?? "") Workaround: if let value = item.value {
return (parameter, value)
} else {
return (parameter, "")
} |
If it could help this is a shorter version of (probably) the same crash: import Foundation
enum T: String {
case a
}
var c = URLComponents()
c.queryItems = [
URLQueryItem(name: "a", value: "1"),
URLQueryItem(name: "a", value: "1")
]
c.queryItems?.flatMap { item -> (T, String)? in
if let parameter = T(rawValue: item.name) {
return (parameter, item.value ?? "")
} else {
return nil
}
} |
This is a bug in RedundantLoadElimination. It's eliminating loads of the array storage pointer across reallocation of the array. I've been working on the general debug-ability of this pass. |
The problem may be in escape analysis. We have an Array copy method taking a local array %5 as its argument. // function_ref specialized Array._copyToNewBuffer(oldCount : Int) -> ()
%172 = function_ref
%173 = apply %172(%160, %5)
: $@convention(method) (Int, @inout Array<(StringEnum, String)>) -> () We then have an address-type block argument %151: bb22(%151 : $*Builtin.BridgeObject) Which is derived from the same local array: %99 = struct_element_addr %5
: $*Array<(StringEnum, String)>, #Array._buffer
%100 = struct_element_addr %99
: $*_ArrayBuffer<(StringEnum, String)>, #_ArrayBuffer._storage
%101 = struct_element_addr %100
: $*_BridgeStorage<_ContiguousArrayStorageBase, _NSArrayCore>, #_BridgeStorage.rawValue
br bb21(%101 : $*Builtin.BridgeObject)
bb21(%137 : $*Builtin.BridgeObject)
br bb22(%137 : $*Builtin.BridgeObject)
bb22(%151 : $*Builtin.BridgeObject) However, the connection graph does not think apply is a use point. I'll work on hand-coding a SIL test case. |
Helpful RLE debugging support should be merged in soon: I've reduced this to a .sil test case. See testrle.sil. It could See the RLE output in testrle.out.sil. These two loads are being eliminated: bb12: // Preds: bb11 bb10
%63 = load %49 : $*Builtin.BridgeObject
%64 = load %49 : $*Builtin.BridgeObject
cond_br undef, bb13, bb2
FORWARD %63 = argument of bb12 : $Builtin.BridgeObject
to %64 = load %49 : $*Builtin.BridgeObject
FORWARD %63 = argument of bb12 : $Builtin.BridgeObject
to %65 = load %49 : $*Builtin.BridgeObject ...even though bb11 overwrites the memory during Array.copyToNewBuffer. The current suspect is: It seems the the apply should be a use point of the local Array object, but it is not. I'm waiting for a build before I can debug further. Meanwhile I'll ask Erik to take a look. |
Reproducer:
|
Are you adding the Swift test case as well? Otherwise this could regress for a different reason in the future. |
Thanks for the sil file. |
I verified this is fixed by commit 2fce87c EscapeAnalysis: fix a wrong use-point detection. This caused redundant load elimination to remove a load although the value is overwritten in a called function. fixes SR-4393 It also fixes SR-4497, and is likely the cause of other recent bugs. I actually cannot reproduce crash from the original test case; at |
done in 2fce87c |
You ignored my previous comment: why is this reduced Swift code not added as a test case? You fixed that SIL issue, but the same Swift code could regress again in a future version of Swift since it's not part of the test suite, and nobody would notice until getting it released. |
I actually tried for several hours yesterday to reproduce with the original test case unsuccessfully. Instead, I attached a new `reduced.swift` test case that exposes the same problem the same way but doesn't depend on Foundation. However, there's nothing special about that swift code. The same bug is now showing up in multiple Swift test cases. Reproducing it has more to do with the stdlib code than the test code. My swift test case is unlikely to expose the same issue in future versions of stdlib/compiler. So while I agree with the motivation for source-level test, I don't really see an interesting .swift test case here and don't have time to invest in a stdlib-independent version of the source test. That said, there should be a process for adding tests derived from external source if you think it's an interesting test. Erik will follow up on that. |
If you want your swift code to be added to the compiler tests, I would recommend that you create a pull request for https://github.com/apple/swift If you need any help with that, please let me know. |
Attachment: Download
Environment
Apple Swift version 3.1 (swiftlang-802.0.48 clang-802.0.38)
Target: x86_64-apple-macosx10.9
Additional Detail from JIRA
md5: f6a66d588871a047e6ed660ea42d4841
is duplicated by:
Issue Description:
main.swift
lib.swift
This is a reduced version of the real crash, which I ran with the address sanitizer, and this is the full output: https://gist.github.com/NachoSoto/8d991d1dec478eb0e1fc4f2b294e2cba.
The text was updated successfully, but these errors were encountered: