Merge pull request #64885 from tshortli/ban-unavailable-stored-properties

tshortli · web-flow · commit 24a33174202f · 2023-04-04T22:27:50.000-07:00
Sema: Ban unavailable stored properties
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,28 @@ _**Note:** This is in reverse chronological order, so newer entries are added to
 
 ## Swift 5.9
 
+* Marking stored properties as unavailable with `@available` has been banned,
+  closing an unintentional soundness hole that had allowed arbitrary
+  unavailable code to run and unavailable type metadata to be used at runtime:
+  
+  ```swift
+  @available(*, unavailable)
+  struct Unavailable {
+    init() {
+      print("Unavailable.init()")
+    }
+  }
+
+  struct S {
+    @available(*, unavailable)
+    var x = Unavailable()
+  }
+
+  _ = S() // prints "Unavailable.init()"
+  ```
+  
+  Marking `deinit` as unavailable has also been banned for similar reasons.
+
 * [SE-0366][]:
 
   The lifetime of a local variable value can be explicitly ended using the
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -6133,10 +6133,18 @@ ERROR(availability_global_script_no_potential,
       none, "global variable cannot be marked potentially "
       "unavailable with '@available' in script mode", ())
 
+ERROR(availability_global_script_no_unavailable,
+      none, "global variable cannot be marked unavailable "
+      "with '@available' in script mode", ())
+
 ERROR(availability_stored_property_no_potential,
       none, "stored properties cannot be marked potentially unavailable with "
       "'@available'", ())
 
+ERROR(availability_stored_property_no_unavailable,
+      none, "stored properties cannot be marked unavailable with '@available'",
+      ())
+
 ERROR(availability_enum_element_no_potential,
       none, "enum cases with associated values cannot be marked potentially unavailable with "
       "'@available'", ())
diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp
@@ -4521,6 +4521,24 @@ TypeChecker::diagnosticIfDeclCannotBeUnavailable(const Decl *D) {
     return diag::availability_deinit_no_unavailable;
   }
 
+  if (auto *VD = dyn_cast<VarDecl>(D)) {
+    if (!VD->hasStorageOrWrapsStorage())
+      return None;
+
+    if (parentIsUnavailable(D))
+      return None;
+
+    // Do not permit unavailable script-mode global variables; their initializer
+    // expression is not lazily evaluated, so this would not be safe.
+    if (VD->isTopLevelGlobal())
+      return diag::availability_global_script_no_unavailable;
+
+    // Globals and statics are lazily initialized, so they are safe for
+    // unavailability.
+    if (!VD->isStatic() && !D->getDeclContext()->isModuleScopeContext())
+      return diag::availability_stored_property_no_unavailable;
+  }
+
   return None;
 }
 
diff --git a/test/ClangImporter/Inputs/frameworks/SPIContainer.framework/Headers/SPIContainer.h b/test/ClangImporter/Inputs/frameworks/SPIContainer.framework/Headers/SPIContainer.h
@@ -12,10 +12,12 @@
 
 SPI_AVAILABLE(macos(10.7))
 @interface SPIInterface1
+- (instancetype)init;
 @end
 
 __SPI_AVAILABLE(macos(10.7))
 @interface SPIInterface2
+- (instancetype)init;
 @end
 
 @interface SharedInterface
diff --git a/test/ClangImporter/availability_spi_as_unavailable.swift b/test/ClangImporter/availability_spi_as_unavailable.swift
@@ -1,27 +1,27 @@
 // REQUIRES: OS=macosx
-// RUN: %target-swift-frontend -typecheck %s -F %S/Inputs/frameworks -verify -DNOT_UNDERLYING -library-level api
-// RUN: %target-swift-frontend -typecheck %s -F %S/Inputs/frameworks -module-name SPIContainer -import-underlying-module -verify -library-level api
+// RUN: %target-swift-frontend -typecheck %s -F %S/Inputs/frameworks -verify -DNOT_UNDERLYING -library-level api -parse-as-library -require-explicit-availability=ignore
+// RUN: %target-swift-frontend -typecheck %s -F %S/Inputs/frameworks -module-name SPIContainer -import-underlying-module -verify -library-level api -parse-as-library  -require-explicit-availability=ignore
 
 #if NOT_UNDERLYING
 import SPIContainer
 #endif
 
-@_spi(a) public let a: SPIInterface1
-@_spi(a) public let b: SPIInterface2
+@_spi(a) public let a: SPIInterface1 = .init()
+@_spi(a) public let b: SPIInterface2 = .init()
 
-public let c: SPIInterface1 // expected-error{{cannot use class 'SPIInterface1' here; it is an SPI imported from 'SPIContainer'}}
-public let d: SPIInterface2 // expected-error{{cannot use class 'SPIInterface2' here; it is an SPI imported from 'SPIContainer'}}
+public let c: SPIInterface1 = .init() // expected-error{{cannot use class 'SPIInterface1' here; it is an SPI imported from 'SPIContainer'}}
+public let d: SPIInterface2 = .init() // expected-error{{cannot use class 'SPIInterface2' here; it is an SPI imported from 'SPIContainer'}}
 
 @inlinable
-public func inlinableUsingSPI() { // expected-warning{{public declarations should have an availability attribute with an introduction version}}
+public func inlinableUsingSPI() {
   SharedInterface.foo() // expected-error{{class method 'foo()' cannot be used in an '@inlinable' function because it is an SPI imported from 'SPIContainer'}}
 }
 
 @available(macOS, unavailable)
-public let e: SPIInterface2
+public let e: SPIInterface2 = .init()
 
 @available(iOS, unavailable)
-public let f: SPIInterface2 // expected-error{{cannot use class 'SPIInterface2' here; it is an SPI imported from 'SPIContainer'}}
+public let f: SPIInterface2 = .init() // expected-error{{cannot use class 'SPIInterface2' here; it is an SPI imported from 'SPIContainer'}}
 
 @inlinable
 @available(macOS, unavailable)
diff --git a/test/ClangImporter/availability_spi_library_level_spi.swift b/test/ClangImporter/availability_spi_library_level_spi.swift
@@ -1,32 +1,32 @@
 // REQUIRES: OS=macosx
 
-// RUN: %target-swift-frontend -typecheck %s -F %S/Inputs/frameworks -verify -DNOT_UNDERLYING
-// RUN: %target-swift-frontend -typecheck %s -F %S/Inputs/frameworks -verify -DNOT_UNDERLYING -library-level spi
+// RUN: %target-swift-frontend -typecheck %s -F %S/Inputs/frameworks -verify -DNOT_UNDERLYING -parse-as-library -require-explicit-availability=ignore
+// RUN: %target-swift-frontend -typecheck %s -F %S/Inputs/frameworks -verify -DNOT_UNDERLYING -library-level spi -parse-as-library -require-explicit-availability=ignore
 
-// RUN: %target-swift-frontend -typecheck %s -F %S/Inputs/frameworks -module-name SPIContainer -import-underlying-module -verify
-// RUN: %target-swift-frontend -typecheck %s -F %S/Inputs/frameworks -module-name SPIContainer -import-underlying-module -verify -library-level spi
+// RUN: %target-swift-frontend -typecheck %s -F %S/Inputs/frameworks -module-name SPIContainer -import-underlying-module -verify -parse-as-library -require-explicit-availability=ignore
+// RUN: %target-swift-frontend -typecheck %s -F %S/Inputs/frameworks -module-name SPIContainer -import-underlying-module -verify -library-level spi -parse-as-library -require-explicit-availability=ignore
 
 
 #if NOT_UNDERLYING
 import SPIContainer
 #endif
 
-@_spi(a) public let a: SPIInterface1
-@_spi(a) public let b: SPIInterface2
+@_spi(a) public let a: SPIInterface1 = .init()
+@_spi(a) public let b: SPIInterface2 = .init()
 
-public let c: SPIInterface1
-public let d: SPIInterface2
+public let c: SPIInterface1 = .init()
+public let d: SPIInterface2 = .init()
 
 @inlinable
 public func inlinableUsingSPI() {
   SharedInterface.foo()
 }
 
 @available(macOS, unavailable)
-public let e: SPIInterface2
+public let e: SPIInterface2 = .init()
 
 @available(iOS, unavailable)
-public let f: SPIInterface2
+public let f: SPIInterface2 = .init()
 
 @inlinable
 @available(macOS, unavailable)
diff --git a/test/IRGen/float16_macos.swift b/test/IRGen/float16_macos.swift
@@ -5,10 +5,12 @@
 // REQUIRES: CPU=x86_64
 // UNSUPPORTED: use_os_stdlib
 
-@available(macOS 11, *)
-public struct Float16Wrapper {
-  @available(macOS, unavailable)
-  var x: Float16
+@inline(never)
+func blackHole<T>(_ t: T.Type) {}
+
+@available(macOS, unavailable)
+public func useFloat16() {
+  blackHole(Float16.self)
 }
 
-// CHECK-LABEL: @"$ss7Float16VMn" = extern_weak global %swift.type_descriptor
+// CHECK-LABEL: @"$ss7Float16VN" = extern_weak global %swift.type
diff --git a/test/IRGen/unavailable_decl_optimization_class.swift b/test/IRGen/unavailable_decl_optimization_class.swift
@@ -10,7 +10,11 @@ public class AvailableClass<T> {
   // CHECK-STRIP-NOT: s4Test14AvailableClassC19unavailablePropertyxvs
   // CHECK-STRIP-NOT: s4Test14AvailableClassC19unavailablePropertyxvM
   @available(*, unavailable)
-  public var unavailableProperty: T
+  public var unavailableProperty: T {
+    get { fatalError() }
+    set { fatalError() }
+    _modify { fatalError() }
+  }
 
   // CHECK-NO-STRIP: s4Test14AvailableClassCyACyxGxcfC
   // CHECK-NO-STRIP: s4Test14AvailableClassCyACyxGxcfc
diff --git a/test/IRGen/unavailable_decl_optimization_struct.swift b/test/IRGen/unavailable_decl_optimization_struct.swift
@@ -15,7 +15,11 @@ public struct AvailableStruct<T> {
   // CHECK-STRIP-NOT: s4Test15AvailableStructV19unavailablePropertyxvs
   // CHECK-STRIP-NOT: s4Test15AvailableStructV19unavailablePropertyxvM
   @available(*, unavailable)
-  public var unavailableProperty: T
+  public var unavailableProperty: T {
+    get { fatalError() }
+    set { fatalError() }
+    _modify { fatalError() }
+  }
 
   // CHECK-NO-STRIP: s4Test15AvailableStructVyACyxGxcfC
   // CHECK-STRIP-NOT: s4Test15AvailableStructVyACyxGxcfC
diff --git a/test/Migrator/double_fixit_ok.swift b/test/Migrator/double_fixit_ok.swift
@@ -1,7 +1,7 @@
 // RUN: %empty-directory(%t)
-// RUN: not %target-swift-frontend -typecheck -update-code -primary-file %s -emit-migrated-file-path %t/double_fixit_ok.result -swift-version 4
+// RUN: not %target-swift-frontend -typecheck -update-code -primary-file %s -emit-migrated-file-path %t/double_fixit_ok.result -swift-version 4 -parse-as-library
 // RUN: %diff -u %s.expected %t/double_fixit_ok.result
-// RUN: %target-swift-frontend -typecheck %s.expected -swift-version 5
+// RUN: %target-swift-frontend -typecheck %s.expected -swift-version 5 -parse-as-library
 
 @available(swift, obsoleted: 4, renamed: "Thing.constant___renamed")
 let ThingConstantGotRenamed = 1
diff --git a/test/Migrator/double_fixit_ok.swift.expected b/test/Migrator/double_fixit_ok.swift.expected
@@ -1,7 +1,7 @@
 // RUN: %empty-directory(%t)
-// RUN: not %target-swift-frontend -typecheck -update-code -primary-file %s -emit-migrated-file-path %t/double_fixit_ok.result -swift-version 4
+// RUN: not %target-swift-frontend -typecheck -update-code -primary-file %s -emit-migrated-file-path %t/double_fixit_ok.result -swift-version 4 -parse-as-library
 // RUN: %diff -u %s.expected %t/double_fixit_ok.result
-// RUN: %target-swift-frontend -typecheck %s.expected -swift-version 5
+// RUN: %target-swift-frontend -typecheck %s.expected -swift-version 5 -parse-as-library
 
 @available(swift, obsoleted: 4, renamed: "Thing.constant___renamed")
 let ThingConstantGotRenamed = 1
diff --git a/test/SILGen/unavailable_decl_optimization.swift b/test/SILGen/unavailable_decl_optimization.swift
@@ -88,7 +88,11 @@ public struct S<T> {
   // CHECK-STRIP-NOT: s4Test1SV19unavailablePropertyxvs
   // CHECK-STRIP-NOT: s4Test1SV19unavailablePropertyxvM
   @available(*, unavailable)
-  public var unavailableProperty: T
+  public var unavailableProperty: T {
+    get { fatalError() }
+    set { fatalError() }
+    _modify { fatalError() }
+  }
 
   // CHECK-NO-STRIP: s4Test1SVyACyxGxcfC
   // CHECK-STRIP-NOT: s4Test1SVyACyxGxcfC
@@ -128,7 +132,11 @@ public class C<T> {
   // CHECK-STRIP-NOT: s4Test1CC19unavailablePropertyxvs
   // CHECK-STRIP-NOT: s4Test1CC19unavailablePropertyxvM
   @available(*, unavailable)
-  public var unavailableProperty: T
+  public var unavailableProperty: T {
+    get { fatalError() }
+    set { fatalError() }
+    _modify { fatalError() }
+  }
 
   // CHECK-NO-STRIP: s4Test1CCyACyxGxcfC
   // CHECK-NO-STRIP: s4Test1CCyACyxGxcfc
diff --git a/test/Sema/availability.swift b/test/Sema/availability.swift
@@ -1,4 +1,4 @@
-// RUN: %target-typecheck-verify-swift -module-name MyModule
+// RUN: %target-typecheck-verify-swift -parse-as-library -module-name MyModule
 
 // REQUIRES: OS=macosx
 
@@ -175,7 +175,10 @@ func testPlatforms() {
 
 struct VarToFunc {
   @available(*, unavailable, renamed: "function()")
-  var variable: Int // expected-note 2 {{explicitly marked unavailable here}}
+  var variable: Int { // expected-note 2 {{explicitly marked unavailable here}}
+    get { 0 }
+    set {}
+  }
 
   @available(*, unavailable, renamed: "function()")
   func oldFunction() -> Int { return 42 } // expected-note 2 {{explicitly marked unavailable here}}
diff --git a/test/Sema/availability_script_mode.swift b/test/Sema/availability_script_mode.swift
@@ -0,0 +1,34 @@
+// RUN: %target-typecheck-verify-swift -target %target-cpu-apple-macosx10.50
+
+// REQUIRES: OS=macosx
+
+struct AlwaysAvailable {}
+
+@available(macOS, introduced: 10.50)
+struct Available10_50 {}
+
+@available(macOS, introduced: 10.51)
+struct Available10_51 {}
+
+@available(macOS, unavailable)
+struct UnavailableOnMacOS {}
+
+@available(*, unavailable)
+struct UnavailableUnconditionally {}
+
+var alwaysAvailableVar: AlwaysAvailable = .init() // Ok
+
+@available(macOS, introduced: 10.50)
+var availableOn10_50Var: Available10_50 = .init() // Ok
+
+// Script-mode globals have eagerly executed initializers so it isn't safe for
+// them to be unavailable.
+
+@available(macOS, introduced: 10.51) // expected-error {{global variable cannot be marked potentially unavailable with '@available' in script mode}}
+var potentiallyUnavailableVar: Available10_51 = .init()
+
+@available(macOS, unavailable) // expected-error {{global variable cannot be marked unavailable with '@available' in script mode}}
+var unavailableOnMacOSVar: UnavailableOnMacOS = .init()
+
+@available(*, unavailable) // expected-error {{global variable cannot be marked unavailable with '@available' in script mode}}
+var unconditionallyUnavailableVar: UnavailableUnconditionally = .init()
diff --git a/test/Sema/availability_stored_unavailable.swift b/test/Sema/availability_stored_unavailable.swift
@@ -0,0 +1,78 @@
+// RUN: %target-typecheck-verify-swift -parse-as-library
+
+// REQUIRES: OS=macosx
+
+@available(macOS, unavailable)
+struct UnavailableMacOSStruct {}
+
+@available(*, unavailable)
+public struct UniversallyUnavailableStruct {}
+
+// Ok, initialization of globals is lazy and boxed.
+@available(macOS, unavailable)
+var unavailableMacOSGlobal: UnavailableMacOSStruct = .init()
+
+@available(*, unavailable)
+var universallyUnavailableGlobal: UniversallyUnavailableStruct = .init()
+
+struct GoodAvailableStruct {
+  // Ok, computed property.
+  @available(macOS, unavailable)
+  var unavailableMacOS: UnavailableMacOSStruct {
+    get { UnavailableMacOSStruct() }
+  }
+
+  // Ok, initialization of static vars is lazy and boxed.
+  @available(macOS, unavailable)
+  static var staticUnavailableMacOS: UnavailableMacOSStruct = .init()
+}
+
+@available(macOS, unavailable)
+struct GoodUnavailableMacOSStruct {
+  var unavailableMacOS: UnavailableMacOSStruct = .init()
+  lazy var lazyUnavailableMacOS: UnavailableMacOSStruct = .init()
+
+  // Ok, the container is unavailable.
+  @available(macOS, unavailable)
+  var unavailableMacOSExplicit: UnavailableMacOSStruct = .init()
+}
+
+@available(macOS, unavailable)
+struct GoodNestedUnavailableMacOSStruct {
+  struct Inner {
+    var unavailableMacOS: UnavailableMacOSStruct = .init()
+    lazy var lazyUnavailableMacOS: UnavailableMacOSStruct = .init()
+
+    // Ok, the container is unavailable.
+    @available(macOS, unavailable)
+    var unavailableMacOSExplicit: UnavailableMacOSStruct = .init()
+  }
+}
+
+@available(*, unavailable)
+struct GoodUniversallyUnavailableStruct {
+  var universallyUnavailable: UniversallyUnavailableStruct = .init()
+  lazy var lazyUniversallyUnavailable: UniversallyUnavailableStruct = .init()
+
+  @available(*, unavailable)
+  var universallyUnavailableExplicit: UniversallyUnavailableStruct = .init()
+}
+
+struct BadStruct {
+  // expected-error@+1 {{stored properties cannot be marked unavailable with '@available'}}
+  @available(macOS, unavailable)
+  var unavailableMacOS: UnavailableMacOSStruct = .init()
+
+  // expected-error@+1 {{stored properties cannot be marked unavailable with '@available'}}
+  @available(macOS, unavailable)
+  lazy var lazyUnavailableMacOS: UnavailableMacOSStruct = .init()
+
+  // expected-error@+1 {{stored properties cannot be marked unavailable with '@available'}}
+  @available(*, unavailable)
+  var universallyUnavailable: UniversallyUnavailableStruct = .init()
+}
+
+enum GoodAvailableEnum {
+  @available(macOS, unavailable)
+  case unavailableMacOS(UnavailableMacOSStruct)
+}
diff --git a/test/Sema/availability_versions.swift b/test/Sema/availability_versions.swift
@@ -57,11 +57,6 @@ func functionAvailableOn10_51() {
       // expected-note@-1 {{add 'if #available' version check}}
 }
 
-// Don't allow script-mode globals to marked potentially unavailable. Their
-// initializers are eagerly executed.
-@available(OSX, introduced: 10.51) // expected-error {{global variable cannot be marked potentially unavailable with '@available' in script mode}}
-var potentiallyUnavailableGlobalInScriptMode: Int = globalFuncAvailableOn10_51()
-
 // Still allow other availability annotations on script-mode globals
 @available(OSX, deprecated: 10.51)
 var deprecatedGlobalInScriptMode: Int = 5
diff --git a/test/Sema/generalized_accessors_availability.swift b/test/Sema/generalized_accessors_availability.swift
diff --git a/test/api-digester/Inputs/cake.swift b/test/api-digester/Inputs/cake.swift
diff --git a/test/api-digester/Outputs/cake-abi.json b/test/api-digester/Outputs/cake-abi.json
diff --git a/test/attr/attr_availability_transitive_osx.swift b/test/attr/attr_availability_transitive_osx.swift
diff --git a/test/expr/unary/keypath/keypath.swift b/test/expr/unary/keypath/keypath.swift
