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

Jump to bottom

[SR-5987] Data.insert(:at:) fails at runtime on sliced Data #4411

Closed
swift-ci opened this issue Sep 26, 2017 · 11 comments
Closed

[SR-5987] Data.insert(:at:) fails at runtime on sliced Data #4411

swift-ci opened this issue Sep 26, 2017 · 11 comments
Labels
bug Foundation

Comments

@swift-ci
Copy link
Contributor

Previous ID SR-5987
Radar None
Original Reporter Reitzig (JIRA User)
Type Bug
Status Closed
Resolution Invalid
Environment

Apple Swift version 4.0 (swiftlang-900.0.65 clang-900.0.37)
Target: x86_64-apple-macosx10.9

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

md5: 50a048c9a3914aed17d370f936e0f2ae

Issue Description:

var data = Data(bytes: [0, 128])
data.removeFirst(1)
data.insert(1, at: 0)

Expected: data == Data(bytes: [1,128])

Actual:

data: Data =Process 76646 stopped
* thread #​1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0)
    frame #&#8203;0: 0x000000010290ef4c libswiftFoundation.dylib`function signature specialization <Arg[0] = Owned To Guaranteed, Arg[1] = Exploded> of Foundation.Data._validateRange<A where A: Swift.RangeExpression, A.Bound == Swift.Int>(A) -> () + 268
libswiftFoundation.dylib`function signature specialization <Arg[0] = Owned To Guaranteed, Arg[1] = Exploded> of Foundation.Data._validateRange<A where A: Swift.RangeExpression, A.Bound == Swift.Int>(A) -> ():
->  0x10290ef4c <+268>: ud2    
    0x10290ef4e <+270>: nop    

libswiftFoundation.dylib`___lldb_unnamed_symbol4$$libswiftFoundation.dylib:
    0x10290ef50 <+0>:   pushq  %rbp
    0x10290ef51 <+1>:   movq   %rsp, %rbp
Target 0: (repl_swift) stopped.
 {
  _backing = <extracting data from value failed>

  _sliceRange = {
    lowerBound = <extracting data from value failed>

    upperBound = <extracting data from value failed>

  }
}

This seems to be a regression from 3.1 –> 4.0
@swift-ci
Copy link
Contributor Author

Comment by Raphael (JIRA)

On https://swift.sandbox.bluemix.net/#/repl with Swift Dev. 4.0 (Sep 5, 2017) Platform: Linux (x86_64):

Error running code: 
WARNING: Your kernel does not support swap limit capabilities or the cgroup is not mounted. Memory limited without swap.
/swiftfiles/doit.sh: line 51:    52 Illegal instruction     timeout ${TIMEOUT} .build/debug/TempCode

But no error with <= 3.1 Dev

@belkadan
Copy link

Likely fixed already. @phausler?

@phausler
Copy link
Member

yup the `data.removeFirst` is a creation of a slice and the `data.insert` was a mutation so this falls into the mutation of slices changes which as been fixed; please verify with a newer toolchain

@phausler
Copy link
Member

Can you verify with a toolchain that contains apple/swift#12011

@swift-ci
Copy link
Contributor Author

Comment by Raphael (JIRA)

(Honest) Questions:

  1. How did a bug as fundamental ❓ as this slip through testing?

  2. If it's already fixed, wasn't it fixed before 4.0 was released?

  3. When can we expect the fix to be released?

@phausler
Copy link
Member

1) Partially I presumed the collection unit tests covered similar cases (at least for mutations of slices). Turns out they did not, and I should have taken the time to write the 240+ unit tests before to validate the work further.
2) it just landed on master and is staged for the next update (4.0.1) by being in the 4.0 branch
3) that I have no answer for

4) I think to catch these types of things earlier it would be really good to have coverage reports

@swift-ci
Copy link
Contributor Author

Comment by Raphael (JIRA)

1) Ugh, 240 tests is a lot of wood. At least now all collections are much more rigorously tested, so that's great![]( Thanks for the quick fix, too)

4) This is probably a path coverage issue, though, isn't it? Each method probably works fine in isolation (code coverage close to 100% easy to achieve?). Are there path coverage tools for Swift?

2+3) Great, thanks! We'll see if we have to ship a workaround copying sliced Data explicitly.

@swift-ci
Copy link
Contributor Author

Comment by Raphael (JIRA)

Sorry to bring this up here, but I have no idea where to find this information resp. ask for it. What's the timeline for 4.0.1?
(3.0.1 release roughly six weeks after 3.0 – can we use this as precedence?)

I'm asking because there are other regressions (we use Security Framework via its low-level methods) we have not yet been able to identify the causes of, even after working this specific issue here. But they are gone with the 9.1 beta, so that's good. Anyway, we're kind of stuck at Swift 3.1 and XCode 8.x until the fixes are released. Which is ... not nice.

@swift-ci
Copy link
Contributor Author

swift-ci commented Nov 8, 2017

Comment by Raphael (JIRA)

Unfortunately, the issue persists under XCode 9.1 (9B55) with Swift version 4.0.2.

@phausler
Copy link
Member

phausler commented Nov 8, 2017

This is now correct behavior: your sample has a slight bug: instead of inserting at 0 you really are supposed to insert at the startIndex because the sliced data from removeFirst changes it from being 0 indexes to being a startIndex of 1

@swift-ci
Copy link
Contributor Author

swift-ci commented Nov 9, 2017

Comment by Raphael (JIRA)

That is an ... unfortunate design choice.

Also unfortunate is that there is no descriptive assertion; EXC_BAD_INSTRUCTION is all but impossible to debug.

Thanks for clarifying.

@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
Assignees
No one assigned
Labels
bug Foundation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@phausler @belkadan @swift-ci