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-1720] Miscompile / misoptimize / Foundation bug with JSON decoding #4141

Closed
ddunbar opened this issue Jun 9, 2016 · 8 comments
Closed

[SR-1720] Miscompile / misoptimize / Foundation bug with JSON decoding #4141

ddunbar opened this issue Jun 9, 2016 · 8 comments
Assignees
@phausler
Labels
bug Foundation

Comments

@ddunbar
Copy link
Member

ddunbar commented Jun 9, 2016

Previous ID SR-1720
Radar None
Original Reporter @ddunbar
Type Bug
Status Resolved
Resolution Done
Additional Detail from JIRA
Votes 0
Component/s Foundation
Labels Bug
Assignee @phausler
Priority Medium

md5: 881ab4f14492257a2a5b9d5987ff0c18

Issue Description:

The following small test case, extracted from SwiftPM, fails non-deterministically when built with optimizations enabled. I haven't yet isolated it past this point, so this could be a memory smasher in XCTest, a bug in Foundation, or a misoptimization.

I have only reproduced this failure on Ubuntu 14.04.

This is causing non-deterministic CI test failures.

import XCTest
import Foundation

class JSONTests: XCTestCase {
    func testDecoding() {
        func decode(_ string: String) -> Any {
                let bytes = [UInt8](string.utf8)
                let data = NSData(bytes: bytes, length: bytes.count)
                return try! NSJSONSerialization.jsonObject(with: data, options: [.allowFragments])
        }

        _ = decode("false")
        _ = decode("true")
        switch decode("1") {
        case let asInt as Int:
           if asInt != 1 { fatalError("unexpected value: \(asInt)") }
        case let value:
           fatalError("unexpected type: \(value)")
        }
    }

    static var allTests = [("testDecoding", testDecoding)]
}

XCTMain([testCase(JSONTests.allTests)])

This is the test script I am using to reproduce the problems:

$ ~/public/swift-project/build/Ninja-ReleaseAssert/swift-linux-x86_64/bin/swiftc -O foo.swift -I ~/public/swift-project/build/Ninja-ReleaseAssert/foundation-linux-x86_\
64/Foundation/ -I ~/public/swift-project/build/Ninja-ReleaseAssert/foundation-linux-x86_64/Foundation/usr/lib/swift/ -I ~/public/swift-project/build/Ninja-ReleaseAssert/xctest-linux-x86_64/\
 -L ~/public/swift-project/build/Ninja-ReleaseAssert/xctest-linux-x86_64/ -L ~/public/swift-project/build/Ninja-ReleaseAssert/foundation-linux-x86_64/Foundation/ && (set -ex; for i in {0..1\
000}; do LD_LIBRARY_PATH=$(echo ~/public/swift-project/build/Ninja-ReleaseAssert/foundation-linux-x86_64/Foundation/):$(echo ~/public/swift-project/build/Ninja-ReleaseAssert/xctest-linux-x8\
6_64/) ./foo; done)

This script will build and run (once adjusted for your build directory) and usually fails in the first few iterations with something like:

++ echo /home/daniel_dunbar/public/swift-project/build/Ninja-ReleaseAssert/foundation-linux-x86_64/Foundation/
++ echo /home/daniel_dunbar/public/swift-project/build/Ninja-ReleaseAssert/xctest-linux-x86_64/
+ LD_LIBRARY_PATH=/home/daniel_dunbar/public/swift-project/build/Ninja-ReleaseAssert/foundation-linux-x86_64/Foundation/:/home/daniel_dunbar/public/swift-project/build/Ninja-ReleaseAssert/x\
ctest-linux-x86_64/
+ ./foo
Test Suite 'All tests' started at 20:36:27.472
Test Suite 'tmp.xctest' started at 20:36:27.487
Test Suite 'JSONTests' started at 20:36:27.488
Test Case 'JSONTests.testDecoding' started at 20:36:27.488
fatal error: unexpected value: 17: file foo.swift, line 16
@ddunbar
Copy link
Member Author

ddunbar commented Jun 9, 2016

/cc @phausler

@ddunbar
Copy link
Member Author

ddunbar commented Jun 9, 2016

Starting this off on Foundation, I think the compiler -O is maybe a red herring.

@phausler
Copy link
Member

let bytes = [UInt8](string.utf8)
let data = NSData(bytes: bytes, length: bytes.count)

This may result in non null terminated data from the string. You probably want

string.data(using: .utf8)

@ddunbar
Copy link
Member Author

ddunbar commented Jun 21, 2016

I wouldn't expect the data to need to be null terminated... is the Linux Foundation assuming it is?

@phausler
Copy link
Member

I think the objc one is tolerant of it, the linux version I think expects it.
The objc version transacts upon a byte buffer limited by the length of the buffer. Whereas the swift version calls into

func parseTypedNumber(_ address: UnsafePointer<UInt8>) -> (Any, IndexDistance)?

This calls out to strtol with a non-nil terminated buffer.

@ddunbar
Copy link
Member Author

ddunbar commented Jun 25, 2016

This is fixed, right?

@phausler
Copy link
Member

At least one bug around this failure has been fixed, and I added a unit test to verify (which went from failing 100% to passing 100%)

@phausler
Copy link
Member

Pushed e6f84e9 -> swift-corelibs-foundation/master

@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

@phausler phausler

Labels
bug Foundation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ddunbar @phausler