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-15225] return value of async closures are clobbered when running release builds #57547

Closed
tayloraswift opened this issue Sep 22, 2021 · 4 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself concurrency Feature: umbrella label for concurrency language features

Comments

@tayloraswift
Copy link
Contributor

Previous ID SR-15225
Radar rdar://problem/83411917
Original Reporter @Kelvin13
Type Bug
Status Resolved
Resolution Done
Environment

$ swiftc --version Swift version 5.6-dev (LLVM a29f52d415422f3, Swift db90ea2) Target: x86_64-unknown-linux-gnu

Additional Detail from JIRA
Votes 5
Component/s Compiler
Labels Bug, Concurrency
Assignee None
Priority Medium

md5: 7491f78485633b45441c25d284129548

Issue Description:

Calling an `async` closure parameter causes parts of its return value to be overwritten with garbage values. This only occurs on release builds.

A minimal example, which reproduces the stack corruption on almost 100% of the loop iterations, is given below:

// async-return-value-corruption.swift

@main 
enum Main 
{
    actor A
    {
        init() 
        {
        }
        
        func a(_ f:@Sendable () async -> (Int, (Int, Int, Int, Int))?) 
            async -> Void
        {
            guard let (head, tail):(Int, (Int, Int, Int, Int)) = await f()
            else 
            {
                return 
            }
            
            print((head, tail))
            return 
        }
    }
    
    static 
    func p() async -> Bool
    {
        true 
    }
    
    static 
    func main() async
    {
        while true 
        {
            let a:A     = .init()
            
            async let task:Void = a.a
            {
                if await Self.p()
                {
                    return (0, (0, 0, 0 ,0)) 
                }
                else 
                {
                    return nil 
                }
            }
            await task
        }
    }
}



$ swiftc --version
Swift version 5.6-dev (LLVM a29f52d415422f3, Swift db90ea20e70c92a)
Target: x86_64-unknown-linux-gnu

$ swiftc -parse-as-library -O async-return-value-corruption.swift
$ ./async-return-value-corruption 
(139787763716080, (0, 0, 0, 0)) 
(139787629498352, (0, 0, 0, 0)) 
(139787965042672, (0, 0, 0, 0)) 
(139787763716080, (0, 0, 0, 0)) 
(139787629498352, (0, 0, 0, 0))
...

The memory offset of the word being clobbered, the frequency at which it occurs, and the “appearance” (e.g., stack address, small integer, `1`, etc.) varies depending on the size and shape of the return value, whether it is generic or not, and if so, whether it is passed as an existential.

The method `A.a(_:` does not need to be isolated to the actor, but the stack corruption occurs more frequently when it is.

The appearance of the garbage value tends to either be:

(a) a small integer (1, 2, ... 7)

(b) a large integer, possibly a size or count (2000-20,000)

(c) an integer with most of its bits set, probably a pointer

The exact garbage values are usually deterministic across multiple runs, but there can be more than one within a single run, which show up in pretty stable proportions to one another.

This bug was discovered in a financial application. It had gone unnoticed because it (1) only occurs in release builds, (2) it is only observable from the caller’s side, and (3) in similar call sites, the word being clobbered was either padding, or “unimportant” diagnostic data. Unfortunately, after a minor code change, the exponent field of a `Decimal` struct fell under the memory offset being corrupted. Because it was getting exclusively garbage values of class (a), this did not trip any sanity checks in the application, which caused it to attempt to buy over 10,000x more cryptocurrency than intended. Had the exchange server not rejected the orders for overrunning margin limits, this bug would have likely resulted in thousands of dollars of losses.

@typesanitizer
Copy link

@swift-ci create

@tayloraswift
Copy link
Contributor Author

update: can confirm that this issue is present in the 5.5-RELEASE toolchain

@DougGregor
Copy link
Member

We found the underlying issue in an incorrect coroutine optimization, disabled here: apple/llvm-project#3314

@tayloraswift
Copy link
Contributor Author

appears to be fixed, after testing with a locally-compiled swift toolchain

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself concurrency Feature: umbrella label for concurrency language features
Projects
None yet
Development

No branches or pull requests

3 participants