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-7090] Crashing with EXC_BAD_ACCESS after decoding Codable subclass #4253

Closed
swift-ci opened this issue Feb 27, 2018 · 18 comments
Closed

Comments

@swift-ci
Copy link
Contributor

Previous ID SR-7090
Radar rdar://problem/38030107
Original Reporter rishabhtayal (JIRA User)
Type Bug
Status Resolved
Resolution Duplicate

Attachment: Download

Additional Detail from JIRA
Votes 0
Component/s Compiler, Foundation
Labels Bug, Codable
Assignee @slavapestov
Priority Medium

md5: 0795ebdf5c0b062eb8bdc8bf5f938d84

duplicates:

  • SR-6468 inheriting from class with synthesized Codable implementation creates invalid code

relates to:

  • SR-7210 Codable decoder issue when inheriting from a Class conforming to Codable
  • SR-7156 Synthesized instance methods result in vtable-related crashes

Issue Description:

I am using little complicated nested json structure to parse it in my swift objects.

I am subclassing a parent class and decoding json structure. When accessing a variable from the subclass object, it crashes with a EXC_BAD_ACCESS.

Tried it with swift-4.1 toolchain. Crashes with 4.1 as well.

Please check the attached zip project for the crash.

@belkadan
Copy link

belkadan commented Mar 1, 2018

@swift-ci create

@itaiferber
Copy link
Contributor

This shouldn't compile — in fact, when I try to, I correctly get the following:

Untitled 3.swift:5:7: error: class 'Dog' has no initializers
class Dog: Animal {
      ^
Untitled 3.swift:5:7: note: did you mean to override 'init(from:)' and 'encode(to:)'?
class Dog: Animal {
      ^
Untitled 3.swift:6:9: note: stored property 'name' without initial value prevents synthesized initializers
    var name: String
        ^
                     = ""

If Dog inherits Animal's initializer, its .name property is left uninitialized, hence the crash. This seems like a regression that likely isn't related to Codable directly.

@itaiferber
Copy link
Contributor

rishabhtayal (JIRA User) Which Swift 4.1 toolchain are you using? How recent is it?

@itaiferber
Copy link
Contributor

And with that toolchain, does the following compile (and crash) for you?

class Animal {
    var type: String = "Cute"
}

class Dog : Animal {
    var name: String
}

print(Dog().name)

@swift-ci
Copy link
Contributor Author

swift-ci commented Mar 6, 2018

Comment by Rishabh Tayal (JIRA)

@itaiferber The example I gave in the Description section is just an example I made up. It may not be compileable piece of code. For a real example please check the attached zip file. It has the Xcode project.

@itaiferber
Copy link
Contributor

rishabhtayal (JIRA User) In the future, please make sure that the examples you give inline are representative of the actual problem at hand; otherwise we might waste trying to figure out what's going on based on code that doesn't actually represent the issue. I'll take a look at your project.

@swift-ci
Copy link
Contributor Author

swift-ci commented Mar 6, 2018

Comment by Rishabh Tayal (JIRA)

I will update the Description and remove the example from there.

@itaiferber
Copy link
Contributor

rishabhtayal (JIRA User) It looks like your Xcode project isn't quite standalone — I wasn't able to build it because it looks like it references a file without having copied it into the project:

error: could not read data from '/Users/itai/Downloads/CodableCrash/TTTT/Supporting Files/Info.plist': The fileInfo.plistcouldnt be opened because there is no such file.

Indeed, the attached project does not have a TTTT directory.

However, copying the relevant models out of your app into a single file and including the relevant code from the view controller appears to work just fine:

@swift-ci
Copy link
Contributor Author

swift-ci commented Mar 6, 2018

Comment by Rishabh Tayal (JIRA)

@itaiferber I will check my example project again to see why it's not compiling for you.

I am using swift 4.1 Snapshot 2018-02-26 (a) version of toolchain.

@swift-ci
Copy link
Contributor Author

swift-ci commented Mar 6, 2018

Comment by Rishabh Tayal (JIRA)

@itaiferber I have updated the example project now.

@itaiferber
Copy link
Contributor

Okay, I'm now seeing the same issue. The crash is an attempt to dereference 0x10, which is indeed not a valid pointer (this might be related to the fact that product.price is 10 and the value is being reinterpreted somehow?).

Interestingly, turning on whole-module compilation (with or without optimization) causes the issue to go away, as does sticking the implementation of TaskRecommendationItemProductSuggestion in ViewController.swift. @belkadan Does this type of thing sound familiar? I've got no stack frames showing between print(product.price) and the bad dereference in Xcode, so it's hard to tell exactly what's being dereferenced here.

@belkadan
Copy link

belkadan commented Mar 7, 2018

No idea, but that does kind of sound like it has nothing to do with Codable. If you build a TaskRecommendationItemProductSuggestion by hand in TaskRecommendationInfo.swift and return it as an Any or Any?, does it have the same problem? If so, pass it over to us.

@swift-ci
Copy link
Contributor Author

swift-ci commented Mar 9, 2018

Comment by Kazuhiro (JIRA)

I reproduced the same issue with the following code. In addition, turning on whole-module compilation (with or without optimization) or placing the code in same file causes the issue to go away.

class A: Codable {}

class B: A {
    var value: Int
    private enum CodingKeys: String, CodingKey {
        case value
    }

    init() {
        super.init()
    }

    required init(from decoder: Decoder) throws {
        let container = try decoder.container(keyedBy: CodingKeys.self)
        value = try container.decode(Int.self, forKey: .value)
        try super.init(from: decoder)
    }
}
         let b = B(value: 1)
        print(b.value) // Thread 1: EXC_BAD_ACCESS (code=EXC_I386_GPFLT)

@itaiferber
Copy link
Contributor

Hayashi (JIRA User) Thanks for trying to minimize the failure case! If this happens when creating a B without using init(from🙂, then this seems much more like a different compilation/optimization failure. It seems that your example code doesn't quite line up, though — the init() you have there should be init(value: Int), and it initializes value before calling super.init(), right?

@swift-ci
Copy link
Contributor Author

Comment by Kazuhiro (JIRA)

@itaiferber I'm sorry for posting wrong code. the followings is correct.

class A: Codable {}

class B: A {
    var value: Int
    private enum CodingKeys: String, CodingKey {
        case value
    }

    init(value: Int) { // <-
           self.value = value // <-
        super.init()
    }

    required init(from decoder: Decoder) throws {
        let container = try decoder.container(keyedBy: CodingKeys.self)
        value = try container.decode(Int.self, forKey: .value)
        try super.init(from: decoder)
    }
}

@itaiferber
Copy link
Contributor

Hayashi (JIRA User) Thanks for the trimmed down code! I see this too, and in fact, you can trim it down even further:

// Models.swift
class A : Encodable {}

class B : A {
    var value: Int
    init(value: Int) {
        self.value = value
    }
}

// main.swift
let b = B(value: 42)
print(b.value) // EXC_BAD_ACCESS (code=1, address=0xfffffffffffffff8)

Getting rid of Encodable conformance eliminates the issue, as does implementing encode(to🙂 in A or overriding it in B.

@belkadan This is similar to what we've got in rdar://problem/35647420, and is likely the same issue. Any idea what could possibly be happening here? The presence of Encodable is somehow affecting the accessor even if never used? I'll try to take a look at the SIL generated here to see if anything pops out (with my limited SIL experience), but does this sort of thing sound familiar at all?

Visibility doesn't appear to be a cause (making everything public doesn't affect this), but turning on whole-module compilation (not optimization) makes it work. Could this be an optimization issue somehow?

@belkadan
Copy link

I think the problem is that when we do vtable layout we have to know all the overridable members up front, and if any are missing we might try to dispatch to the wrong one at run time (which we just saw in SR-7156; cc @slavapestov here as well). This isn't exactly the same problem because there's something about the subclass, but it's probably the same basic idea. (And yes, it looks like the Radar is the same as well.)

@belkadan
Copy link

Consolidating.

@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

3 participants