Merge branch 'main' into es-sil-pkg

Author: elsh

CHANGELOG.md

@@ -61,52 +61,142 @@
 
 ## Swift 5.10
 
+* Swift 5.10 closes all known static data-race safey holes in complete strict
+concurrency checking.
+
+  When writing code against `-strict-concurrency=complete`, Swift 5.10 will
+  diagnose all potential for data races at compile time unless an explicit
+  unsafe opt out, such as `nonisolated(unsafe)` or `@unchecked Sendable`, is
+  used.
+
+  For example, in Swift 5.9, the following code crashes at runtime due to a
+  `@MainActor`-isolated initializer being evaluated outside the actor, but it
+  was not diagnosed under `-strict-concurrency=complete`:
+
+  ```swift
+  @MainActor
+  class MyModel {
+    init() {
+      MainActor.assertIsolated()
+    }
+
+    static let shared = MyModel()
+  }
+
+  func useShared() async {
+    let model = MyModel.shared
+  }
+
+  await useShared()
+  ```
+
+  The above code admits data races because a `@MainActor`-isolated static
+  variable, which evaluates a `@MainActor`-isolated initial value upon first
+  access, is accessed synchronously from a `nonisolated` context. In Swift
+  5.10, compiling the code with `-strict-concurrency=complete` produces a
+  warning that the access must be done asynchronously:
+
+  ```
+  warning: expression is 'async' but is not marked with 'await'
+    let model = MyModel.shared
+                ^~~~~~~~~~~~~~
+                await
+  ```
+
+  Swift 5.10 fixed numerous other bugs in `Sendable` and actor isolation
+  checking to strengthen the guarantees of complete concurrency checking.
+
+  Note that the complete concurrency model in Swift 5.10 is conservative.
+  Several Swift Evolution proposals are in active development to improve the
+  usability of strict concurrency checking ahead of Swift 6.
+
 * [SE-0412][]:
 
-  Under strict concurrency checking, every global or static variable must be either isolated to a global actor or be both immutable and of `Sendable` type.
+  Global and static variables are prone to data races because they provide memory that can be accessed from any program context.  Strict concurrency checking in Swift 5.10 prevents data races on global and static variables by requiring them to be either:
+
+    1. isolated to a global actor, or
+    2. immutable and of `Sendable` type.
+
+  For example:
 
   ```swift
   var mutableGlobal = 1
   // warning: var 'mutableGlobal' is not concurrency-safe because it is non-isolated global shared mutable state
   // (unless it is top-level code which implicitly isolates to @MainActor)
 
-  final class NonsendableType {
-    init() {}
+  @MainActor func mutateGlobalFromMain() {
+    mutableGlobal += 1
+  }
+
+  nonisolated func mutateGlobalFromNonisolated() async {
+    mutableGlobal += 10
   }
 
   struct S {
-    static let immutableNonsendable = NonsendableType()
-    // warning: static property 'immutableNonsendable' is not concurrency-safe because it is not either conforming to 'Sendable' or isolated to a global actor
+    static let immutableSendable = 10
+    // okay; 'immutableSendable' is safe to access concurrently because it's immutable and 'Int' is 'Sendable'
   }
   ```
 
-  The attribute `nonisolated(unsafe)` can be used to annotate a global variable (or any form of storage) to disable static checking of data isolation, but note that without correct implementation of a synchronization mechanism to achieve data isolation, dynamic run-time analysis from exclusivity enforcement or tools such as Thread Sanitizer could still identify failures.
+  A new `nonisolated(unsafe)` modifier can be used to annotate a global or static variable to suppress data isolation violations when manual synchronization is provided:
 
   ```swift
-  nonisolated(unsafe) var global: String
+  // This global is only set in one part of the program
+  nonisolated(unsafe) var global: String!
   ```
 
+  `nonisolated(unsafe)` can be used on any form of storage, including stored properties and local variables, as a more granular opt out for `Sendable` checking, eliminating the need for `@unchecked Sendable` wrapper types in many use cases:
+
+  ```swift
+  import Dispatch
+
+  // 'MutableData' is not 'Sendable'
+  class MutableData { ... } 
+
+  final class MyModel: Sendable {
+    private let queue = DispatchQueue(...)
+    // 'protectedState' is manually isolated by 'queue'
+    nonisolated(unsafe) private var protectedState: MutableData
+  }
+  ```
+
+  Note that without correct implementation of a synchronization mechanism to achieve data isolation, dynamic run-time analysis from exclusivity enforcement or tools such as the Thread Sanitizer could still identify failures.
+
 * [SE-0411][]:
 
-  Default value expressions can now have the same isolation as the enclosing
-  function or the corresponding stored property:
+  Swift 5.10 closes a data-race safety hole that previously permitted isolated
+  default stored property values to be synchronously evaluated from outside the
+  actor. For example, the following code compiles warning-free under
+  `-strict-concurrency=complete` in Swift 5.9, but it will crash at runtime at
+  the call to `MainActor.assertIsolated()`:
 
   ```swift
-  @MainActor
-  func requiresMainActor() -> Int { ... }
+  @MainActor func requiresMainActor() -> Int {
+    MainActor.assertIsolated()
+    return 0
+  }
 
-  class C {
-    @MainActor
-    var x: Int = requiresMainActor()
+  @MainActor struct S {
+    var x = requiresMainActor()
+    var y: Int
+  }
+
+  nonisolated func call() async {
+    let s = await S(y: 10)
   }
 
-  @MainActor func defaultArg(value: Int = requiresMainActor()) { ... }
+  await call()
   ```
 
-  For isolated default values of stored properties, the implicit initialization
-  only happens in the body of an `init` with the same isolation. This closes
-  an important data-race safety hole where global-actor-isolated default values
-  could inadvertently run synchronously from outside the actor.
+  This happens because `requiresMainActor()` is used as a default argument to
+  the member-wise initializer of `S`, but default arguments are always
+  evaluated in the caller. In this case, the caller runs on the generic
+  executor, so the default argument evaluation crashes.
+
+  Under `-strict-concurrency=complete` in Swift 5.10, default argument values
+  can safely share the same isolation as the enclosing function or stored
+  property. The above code is still valid, but the isolated default argument is
+  guaranteed to be evaluated in the callee's isolation domain.
 
 ## Swift 5.9.2
 
@@ -9998,4 +10088,4 @@ using the `.dynamicType` member to retrieve the type of an expression should mig
 [#57225]: <https://github.com/apple/swift/issues/57225>
 [#56139]: <https://github.com/apple/swift/issues/56139>
 [#70065]: <https://github.com/apple/swift/pull/70065>
-[swift-syntax]: https://github.com/apple/swift-syntax
+[swift-syntax]: https://github.com/apple/swift-syntax

CMakeLists.txt

@@ -784,11 +784,17 @@ if (CMAKE_Swift_COMPILER)
 else()
   message(STATUS "Swift Compiler (None).")
 endif()
+
+set(THREADS_PREFER_PTHREAD_FLAG YES)
+include(FindThreads)
+
 if(SWIFT_PATH_TO_CMARK_BUILD)
   execute_process(COMMAND ${SWIFT_PATH_TO_CMARK_BUILD}/src/cmark --version
     OUTPUT_VARIABLE _CMARK_VERSION
     OUTPUT_STRIP_TRAILING_WHITESPACE)
   message(STATUS "CMark Version: ${_CMARK_VERSION}")
+elseif(SWIFT_INCLUDE_TOOLS)
+  find_package(cmark-gfm CONFIG REQUIRED)
 endif()
 message(STATUS "")
 

SwiftCompilerSources/Sources/Optimizer/Utilities/AddressUtils.swift

@@ -0,0 +1,178 @@
+//===--- AddressUtils.swift - Utilities for handling SIL addresses -------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+import SIL
+
+/// Classify address uses. This can be used by def-use walkers to
+/// ensure complete handling of all legal SIL patterns.
+///
+/// TODO: Integrate this with SIL verification to ensure completeness.
+///
+/// TODO: Convert AddressDefUseWalker to conform to AddressUtils after
+/// checking that the additional instructions are handled correctly by
+/// escape analysis.
+///
+/// TODO: Verify that pointerEscape is only called for live ranges in which
+/// `findPointerEscape()` returns true.
+protocol AddressUseVisitor {
+  var context: Context { get }
+
+  /// An address projection produces a single address result and does not
+  /// escape its address operand in any other way.
+  mutating func projectedAddressUse(of operand: Operand, into value: Value)
+    -> WalkResult
+
+  /// An access scope: begin_access, begin_apply, load_borrow.
+  mutating func scopedAddressUse(of operand: Operand) -> WalkResult
+
+  /// A address leaf use cannot propagate the address bits beyond the
+  /// instruction.
+  ///
+  /// An apply or builtin propagates an address into the callee, but
+  /// it is considered a leaf use as long as the argument does not escape.
+  mutating func leafAddressUse(of operand: Operand) -> WalkResult
+
+  /// A loaded address use propagates the value at the address.
+  mutating func loadedAddressUse(of operand: Operand, into value: Value)
+    -> WalkResult
+
+  /// A loaded address use propagates the value at the address to the
+  /// destination address operand.
+  mutating func loadedAddressUse(of operand: Operand, into address: Operand)
+    -> WalkResult
+
+  /// A non-address owned `value` whose ownership depends on the in-memory
+  /// value at `address`, such as `mark_dependence %value on %address`.
+  mutating func dependentAddressUse(of operand: Operand, into value: Value)
+    -> WalkResult
+
+  /// A pointer escape may propagate the address beyond the current instruction.
+  mutating func escapingAddressUse(of operand: Operand) -> WalkResult
+
+  /// A unknown address use. This should never be called in valid SIL.
+  mutating func unknownAddressUse(of operand: Operand) -> WalkResult
+}
+
+extension AddressUseVisitor {
+  /// Classify an address-type operand, dispatching to one of the
+  /// protocol methods above.
+  mutating func classifyAddress(operand: Operand) -> WalkResult {
+    switch operand.instruction {
+    case is BeginAccessInst, is BeginApplyInst, is LoadBorrowInst,
+         is StoreBorrowInst:
+      return scopedAddressUse(of: operand)
+
+    case let markDep as MarkDependenceInst:
+      if markDep.valueOperand == operand {
+        return projectedAddressUse(of: operand, into: markDep)
+      }
+      assert(markDep.baseOperand == operand)
+      // If another address depends on the current address,
+      // handle it like a projection.
+      if markDep.type.isAddress {
+        return projectedAddressUse(of: operand, into: markDep)
+      }
+      if LifetimeDependence(markDependence: markDep, context) != nil {
+        // This is unreachable from InteriorUseVisitor because the
+        // base address of a `mark_dependence [nonescaping]` must be a
+        // `begin_access`, and interior liveness does not check uses of
+        // the accessed address.
+        return dependentAddressUse(of: operand, into: markDep)
+      }
+      // A potentially escaping value depends on this address.
+      return escapingAddressUse(of: operand)
+
+    case let pai as PartialApplyInst where pai.isOnStack:
+      return dependentAddressUse(of: operand, into: pai)
+
+    case let pai as PartialApplyInst where !pai.isOnStack:
+      return escapingAddressUse(of: operand)
+
+    case is AddressToPointerInst:
+      return escapingAddressUse(of: operand)
+
+    case is StructElementAddrInst, is TupleElementAddrInst,
+         is IndexAddrInst, is TailAddrInst, is TuplePackElementAddrInst, 
+         is InitEnumDataAddrInst, is UncheckedTakeEnumDataAddrInst,
+         is InitExistentialAddrInst, is OpenExistentialAddrInst,
+         is ProjectBlockStorageInst, is UncheckedAddrCastInst,
+         is UnconditionalCheckedCastAddrInst,
+         is MarkUninitializedInst, is DropDeinitInst,
+         is CopyableToMoveOnlyWrapperAddrInst,
+         is MoveOnlyWrapperToCopyableAddrInst,
+         is MarkUnresolvedNonCopyableValueInst:
+      let svi = operand.instruction as! SingleValueInstruction
+      return projectedAddressUse(of: operand, into: svi)
+
+    case is ReturnInst, is ThrowInst, is YieldInst, is TryApplyInst,
+         is SwitchEnumAddrInst, is CheckedCastAddrBranchInst,
+         is SelectEnumAddrInst, is InjectEnumAddrInst,
+         is StoreInst, is StoreUnownedInst, is StoreWeakInst,
+         is AssignInst, is AssignByWrapperInst, is AssignOrInitInst,
+         is TupleAddrConstructorInst, is InitBlockStorageHeaderInst,
+         is RetainValueAddrInst, is ReleaseValueAddrInst,
+         is DestroyAddrInst, is DeallocStackInst, 
+         is DeinitExistentialAddrInst,
+         is EndApplyInst, is IsUniqueInst, is MarkFunctionEscapeInst,
+         is PackElementSetInst:
+      return leafAddressUse(of: operand)
+
+    case is LoadInst, is LoadUnownedInst,  is LoadWeakInst, 
+         is ValueMetatypeInst, is ExistentialMetatypeInst,
+         is PackElementGetInst:
+      let svi = operand.instruction as! SingleValueInstruction
+      return loadedAddressUse(of: operand, into: svi)
+
+    case let sdai as SourceDestAddrInstruction
+           where sdai.sourceOperand == operand:
+      return loadedAddressUse(of: operand, into: sdai.destinationOperand)
+
+    case let sdai as SourceDestAddrInstruction
+           where sdai.destinationOperand == operand:
+      return leafAddressUse(of: operand)
+
+    case let builtin as BuiltinInst:
+      switch builtin.id {
+      case .Copy where builtin.operands[1] == operand: // source
+        return loadedAddressUse(of: operand, into: builtin.operands[0])
+
+      case .Copy where builtin.operands[0] == operand: // dest
+        return leafAddressUse(of: operand)
+
+      // Builtins that cannot load a nontrivial value.
+      case .TSanInoutAccess, .ResumeThrowingContinuationReturning,
+           .ResumeNonThrowingContinuationReturning, .GenericAdd,
+           .GenericFAdd, .GenericAnd, .GenericAShr, .GenericLShr, .GenericOr,
+           .GenericFDiv, .GenericMul, .GenericFMul, .GenericSDiv,
+           .GenericExactSDiv, .GenericShl, .GenericSRem, .GenericSub,
+           .GenericFSub, .GenericUDiv, .GenericExactUDiv, .GenericURem,
+           .GenericFRem, .GenericXor, .TaskRunInline, .ZeroInitializer,
+           .GetEnumTag, .InjectEnumTag:
+        return leafAddressUse(of: operand)
+      default:
+        // TODO: SIL verification should check that this exhaustively
+        // recognizes all builtin address uses.
+        return .abortWalk
+      }
+
+    case is BranchInst, is CondBranchInst:
+      fatalError("address phi is not allowed")
+
+    default:
+      if operand.instruction.isIncidentalUse {
+        return leafAddressUse(of: operand)
+      }
+      // Unkown instruction.
+      return unknownAddressUse(of: operand)
+    }
+  }
+}