Skip to content
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-5292] Another Data slice-related crash #3842

Closed
CharlesJS opened this issue Jun 24, 2017 · 14 comments
Closed

[SR-5292] Another Data slice-related crash #3842

CharlesJS opened this issue Jun 24, 2017 · 14 comments

Comments

@CharlesJS
Copy link

CharlesJS commented Jun 24, 2017

Previous ID SR-5292
Radar rdar://problem/32983214
Original Reporter @CharlesJS
Type Bug
Status Resolved
Resolution Done
Environment

Xcode 9 beta 2; build 9M137d

Additional Detail from JIRA
Votes 0
Component/s Foundation
Labels Bug
Assignee None
Priority Medium

md5: 668b3bfde9322352f6ec59455f0a29b2

Issue Description:

If you attempt to append a Data object to another Data object, and that Data object is a slice of a larger Data object that is at least 101,582 bytes large, your app will non-reproducibly crash (although if you put it in a loop, it will happen quickly enough). The code below will demonstrate:

import Foundation

for i in 0..<Int.max {
    print("attempt number \(i)")
    
    var data1 = Data()
    let data2 = Data(count: 101582)

    data1.append(data2.suffix(8))
}

If the slice is wrapped in a Data, as in "data1.append(Data(data2.suffix(8)))", the crash does not occur.

@belkadan
Copy link

belkadan commented Jun 26, 2017

@swift-ci create

@phausler
Copy link
Member

phausler commented Jun 26, 2017

my guess is this is a duplicate of the same issue for rdar://problem/32982494 which slicing did not handle relative adjustments in RangeExpressions. suffix is a method provided by the protocol for Collection;

if you can verify that it does not reproduce with the following adjustment I am certain it is the same issue:

extension Data { 
   func suffix_fix(_ count: Int) -> Data { return self[(endIndex - count)..<endIndex] }
}

for i in 0..<Int.max {
    print("attempt number \(i)")
    
    var data1 = Data()
    let data2 = Data(count: 101582)

    data1.append(data2.suffix_fix(8))
}

Unfortunately RangeExpression's relative(to: R) method was a bit ambiguous with its documentation so we were incorrectly using the API in Data.

@phausler
Copy link
Member

phausler commented Jun 26, 2017

ASAN catches this in one pass

@CharlesJS
Copy link
Author

CharlesJS commented Jun 26, 2017

This code still crashes on Xcode 9 beta 2:

import Foundation

extension Data { 
   func suffix_fix(_ count: Int) -> Data { return self[(endIndex - count)..<endIndex] }
}

for i in 0..<Int.max {
    print("attempt number \(i)")
    
    var data1 = Data()
    let data2 = Data(count: 101582)

    data1.append(data2.suffix_fix(8))
}

I will try it on the trunk later, but as I am still waiting for it to compile, it will be a while before I can.

@phausler
Copy link
Member

phausler commented Jun 26, 2017

hmm looking at it more is we are missing the passed end range for that specific append variant

@phausler
Copy link
Member

phausler commented Jun 26, 2017

one neat trick: using swift-corelibs-foundation you can easily test most of Data's behavior without needing to build the whole swift toolchain. It makes testing and iteration pretty painless (unless you need to worry about objc interoperation)

@phausler
Copy link
Member

phausler commented Jun 26, 2017

I added an additional fix for the slice range append to apple/swift#10584 it now passes ASAN for a reduced version of the failure (without needing to allocate such a large region)

@CharlesJS
Copy link
Author

CharlesJS commented Jun 26, 2017

2 for 4 here. First two times I tried it, it crashed; second two times I tried it, it didn't.

@phausler
Copy link
Member

phausler commented Jun 26, 2017

you need the remainder of the fix in that pr

@CharlesJS
Copy link
Author

CharlesJS commented Jun 26, 2017

Okay, compiling again. See you after this all finishes.

@phausler
Copy link
Member

phausler commented Jun 26, 2017

added #1076 for swift-corelibs-foundation

@CharlesJS
Copy link
Author

CharlesJS commented Jun 27, 2017

Okay, using the real SDK as instructed in the wrong thread:

$ ./swiftc -sdk /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk test.swift
$ ./test
attempt number 0
Segmentation fault: 11

So, the crash is still in there even with the new additions to the repository that were added this evening.

@swift-ci
Copy link
Contributor

swift-ci commented Jun 27, 2017

Comment by Garric Nahapetian (JIRA)

Resolved? apple/swift#10584

@belkadan
Copy link

belkadan commented Aug 30, 2017

Yep.
master: apple/swift#10584
4.0: apple/swift#10792

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 5, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants