Uploaded image for project: 'Swift'
  1. Swift
  2. SR-15225

return value of async closures are clobbered when running release builds

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Medium
    • Resolution: Done
    • Component/s: Compiler
    • Labels:
    • Environment:

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

      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.

       

        Attachments

          Activity

            People

            Assignee:
            Unassigned Unassigned
            Reporter:
            taylorswift taylor swift
            Votes:
            5 Vote for this issue
            Watchers:
            12 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved: