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

[Swift] Whole-module optimization may crash the Swift compiler with some apps? #1109

Closed
CodeEagle opened this Issue Jan 25, 2016 · 12 comments

Comments

Projects
None yet
5 participants
@CodeEagle

CodeEagle commented Jan 25, 2016

This block of code cause: Segmentation fault: 11

    import Foundation
    import AsyncDisplayKit

    //MARK:- ASLayoutable
    public extension ASLayoutable {

        func spec() -> ASLayoutSpec {
            return ASStaticLayoutSpec(children: [self])
        }

        func test() {
            let layout = ASLayoutSpec()
            let staticLayuot = layout.spec()
        }
    }

info

0  swift                    0x000000010618bfbb llvm::sys::PrintStackTrace(__sFILE*) + 43
1  swift                    0x000000010618c6fb SignalHandler(int) + 379
2  libsystem_platform.dylib 0x00007fff98e98eaa _sigtramp + 26
3  libsystem_platform.dylib 0x00007fee641294f0 _sigtramp + 3408463456
4  swift                    0x000000010494e31a swift::Substitution::subst(swift::ModuleDecl*, llvm::ArrayRef<swift::Substitution>, llvm::DenseMap<swift::TypeBase*, swift::Type, llvm::DenseMapInfo<swift::TypeBase*>, llvm::detail::DenseMapPair<swift::TypeBase*, swift::Type> >&, llvm::DenseMap<swift::ArchetypeType*, llvm::ArrayRef<swift::ProtocolConformance*>, llvm::DenseMapInfo<swift::ArchetypeType*>, llvm::detail::DenseMapPair<swift::ArchetypeType*, llvm::ArrayRef<swift::ProtocolConformance*> > >&) const + 1002
5  swift                    0x000000010494deff swift::Substitution::subst(swift::ModuleDecl*, swift::GenericParamList*, llvm::ArrayRef<swift::Substitution>) const + 1679
6  swift                    0x00000001046c638c swift::TypeSubstCloner<swift::GenericCloner>::remapSubstitution(swift::Substitution) + 60
7  swift                    0x00000001046c6265 swift::SILCloner<swift::GenericCloner>::getOpConformances(swift::ArchetypeType*, swift::CanType, llvm::ArrayRef<swift::ProtocolConformance*>) + 85
8  swift                    0x00000001046c616f swift::SILCloner<swift::GenericCloner>::getOpConformancesForExistential(swift::CanType, swift::CanType, llvm::ArrayRef<swift::ProtocolConformance*>) + 143
9  swift                    0x00000001046c0ecf swift::SILCloner<swift::GenericCloner>::visitInitExistentialRefInst(swift::InitExistentialRefInst*) + 79
10 swift                    0x00000001046b9beb swift::SILCloner<swift::GenericCloner>::visitSILBasicBlock(swift::SILBasicBlock*) + 59
11 swift                    0x00000001046b95ca swift::GenericCloner::populateCloned() + 522
12 swift                    0x00000001046c780c swift::GenericCloner::cloneFunction(swift::SILFunction*, llvm::DenseMap<swift::TypeBase*, swift::Type, llvm::DenseMapInfo<swift::TypeBase*>, llvm::detail::DenseMapPair<swift::TypeBase*, swift::Type> >&, llvm::DenseMap<swift::TypeBase*, swift::Type, llvm::DenseMapInfo<swift::TypeBase*>, llvm::detail::DenseMapPair<swift::TypeBase*, swift::Type> >&, llvm::StringRef, swift::ApplySite, std::__1::function<void (swift::SILInstruction*, swift::SILInstruction*)>) + 412
13 swift                    0x00000001046c74e1 swift::trySpecializeApplyOfGeneric(swift::ApplySite, swift::SILFunction*&, llvm::SmallVectorImpl<std::__1::pair<swift::ApplySite, swift::ApplySite> >&) + 1073
14 swift                    0x000000010451df03 (anonymous namespace)::SILPerformanceInliner::devirtualizeAndSpecializeApplies(llvm::SmallVectorImpl<swift::ApplySite>&, swift::CallGraphAnalysis*, swift::SILModuleTransform*, llvm::SmallVectorImpl<swift::SILFunction*>&) + 1539
15 swift                    0x000000010451b588 (anonymous namespace)::SILPerformanceInlinerPass::run() + 1544
16 swift                    0x00000001044e8d99 swift::SILPassManager::runOneIteration() + 1689
17 swift                    0x00000001044e72d2 swift::runSILOptimizationPasses(swift::SILModule&) + 402
18 swift                    0x000000010427cb16 performCompile(swift::CompilerInstance&, swift::CompilerInvocation&, llvm::ArrayRef<char const*>, int&) + 11830
19 swift                    0x0000000104279ad3 frontend_main(llvm::ArrayRef<char const*>, char const*, void*) + 2691
20 swift                    0x0000000104276154 main + 2324
21 libdyld.dylib            0x00007fff9de295ad start + 1
22 libdyld.dylib            0x000000000000004a start + 1646095006

1. While running SILModuleTransform "Early Performance Inliner".

change to this code can solve the problem

public extension ASStaticLayoutable {

    var staticSpec: ASLayoutSpec {
        return ASStaticLayoutSpec(children: [self])
    }
}

But, why extension ASLayoutable will cause Segmentation fault: 11
ASLayoutable also comform to ASStaticLayoutable
@protocol ASLayoutable <ASStackLayoutable, ASStaticLayoutable, ASLayoutablePrivate>

@appleguy

This comment has been minimized.

Show comment
Hide comment
@appleguy

appleguy Jan 26, 2016

Contributor

@CodeEagle that commit is huge - what was the change exactly? The code is written in Objective-C...

Contributor

appleguy commented Jan 26, 2016

@CodeEagle that commit is huge - what was the change exactly? The code is written in Objective-C...

@appleguy appleguy added this to the 1.9.7 milestone Jan 26, 2016

@CodeEagle

This comment has been minimized.

Show comment
Hide comment
@CodeEagle

CodeEagle Jan 26, 2016

@appleguy just ignore that commit ,the commit is a shortcut project , not for ASDK.....

and Segmentation fault: 11 will appear when

image

CodeEagle commented Jan 26, 2016

@appleguy just ignore that commit ,the commit is a shortcut project , not for ASDK.....

and Segmentation fault: 11 will appear when

image

@appleguy appleguy added the Swift label Feb 7, 2016

@appleguy appleguy changed the title from [Bug?] Command failed due to signal: Segmentation fault: 11 to [Swift] Whole-module optimization may crash the Swift compiler with some apps? Feb 7, 2016

@appleguy

This comment has been minimized.

Show comment
Hide comment
@appleguy

appleguy Feb 7, 2016

Contributor

@Adlai-Holler - have you tried the whole module optimization in your app? Give it a whirl and let me know if it crashes the compiler… I'm really not sure if this is a framework problem, or some kind of interaction with Extensions or other code that @CodeEagle has in their application.

Contributor

appleguy commented Feb 7, 2016

@Adlai-Holler - have you tried the whole module optimization in your app? Give it a whirl and let me know if it crashes the compiler… I'm really not sure if this is a framework problem, or some kind of interaction with Extensions or other code that @CodeEagle has in their application.

@Adlai-Holler

This comment has been minimized.

Show comment
Hide comment
@Adlai-Holler

Adlai-Holler Feb 8, 2016

Contributor

@appleguy Unfortunately this issue does reproduce even in a trivial project. Here's what I've observed:

This is the minimum I can do to get the compiler to crash:

// Compiler Crashes.
let bar = ASLayoutSpec().foo()

extension ASLayoutable {

    func foo() -> ASLayoutSpec {
        return ASStaticLayoutSpec(children: [self])
    }

}

Either one of these variations, and the compiler works great:

// Compiler Works.
// let bar = ASLayoutSpec().foo()

extension ASLayoutable {

    func foo() -> ASLayoutSpec {
        return ASStaticLayoutSpec(children: [self])
    }

}
// Compiler Works.
let bar = ASLayoutSpec().foo()

extension ASLayoutable {

    func foo() -> ASLayoutSpec {
        return ASStaticLayoutSpec(children: [])
    }

}

We definitely have a lot of protocol composition going on with ASLayoutable and its implementors but it all makes good sense. Considering the fact that removing self fixes the compiler crash, I think this is something out of my league in the Swift optimizer.

The issue is not resolved in Xcode 7.3 Beta 2, so I've submitted a bug to Apple https://openradar.appspot.com/radar?id=6191944728313856 as well as a reduced test case at https://github.com/Adlai-Holler/ASDKWMOTest

Unless someone can find something incorrect or janky in our Layoutable protocol hierarchy, we may have no path forward.

Contributor

Adlai-Holler commented Feb 8, 2016

@appleguy Unfortunately this issue does reproduce even in a trivial project. Here's what I've observed:

This is the minimum I can do to get the compiler to crash:

// Compiler Crashes.
let bar = ASLayoutSpec().foo()

extension ASLayoutable {

    func foo() -> ASLayoutSpec {
        return ASStaticLayoutSpec(children: [self])
    }

}

Either one of these variations, and the compiler works great:

// Compiler Works.
// let bar = ASLayoutSpec().foo()

extension ASLayoutable {

    func foo() -> ASLayoutSpec {
        return ASStaticLayoutSpec(children: [self])
    }

}
// Compiler Works.
let bar = ASLayoutSpec().foo()

extension ASLayoutable {

    func foo() -> ASLayoutSpec {
        return ASStaticLayoutSpec(children: [])
    }

}

We definitely have a lot of protocol composition going on with ASLayoutable and its implementors but it all makes good sense. Considering the fact that removing self fixes the compiler crash, I think this is something out of my league in the Swift optimizer.

The issue is not resolved in Xcode 7.3 Beta 2, so I've submitted a bug to Apple https://openradar.appspot.com/radar?id=6191944728313856 as well as a reduced test case at https://github.com/Adlai-Holler/ASDKWMOTest

Unless someone can find something incorrect or janky in our Layoutable protocol hierarchy, we may have no path forward.

@aaronschubert0

This comment has been minimized.

Show comment
Hide comment
@aaronschubert0

aaronschubert0 Feb 9, 2016

Contributor

@appleguy @Adlai-Holler @CodeEagle
I would say this is more a programming error to be honest (granted there should be a better error message)

That is because you are basically trying to do this:
let foo = ASStaticLayoutSpec(children: [foo])
Which produces the error = "Variable used within its own initial value". The compiler is just getting confused at this point because the code passes the inline syntax validation (since it isn't fully aware of what your code is trying to accomplish)

So I would just use @Adlai-Holler second solution where you initialise the object with an empty array.
Maybe @CodeEagle you can share where you envisioned on using this, and we can code it in a different way?

Contributor

aaronschubert0 commented Feb 9, 2016

@appleguy @Adlai-Holler @CodeEagle
I would say this is more a programming error to be honest (granted there should be a better error message)

That is because you are basically trying to do this:
let foo = ASStaticLayoutSpec(children: [foo])
Which produces the error = "Variable used within its own initial value". The compiler is just getting confused at this point because the code passes the inline syntax validation (since it isn't fully aware of what your code is trying to accomplish)

So I would just use @Adlai-Holler second solution where you initialise the object with an empty array.
Maybe @CodeEagle you can share where you envisioned on using this, and we can code it in a different way?

@Adlai-Holler

This comment has been minimized.

Show comment
Hide comment
@Adlai-Holler

Adlai-Holler Feb 9, 2016

Contributor

Hmm @aaronschubert0 are you sure that's the case? I believe the code is equivalent to:

let bar = ASStaticLayoutSpec(children: [ASLayoutSpec()])
Contributor

Adlai-Holler commented Feb 9, 2016

Hmm @aaronschubert0 are you sure that's the case? I believe the code is equivalent to:

let bar = ASStaticLayoutSpec(children: [ASLayoutSpec()])
@aaronschubert0

This comment has been minimized.

Show comment
Hide comment
@aaronschubert0

aaronschubert0 Feb 9, 2016

Contributor

@Adlai-Holler From this guide (https://developer.apple.com/library/ios/documentation/Swift/Conceptual/Swift_Programming_Language/Extensions.html) I would assume self to reference the initialised object rather than a new object of that type.

The extension on Int probably highlights this best (from the guide)

extension Int {
    func repetitions(task: () -> Void) {
        for _ in 0..<self {
            task()
        }
    }
}
///Implementation
3.repetitions({
    print("Hello!")
})

Having said that, I've just had a small play around with your project and what you describe is what's happening:

let foo = ASStaticLayoutSpec() //<ASStaticLayoutSpec: 0x136d39180>, this is passed into the extension.
let bar = foo.foo() //bar equals a new object <ASStaticLayoutSpec: 0x136e3dff0>

I overlooked the creation of the new ASLayoutSpec() which you then call foo on. My mistake.

Contributor

aaronschubert0 commented Feb 9, 2016

@Adlai-Holler From this guide (https://developer.apple.com/library/ios/documentation/Swift/Conceptual/Swift_Programming_Language/Extensions.html) I would assume self to reference the initialised object rather than a new object of that type.

The extension on Int probably highlights this best (from the guide)

extension Int {
    func repetitions(task: () -> Void) {
        for _ in 0..<self {
            task()
        }
    }
}
///Implementation
3.repetitions({
    print("Hello!")
})

Having said that, I've just had a small play around with your project and what you describe is what's happening:

let foo = ASStaticLayoutSpec() //<ASStaticLayoutSpec: 0x136d39180>, this is passed into the extension.
let bar = foo.foo() //bar equals a new object <ASStaticLayoutSpec: 0x136e3dff0>

I overlooked the creation of the new ASLayoutSpec() which you then call foo on. My mistake.

@hannahmbanana

This comment has been minimized.

Show comment
Hide comment
@hannahmbanana

hannahmbanana May 11, 2016

Contributor

@Adlai-Holler: Not sure if you are still incredibly busy. If not, could you test this against your complex project?

Contributor

hannahmbanana commented May 11, 2016

@Adlai-Holler: Not sure if you are still incredibly busy. If not, could you test this against your complex project?

@Adlai-Holler

This comment has been minimized.

Show comment
Hide comment
@Adlai-Holler

Adlai-Holler May 11, 2016

Contributor

Hey! Glad to take a look-see, will do it tomorrow morning and report back.

On May 10, 2016, at 8:46 PM, Hannah Troisi notifications@github.com wrote:

@Adlai-Holler https://github.com/Adlai-Holler: Not sure if you are still incredibly busy. If not, could you test this against your complex project?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub #1109 (comment)

Contributor

Adlai-Holler commented May 11, 2016

Hey! Glad to take a look-see, will do it tomorrow morning and report back.

On May 10, 2016, at 8:46 PM, Hannah Troisi notifications@github.com wrote:

@Adlai-Holler https://github.com/Adlai-Holler: Not sure if you are still incredibly busy. If not, could you test this against your complex project?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub #1109 (comment)

@Adlai-Holler

This comment has been minimized.

Show comment
Hide comment
@Adlai-Holler

Adlai-Holler May 11, 2016

Contributor

@hannahmbanana I believe this issue's resolved as far as we can take it – the Swift compiler seems to crash when optimizing the code OP posted, separate from ASDK itself, so we'll have to wait until Apple fixes the radar linked above. @CodeEagle We good?

Contributor

Adlai-Holler commented May 11, 2016

@hannahmbanana I believe this issue's resolved as far as we can take it – the Swift compiler seems to crash when optimizing the code OP posted, separate from ASDK itself, so we'll have to wait until Apple fixes the radar linked above. @CodeEagle We good?

@hannahmbanana

This comment has been minimized.

Show comment
Hide comment
@hannahmbanana

hannahmbanana May 11, 2016

Contributor

@Adlai-Holler: Hmm. I could have sworn that the radar was marked Closed when I checked this yesterday. :) Thanks for checking anyways!

Contributor

hannahmbanana commented May 11, 2016

@Adlai-Holler: Hmm. I could have sworn that the radar was marked Closed when I checked this yesterday. :) Thanks for checking anyways!

@Adlai-Holler

This comment has been minimized.

Show comment
Hide comment
@Adlai-Holler

Adlai-Holler May 14, 2016

Contributor

Heads up @hannahmbanana OP hit me with the thumbs up so we can close this.

Contributor

Adlai-Holler commented May 14, 2016

Heads up @hannahmbanana OP hit me with the thumbs up so we can close this.

@appleguy appleguy closed this May 18, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment