swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.53k stars 10.35k forks source link

[SR-7179] Repeatedly calling a closure passed through identity function causes stack overflow #49727

Open swift-ci opened 6 years ago

swift-ci commented 6 years ago
Previous ID SR-7179
Radar None
Original Reporter AndrewC (JIRA User)
Type Bug
Environment macOS 10.13.3 (17D102) XCode 9.2 Swift 4.0
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 0 | |Component/s | Compiler | |Labels | Bug, Runtime | |Assignee | None | |Priority | Medium | md5: 0dc8da060ef08555dc3c6e2968d997d7

Issue Description:

Run this code as a console application and you'll find the application eventually overflows its stack. It takes about a minute for it to overflow on my computer. The code:

func identity<T>(_ v: T) -> T {
    return v
}

var closure = {
    return
}

while true {
    closure()
    closure = identity(closure)
}

Here's a snip of the stack when it crashes with EXC_BAD_ACCESS code=2:

    frame #&#8203;0: 0x0000000100002204 ClosureTest`thunk for @callee_owned (@in ()) -> (@out ()) at main.swift:0
    frame #&#8203;1: 0x000000010000229a ClosureTest`partial apply for thunk for @callee_owned (@in ()) -> (@out ()) at main.swift:0
...
    frame #&#8203;190581: 0x000000010000229a ClosureTest`partial apply for thunk for @callee_owned (@in ()) -> (@out ()) at main.swift:0
    frame #&#8203;190582: 0x00000001000020dc ClosureTest`thunk for @callee_owned () -> () at main.swift:0
    frame #&#8203;190583: 0x00000001000021aa ClosureTest`partial apply for thunk for @callee_owned () -> () at main.swift:0
    frame #&#8203;190584: 0x0000000100002217 ClosureTest`thunk for @callee_owned (@in ()) -> (@out ()) at main.swift:0
    frame #&#8203;190585: 0x000000010000229a ClosureTest`partial apply for thunk for @callee_owned (@in ()) -> (@out ()) at main.swift:0
  * frame #&#8203;190586: 0x0000000100001ed1 ClosureTest`main at main.swift:38
    frame #&#8203;190587: 0x00007fff75fca115 libdyld.dylib`start + 1
belkadan commented 6 years ago

@jckarter, @slavapestov, do we already have a bug tracking this? The "closure thunks stack" issue?

belkadan commented 6 years ago

Reproduces on master, by the way.

jckarter commented 6 years ago

I don't think we have a bug specifically tracking avoiding repeat reabstractions. cc @rjmccall

rjmccall commented 6 years ago

This one works for me. I think we might have something tracking the perf side of it, but not the correctness side.

slavapestov commented 6 years ago

I think the optimizer can eliminate redundant re-abstractions at compile time sometimes but we definitely can't do anything at runtime or in unoptimized builds.

rjmccall commented 6 years ago

We could definitely do something at runtime:

  1. Add a ``reabstract_function`` SIL instruction to apply the reabstraction instead of just ``partial_apply``. This is probably necessary for the static optimization anyway.
  2. Come up with some way of describing a function abstraction pattern with a string.
  3. Reserve a heap metadata kind for reabstraction thunks. This kind dictates the layout of the thunk, which is just the original function and a pointer to the pattern string for the original function (and possibly some captured type data).
  4. Instead of unconditionally allocating a reabstraction thunk in ``reabstract_function``, ask the runtime to do it, handing it the original and target pattern strings as well as the original function value.
  5. The runtime function (recursively) looks for a reabstraction-thunk context whose original pattern matches the target pattern. If it finds one, it copies the function value from that context; otherwise, it allocates a reabstraction thunk.