[Sema] Diagnose unsound pointer conversions

Diagnose ephemeral conversions that are passed to @_nonEphemeral
parameters. Currently, this defaults to a warning with a frontend flag
to upgrade to an error. Hopefully this will become an error by default
in a future language version.

Author: hamishknight

include/swift/AST/DiagnosticsSema.def

@@ -397,6 +397,35 @@ ERROR(cannot_convert_argument_value_generic,none,
       "cannot convert value of type %0 (%1) to expected argument type %2 (%3)",
       (Type, StringRef, Type, StringRef))
 
+// @_nonEphemeral conversion diagnostics
+ERROR(cannot_pass_type_to_non_ephemeral,none,
+      "cannot pass %0 to parameter; argument #%1 must be a pointer that "
+      "outlives the call%select{| to %3}2", (Type, unsigned, bool, DeclName))
+WARNING(cannot_pass_type_to_non_ephemeral_warning,none,
+        "passing %0 to parameter, but argument #%1 should be a pointer that "
+        "outlives the call%select{| to %3}2", (Type, unsigned, bool, DeclName))
+ERROR(cannot_use_inout_non_ephemeral,none,
+      "cannot use inout expression here; argument #%0 must be a pointer that "
+      "outlives the call%select{| to %2}1", (unsigned, bool, DeclName))
+WARNING(cannot_use_inout_non_ephemeral_warning,none,
+        "inout expression creates a temporary pointer, but argument #%0 should "
+        "be a pointer that outlives the call%select{| to %2}1",
+        (unsigned, bool, DeclName))
+NOTE(ephemeral_pointer_argument_conversion_note,none,
+     "implicit argument conversion from %0 to %1 produces a pointer valid only "
+     "for the duration of the call%select{| to %3}2",
+     (Type, Type, bool, DeclName))
+NOTE(ephemeral_use_with_unsafe_pointer,none,
+     "use 'withUnsafe%select{Bytes|MutableBytes|Pointer|MutablePointer}0' in "
+     "order to explicitly convert argument to %select{buffer |buffer ||}0"
+     "pointer valid for a defined scope", (unsigned))
+NOTE(ephemeral_use_string_with_c_string,none,
+     "use the 'withCString' method on String in order to explicitly "
+     "convert argument to pointer valid for a defined scope", ())
+NOTE(ephemeral_use_array_with_unsafe_buffer,none,
+     "use the 'withUnsafe%select{Bytes|MutableBytes|BufferPointer|"
+     "MutableBufferPointer}0' method on Array in order to explicitly convert "
+     "argument to buffer pointer valid for a defined scope", (unsigned))
 ERROR(cannot_convert_argument_value_protocol,none,
       "argument type %0 does not conform to expected type %1", (Type, Type))
 ERROR(cannot_convert_partial_argument_value_protocol,none,

include/swift/Basic/LangOptions.h

@@ -215,6 +215,10 @@ namespace swift {
     ///        operator protocol designator feature.
     bool SolverEnableOperatorDesignatedTypes = false;
 
+    /// Whether to diagnose an ephemeral to non-ephemeral conversion as an
+    /// error.
+    bool DiagnoseInvalidEphemeralnessAsError = false;
+
     /// The maximum depth to which to test decl circularity.
     unsigned MaxCircularityDepth = 500;
 

include/swift/Option/FrontendOptions.td

@@ -431,6 +431,10 @@ def solver_enable_operator_designated_types :
   Flag<["-"], "solver-enable-operator-designated-types">,
   HelpText<"Enable operator designated types in constraint solver">;
 
+def diagnose_invalid_ephemeralness_as_error :
+  Flag<["-"], "diagnose-invalid-ephemeralness-as-error">,
+  HelpText<"Diagnose invalid ephemeral to non-ephemeral conversions as errors">;
+
 def switch_checking_invocation_threshold_EQ : Joined<["-"],
     "switch-checking-invocation-threshold=">;
 

lib/Frontend/CompilerInvocation.cpp

@@ -276,7 +276,10 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
 
   Opts.EnableOperatorDesignatedTypes |=
       Args.hasArg(OPT_enable_operator_designated_types);
-  
+
+  Opts.DiagnoseInvalidEphemeralnessAsError |=
+      Args.hasArg(OPT_diagnose_invalid_ephemeralness_as_error);
+
   // Always enable operator designated types for the standard library.
   Opts.EnableOperatorDesignatedTypes |= FrontendOpts.ParseStdlib;
 

lib/Sema/CSDiagnostics.cpp

@@ -5426,3 +5426,134 @@ bool ExpandArrayIntoVarargsFailure::diagnoseAsNote() {
   }
   return false;
 }
+
+void NonEphemeralConversionFailure::emitSuggestionNotes() const {
+  auto getPointerKind = [](Type ty) -> PointerTypeKind {
+    PointerTypeKind pointerKind;
+    auto pointeeType = ty->lookThroughSingleOptionalType()
+                         ->getAnyPointerElementType(pointerKind);
+    assert(pointeeType && "Expected a pointer!");
+    (void)pointeeType;
+
+    return pointerKind;
+  };
+
+  // This must stay in sync with diag::ephemeral_use_array_with_unsafe_buffer
+  // and diag::ephemeral_use_with_unsafe_pointer.
+  enum AlternativeKind {
+    AK_Raw = 0,
+    AK_MutableRaw,
+    AK_Typed,
+    AK_MutableTyped,
+  };
+
+  auto getAlternativeKind = [&]() -> Optional<AlternativeKind> {
+    switch (getPointerKind(getParamType())) {
+    case PTK_UnsafeRawPointer:
+      return AK_Raw;
+    case PTK_UnsafeMutableRawPointer:
+      return AK_MutableRaw;
+    case PTK_UnsafePointer:
+      return AK_Typed;
+    case PTK_UnsafeMutablePointer:
+      return AK_MutableTyped;
+    case PTK_AutoreleasingUnsafeMutablePointer:
+      return None;
+    }
+  };
+
+  // First emit a note about the implicit conversion only lasting for the
+  // duration of the call.
+  auto *argExpr = getArgExpr();
+  emitDiagnostic(argExpr->getLoc(),
+                 diag::ephemeral_pointer_argument_conversion_note,
+                 getArgType(), getParamType(), getCallee(), getCalleeFullName())
+      .highlight(argExpr->getSourceRange());
+
+  // Then try to find a suitable alternative.
+  switch (ConversionKind) {
+  case ConversionRestrictionKind::ArrayToPointer: {
+    // Don't suggest anything for optional arrays, as there's currently no
+    // direct alternative.
+    if (getArgType()->getOptionalObjectType())
+      break;
+
+    // We can suggest using withUnsafe[Mutable][Bytes/BufferPointer].
+    if (auto alternative = getAlternativeKind())
+      emitDiagnostic(argExpr->getLoc(),
+                     diag::ephemeral_use_array_with_unsafe_buffer,
+                     *alternative);
+    break;
+  }
+  case ConversionRestrictionKind::StringToPointer: {
+    // Don't suggest anything for optional strings, as there's currently no
+    // direct alternative.
+    if (getArgType()->getOptionalObjectType())
+      break;
+
+    // We can suggest withCString as long as the resulting pointer is
+    // immutable.
+    switch (getPointerKind(getParamType())) {
+    case PTK_UnsafePointer:
+    case PTK_UnsafeRawPointer:
+      emitDiagnostic(argExpr->getLoc(),
+                     diag::ephemeral_use_string_with_c_string);
+      break;
+    case PTK_UnsafeMutableRawPointer:
+    case PTK_UnsafeMutablePointer:
+    case PTK_AutoreleasingUnsafeMutablePointer:
+      // There's nothing really sensible we can suggest for a mutable pointer.
+      break;
+    }
+    break;
+  }
+  case ConversionRestrictionKind::InoutToPointer:
+    // For an arbitrary inout-to-pointer, we can suggest
+    // withUnsafe[Mutable][Bytes/Pointer].
+    if (auto alternative = getAlternativeKind())
+      emitDiagnostic(argExpr->getLoc(), diag::ephemeral_use_with_unsafe_pointer,
+                     *alternative);
+    break;
+  case ConversionRestrictionKind::DeepEquality:
+  case ConversionRestrictionKind::Superclass:
+  case ConversionRestrictionKind::Existential:
+  case ConversionRestrictionKind::MetatypeToExistentialMetatype:
+  case ConversionRestrictionKind::ExistentialMetatypeToMetatype:
+  case ConversionRestrictionKind::ValueToOptional:
+  case ConversionRestrictionKind::OptionalToOptional:
+  case ConversionRestrictionKind::ClassMetatypeToAnyObject:
+  case ConversionRestrictionKind::ExistentialMetatypeToAnyObject:
+  case ConversionRestrictionKind::ProtocolMetatypeToProtocolClass:
+  case ConversionRestrictionKind::PointerToPointer:
+  case ConversionRestrictionKind::ArrayUpcast:
+  case ConversionRestrictionKind::DictionaryUpcast:
+  case ConversionRestrictionKind::SetUpcast:
+  case ConversionRestrictionKind::HashableToAnyHashable:
+  case ConversionRestrictionKind::CFTollFreeBridgeToObjC:
+  case ConversionRestrictionKind::ObjCTollFreeBridgeToCF:
+    llvm_unreachable("Expected an ephemeral conversion!");
+  }
+}
+
+bool NonEphemeralConversionFailure::diagnoseAsError() {
+  auto *argExpr = getArgExpr();
+  if (isa<InOutExpr>(argExpr)) {
+    auto diagID = DowngradeToWarning
+                      ? diag::cannot_use_inout_non_ephemeral_warning
+                      : diag::cannot_use_inout_non_ephemeral;
+
+    emitDiagnostic(argExpr->getLoc(), diagID, getArgPosition(), getCallee(),
+                   getCalleeFullName())
+        .highlight(argExpr->getSourceRange());
+  } else {
+    auto diagID = DowngradeToWarning
+                      ? diag::cannot_pass_type_to_non_ephemeral_warning
+                      : diag::cannot_pass_type_to_non_ephemeral;
+
+    emitDiagnostic(argExpr->getLoc(), diagID, getArgType(), getArgPosition(),
+                   getCallee(), getCalleeFullName())
+        .highlight(argExpr->getSourceRange());
+  }
+  emitSuggestionNotes();
+  return true;
+}

lib/Sema/CSDiagnostics.h

@@ -1910,6 +1910,36 @@ class MissingForcedDowncastFailure final : public ContextualFailure {
   bool diagnoseAsError() override;
 };
 
+/// Diagnose the invalid conversion of a temporary pointer argument generated
+/// from an X-to-pointer conversion to an @_nonEphemeral parameter.
+///
+/// ```swift
+/// func foo(@_nonEphemeral _ ptr: UnsafePointer<Int>) {}
+///
+/// foo([1, 2, 3])
+/// ```
+class NonEphemeralConversionFailure final : public ArgumentMismatchFailure {
+  ConversionRestrictionKind ConversionKind;
+  bool DowngradeToWarning;
+
+public:
+  NonEphemeralConversionFailure(Expr *expr, ConstraintSystem &cs,
+                                ConstraintLocator *locator,
+                                Type fromType, Type toType,
+                                ConversionRestrictionKind conversionKind,
+                                bool downgradeToWarning)
+      : ArgumentMismatchFailure(expr, cs, fromType, toType, locator),
+        ConversionKind(conversionKind), DowngradeToWarning(downgradeToWarning) {
+  }
+
+  bool diagnoseAsError() override;
+
+private:
+  /// Emits a note explaining to the user that an ephemeral conversion is only
+  /// valid for the duration of the call, and suggests an alternative to use.
+  void emitSuggestionNotes() const;
+};
+
 } // end namespace constraints
 } // end namespace swift
 

lib/Sema/CSFix.cpp

@@ -984,3 +984,25 @@ AllowArgumentMismatch::create(ConstraintSystem &cs, Type argType,
   return new (cs.getAllocator())
       AllowArgumentMismatch(cs, argType, paramType, locator);
 }
+
+bool TreatEphemeralAsNonEphemeral::diagnose(Expr *root, bool asNote) const {
+  NonEphemeralConversionFailure failure(
+      root, getConstraintSystem(), getLocator(), getFromType(), getToType(),
+      ConversionKind, isWarning());
+  return failure.diagnose(asNote);
+}
+
+TreatEphemeralAsNonEphemeral *TreatEphemeralAsNonEphemeral::create(
+    ConstraintSystem &cs, ConstraintLocator *locator, Type srcType,
+    Type dstType, ConversionRestrictionKind conversionKind,
+    bool downgradeToWarning) {
+  return new (cs.getAllocator()) TreatEphemeralAsNonEphemeral(
+      cs, locator, srcType, dstType, conversionKind, downgradeToWarning);
+}
+
+std::string TreatEphemeralAsNonEphemeral::getName() const {
+  llvm::SmallString<32> name;
+  name += "treat ephemeral as non-ephemeral for ";
+  name += ::getName(ConversionKind);
+  return name.c_str();
+}

lib/Sema/CSFix.h

@@ -41,6 +41,7 @@ class OverloadChoice;
 class ConstraintSystem;
 class ConstraintLocator;
 class ConstraintLocatorBuilder;
+enum class ConversionRestrictionKind;
 class Solution;
 
 /// Describes the kind of fix to apply to the given constraint before
@@ -211,6 +212,10 @@ enum class FixKind : uint8_t {
   /// If an array was passed to a variadic argument, give a specific diagnostic
   /// and offer to drop the brackets if it's a literal.
   ExpandArrayIntoVarargs,
+
+  /// Allow an ephemeral argument conversion for a parameter marked as being
+  /// non-ephemeral.
+  TreatEphemeralAsNonEphemeral,
 };
 
 class ConstraintFix {
@@ -1469,6 +1474,29 @@ class CoerceToCheckedCast final : public ContextualMismatch {
                                       Type toType, ConstraintLocator *locator);
 };
 
+class TreatEphemeralAsNonEphemeral final : public AllowArgumentMismatch {
+  ConversionRestrictionKind ConversionKind;
+
+  TreatEphemeralAsNonEphemeral(ConstraintSystem &cs, ConstraintLocator *locator,
+                               Type srcType, Type dstType,
+                               ConversionRestrictionKind conversionKind,
+                               bool downgradeToWarning)
+      : AllowArgumentMismatch(cs, FixKind::TreatEphemeralAsNonEphemeral,
+                              srcType, dstType, locator, downgradeToWarning),
+        ConversionKind(conversionKind) {}
+
+public:
+  ConversionRestrictionKind getConversionKind() const { return ConversionKind; }
+  std::string getName() const override;
+
+  bool diagnose(Expr *root, bool asNote = false) const override;
+
+  static TreatEphemeralAsNonEphemeral *
+  create(ConstraintSystem &cs, ConstraintLocator *locator, Type srcType,
+         Type dstType, ConversionRestrictionKind conversionKind,
+         bool downgradeToWarning);
+};
+
 } // end namespace constraints
 } // end namespace swift