[Strict memory safety] Infer safe/unsafe for imported C types

Author: DougGregor

include/swift/ClangImporter/ClangImporterRequests.h

@@ -31,6 +31,7 @@ namespace swift {
 class Decl;
 class DeclName;
 class EnumDecl;
+enum class ExplicitSafety;
 
 /// The input type for a clang direct lookup request.
 struct ClangDirectLookupDescriptor final {
@@ -564,6 +565,25 @@ class ClangTypeEscapability
 void simple_display(llvm::raw_ostream &out, EscapabilityLookupDescriptor desc);
 SourceLoc extractNearestSourceLoc(EscapabilityLookupDescriptor desc);
 
+/// Determine the safety of the given Clang declaration.
+class ClangDeclExplicitSafety
+    : public SimpleRequest<ClangDeclExplicitSafety,
+                           ExplicitSafety(SafeUseOfCxxDeclDescriptor),
+                           RequestFlags::Cached> {
+public:
+  using SimpleRequest::SimpleRequest;
+
+  // Source location
+  SourceLoc getNearestLoc() const { return SourceLoc(); };
+  bool isCached() const;
+
+private:
+  friend SimpleRequest;
+
+  // Evaluation.
+  ExplicitSafety evaluate(Evaluator &evaluator, SafeUseOfCxxDeclDescriptor desc) const;
+};
+
 #define SWIFT_TYPEID_ZONE ClangImporter
 #define SWIFT_TYPEID_HEADER "swift/ClangImporter/ClangImporterTypeIDZone.def"
 #include "swift/Basic/DefineTypeIDZone.h"

include/swift/ClangImporter/ClangImporterTypeIDZone.def

@@ -45,3 +45,6 @@ SWIFT_REQUEST(ClangImporter, CustomRefCountingOperation,
 SWIFT_REQUEST(ClangImporter, ClangTypeEscapability,
               CxxEscapability(EscapabilityLookupDescriptor), Cached,
               NoLocationInfo)
+SWIFT_REQUEST(ClangImporter, ClangDeclExplicitSafety,
+              ExplicitSafety(SafeUseOfCxxDeclDescriptor), Cached,
+              NoLocationInfo)

lib/AST/Decl.cpp

@@ -1150,6 +1150,14 @@ ExplicitSafety Decl::getExplicitSafety() const {
   if (auto safety = getExplicitSafetyFromAttrs(this))
     return *safety;
 
+  // If this declaration is from C, ask the Clang importer.
+  if (auto clangDecl = getClangDecl()) {
+    ASTContext &ctx = getASTContext();
+    return evaluateOrDefault(
+        ctx.evaluator, ClangDeclExplicitSafety({clangDecl, ctx}),
+        ExplicitSafety::Unspecified);
+  }
+  
   // Inference: Check the enclosing context.
   if (auto enclosingDC = getDeclContext()) {
     // Is this an extension with @safe or @unsafe on it?

lib/ClangImporter/ClangImporter.cpp

@@ -8215,6 +8215,106 @@ CustomRefCountingOperationResult CustomRefCountingOperation::evaluate(
   return {CustomRefCountingOperationResult::tooManyFound, nullptr, name};
 }
 
+/// Check whether the given Clang type involves an unsafe type.
+static bool hasUnsafeType(
+    Evaluator &evaluator, ASTContext &swiftContext, clang::QualType clangType
+) {
+  // Handle pointers.
+  auto pointeeType = clangType->getPointeeType();
+  if (!pointeeType.isNull()) {
+    // Function pointers are okay.
+    if (pointeeType->isFunctionType())
+      return false;
+    
+    // Pointers to record types are okay if they come in as foreign reference
+    // types.
+    if (auto recordDecl = pointeeType->getAsRecordDecl()) {
+      if (hasImportAsRefAttr(recordDecl))
+        return false;
+    }
+    
+    // All other pointers are considered unsafe.
+    return true;
+  }
+  
+  // Handle records recursively.
+  if (auto recordDecl = clangType->getAsTagDecl()) {
+    auto safety = evaluateOrDefault(
+        evaluator, 
+        ClangDeclExplicitSafety({recordDecl, swiftContext}),
+        ExplicitSafety::Unspecified);
+    switch (safety) {
+      case ExplicitSafety::Unsafe:
+        return true;
+        
+      case ExplicitSafety::Safe:
+      case ExplicitSafety::Unspecified:
+        return false;        
+    }
+  }
+    
+  // Everything else is safe.
+  return false;
+}
+
+ExplicitSafety ClangDeclExplicitSafety::evaluate(
+    Evaluator &evaluator,
+    SafeUseOfCxxDeclDescriptor desc
+) const {
+  // FIXME: Somewhat duplicative with importAsUnsafe.
+  // FIXME: Also similar to hasPointerInSubobjects
+  // FIXME: should probably also subsume IsSafeUseOfCxxDecl
+  
+  // Explicitly unsafe.
+  auto decl = desc.decl;
+  if (hasUnsafeAPIAttr(decl) || hasSwiftAttribute(decl, "unsafe"))
+    return ExplicitSafety::Unsafe;
+  
+  // Explicitly safe.
+  if (hasSwiftAttribute(decl, "safe"))
+    return ExplicitSafety::Safe;
+  
+  // Enums are always safe.
+  if (isa<clang::EnumDecl>(decl))
+    return ExplicitSafety::Safe;
+  
+  // If it's not a record, leave it unspecified.
+  auto recordDecl = dyn_cast<clang::RecordDecl>(decl);
+  if (!recordDecl)
+    return ExplicitSafety::Unspecified;
+
+  // Escapable and non-escapable annotations imply that the declaration is
+  // safe.
+  if (hasNonEscapableAttr(recordDecl) || hasEscapableAttr(recordDecl))
+    return ExplicitSafety::Safe;
+  
+  // If we don't have a definition, leave it unspecified.
+  recordDecl = recordDecl->getDefinition();
+  if (!recordDecl)
+    return ExplicitSafety::Unspecified;
+  
+  // If this is a C++ class, check its bases.
+  if (auto cxxRecordDecl = dyn_cast<clang::CXXRecordDecl>(recordDecl)) {
+    for (auto base : cxxRecordDecl->bases()) {
+      if (hasUnsafeType(evaluator, desc.ctx, base.getType()))
+        return ExplicitSafety::Unsafe;
+    }
+  }
+  
+  // Check the fields.
+  for (auto field : recordDecl->fields()) {
+    if (hasUnsafeType(evaluator, desc.ctx, field->getType()))
+      return ExplicitSafety::Unsafe;
+  }
+  
+  // Okay, call it safe.
+  return ExplicitSafety::Safe;
+}
+
+bool ClangDeclExplicitSafety::isCached() const {
+  return isa<clang::RecordDecl>(std::get<0>(getStorage()).decl);
+}
+
 void ClangImporter::withSymbolicFeatureEnabled(
     llvm::function_ref<void(void)> callback) {
   llvm::SaveAndRestore<bool> oldImportSymbolicCXXDecls(

test/Interop/Cxx/class/safe-interop-mode.swift

@@ -71,23 +71,20 @@ import Test
 import CoreFoundation
 import CxxStdlib
 
-// expected-warning@+3{{global function 'useUnsafeParam' has an interface that involves unsafe types}}
-// expected-note@+2{{add '@unsafe' to indicate that this declaration is unsafe to use}}{{1-1=@unsafe }}
-// expected-note@+1{{add '@safe' to indicate that this declaration is memory-safe to use}}{{1-1=@safe }}
-func useUnsafeParam(x: Unannotated) { // expected-note{{reference to unsafe struct 'Unannotated'}}
+func useUnsafeParam(x: Unannotated) {
+  // expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}
+ _ = x // expected-note{{reference to parameter 'x' involves unsafe type}}
 }
 
-// expected-warning@+4{{global function 'useUnsafeParam2' has an interface that involves unsafe types}}
-// expected-note@+2{{add '@unsafe' to indicate that this declaration is unsafe to use}}{{1-1=@unsafe }}
-// expected-note@+1{{add '@safe' to indicate that this declaration is memory-safe to use}}{{1-1=@safe }}
 @available(SwiftStdlib 5.8, *)
-func useUnsafeParam2(x: UnsafeReference) { // expected-note{{reference to unsafe class 'UnsafeReference'}}
+func useUnsafeParam2(x: UnsafeReference) {
+  // expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}
+  _ = x // expected-note{{reference to parameter 'x' involves unsafe type}}
 }
 
-// expected-warning@+3{{global function 'useUnsafeParam3' has an interface that involves unsafe types}}
-// expected-note@+2{{add '@unsafe' to indicate that this declaration is unsafe to use}}{{1-1=@unsafe }}
-// expected-note@+1{{add '@safe' to indicate that this declaration is memory-safe to use}}{{1-1=@safe }}
-func useUnsafeParam3(x: UnknownEscapabilityAggregate) { // expected-note{{reference to unsafe struct 'UnknownEscapabilityAggregate'}}
+func useUnsafeParam3(x: UnknownEscapabilityAggregate) {
+  // expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}
+  _ = x // expected-note{{reference to parameter 'x' involves unsafe type}}
 }
 
 func useSafeParams(x: Owner, y: View, z: SafeEscapableAggregate, c: MyContainer) {
@@ -101,10 +98,9 @@ func useCfType(x: CFArray) {
 func useString(x: std.string) {
 }
 
-// expected-warning@+3{{global function 'useVecOfPtr' has an interface}}
-// expected-note@+2{{add '@unsafe' to indicate that this declaration is unsafe to use}}{{1-1=@unsafe }}
-// expected-note@+1{{add '@safe' to indicate that this declaration is memory-safe to use}}{1-1=@safe }}
-func useVecOfPtr(x: VecOfPtr) { // expected-note{{reference to unsafe type alias 'VecOfPtr'}}
+func useVecOfPtr(x: VecOfPtr) {
+  // expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}
+  _ = x // expected-note{{reference to parameter 'x' involves unsafe type}}
 }
 
 func useVecOfInt(x: VecOfInt) {
@@ -113,22 +109,19 @@ func useVecOfInt(x: VecOfInt) {
 func useSafeTuple(x: SafeTuple) {
 }
 
-// expected-warning@+3{{global function 'useUnsafeTuple' has an interface that involves unsafe types}}
-// expected-note@+2{{add '@unsafe' to indicate that this declaration is unsafe to use}}{{1-1=@unsafe }}
-// expected-note@+1{{add '@safe' to indicate that this declaration is memory-safe to use}}{1-1=@safe }}
-func useUnsafeTuple(x: UnsafeTuple) { // expected-note{{reference to unsafe type alias 'UnsafeTuple'}}
+func useUnsafeTuple(x: UnsafeTuple) {
+  // expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}
+  _ = x // expected-note{{reference to parameter 'x' involves unsafe type}}
 }
 
-// expected-warning@+3{{global function 'useCppSpan' has an interface that involves unsafe types}}
-// expected-note@+2{{add '@unsafe' to indicate that this declaration is unsafe to use}}{{1-1=@unsafe }}
-// expected-note@+1{{add '@safe' to indicate that this declaration is memory-safe to use}}{1-1=@safe }}
-func useCppSpan(x: SpanOfInt) { // expected-note{{reference to unsafe type alias 'SpanOfInt'}}
+func useCppSpan(x: SpanOfInt) {
+  // expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}
+  _ = x // expected-note{{reference to parameter 'x' involves unsafe type}}
 }
 
-// expected-warning@+3{{global function 'useCppSpan2' has an interface that involves unsafe types}}
-// expected-note@+2{{add '@unsafe' to indicate that this declaration is unsafe to use}}{{1-1=@unsafe }}
-// expected-note@+1{{add '@safe' to indicate that this declaration is memory-safe to use}}{1-1=@safe }}
-func useCppSpan2(x: SpanOfIntAlias) { // expected-note{{reference to unsafe type alias 'SpanOfIntAlias'}}
+func useCppSpan2(x: SpanOfIntAlias) {
+  // expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}
+  _ = x // expected-note{{reference to parameter 'x' involves unsafe type}}
 }
 
 func useSafeLifetimeAnnotated(v: View) {

test/Unsafe/Inputs/unsafe_decls.h

@@ -5,3 +5,62 @@ struct __attribute__((swift_attr("unsafe"))) UnsafeType {
 };
 
 void print_ints(int *ptr, int count);
+
+#define _CXX_INTEROP_STRINGIFY(_x) #_x
+
+#define SWIFT_SHARED_REFERENCE(_retain, _release)                                \
+  __attribute__((swift_attr("import_reference")))                          \
+  __attribute__((swift_attr(_CXX_INTEROP_STRINGIFY(retain:_retain))))      \
+  __attribute__((swift_attr(_CXX_INTEROP_STRINGIFY(release:_release))))
+
+#define SWIFT_SAFE __attribute__((swift_attr("@safe")))
+#define SWIFT_UNSAFE __attribute__((swift_attr("@unsafe")))
+
+struct NoPointers {
+  float x, y, z;
+};
+
+union NoPointersUnion {
+  float x;
+  double y;
+};
+
+struct NoPointersUnsafe {
+  float x, y, z;
+} SWIFT_UNSAFE;
+
+struct HasPointers {
+  float *numbers;
+};
+
+
+union HasPointersUnion {
+  float *numbers;
+  double x;
+};
+
+struct HasPointersSafe {
+  float *numbers;
+} SWIFT_SAFE;
+
+struct RefCountedType {
+  void *ptr;
+} SWIFT_SHARED_REFERENCE(RCRetain, RCRelease);
+
+struct RefCountedType *RCRetain(struct RefCountedType *object);
+void RCRelease(struct RefCountedType *object);
+
+struct HasRefCounted {
+  struct RefCountedType *ref;
+};
+
+struct ListNode {
+  double data;
+  struct ListNode *next;
+};
+
+enum SomeColor {
+  SCRed,
+  SCGreen,
+  SCBlue
+};

test/Unsafe/unsafe_c_imports.swift

@@ -0,0 +1,25 @@
+// RUN: %target-typecheck-verify-swift -enable-experimental-feature AllowUnsafeAttribute -enable-experimental-feature WarnUnsafe -I %S/Inputs
+
+// REQUIRES: swift_feature_AllowUnsafeAttribute
+// REQUIRES: swift_feature_WarnUnsafe
+
+import unsafe_decls
+
+// expected-warning@+3{{struct 'StoreAllTheThings' has storage involving unsafe types}}
+// expected-note@+2{{add '@unsafe' if this type is also unsafe to use}}
+// expected-note@+1{{add '@safe' if this type encapsulates the unsafe storage in a safe interface}}
+struct StoreAllTheThings {
+  let np: NoPointers
+  let npu: NoPointersUnion
+  let npu2: NoPointersUnsafe // expected-note{{property 'npu2' involves unsafe type 'NoPointersUnsafe'}}
+
+  let hp: HasPointers // expected-note{{property 'hp' involves unsafe type 'HasPointers'}}
+  let hpu: HasPointersUnion // expected-note{{property 'hpu' involves unsafe type 'HasPointersUnion'}}
+  let nps: HasPointersSafe
+
+  let hrc: HasRefCounted
+
+  let ln: ListNode // expected-note{{property 'ln' involves unsafe type 'ListNode'}}
+
+  let sc: SomeColor
+};