Merge pull request #75378 from jckarter/warn-on-runtime-function-symbol-references

Stage in a warning when trying to access symbols used by the compiler.

Author: jckarter

docs/ReferenceGuides/UnderscoredAttributes.md

@@ -465,6 +465,10 @@ Also similar to `@_silgen_name`, but a function declared with
 `@_extern(c)` is assumed to use the C ABI, while `@_silgen_name`
 assumes the Swift ABI.
 
+It is always better to refer to C declarations by importing their
+native declarations from a header or module using Swift's C interop
+support when possible.
+
 ## `@_fixed_layout`
 
 Same as `@frozen` but also works for classes.
@@ -1106,9 +1110,43 @@ Darwin) is maintained, unless "raw:" is used, in which case the name provided is
 expected to already be mangled.
 
 Since this has label-like behavior, it may not correspond to any declaration;
-if so, it is assumed that the function/global is implemented in C.
-
-A function defined by `@_silgen_name` is assumed to use the Swift ABI.
+if so, it is assumed that the function/global is implemented possibly
+in some other language; that implementation however is assumed to use
+the Swift ABI as if it were defined in Swift.
+
+There are very few legitimate uses for this attribute. There are many
+ways to misuse it:
+
+- Don't use `@_silgen_name` to access C functions, since those use the C ABI.
+  Import a header or C module to access C functions.
+- Don't use `@_silgen_name` to export Swift functions to C/ObjC. `@_cdecl` or
+  `@objc` can do that.
+- Don't use `@_silgen_name` to link to `swift_*` symbols from the Swift runtime.
+  Calls to these functions have special semantics to the compiler, and accessing
+  them directly will lead to unpredictable compiler crashes and undefined
+  behavior. Use language features, or if you must, the `Builtin` module, instead.
+- Don't use `@_silgen_name` for dynamic linker discovery. Swift symbols cannot
+  be reliably recovered through C interfaces like `dlsym`. If you want to
+  implement a plugin-style interface, use `Bundle`/`NSBundle` if available, or
+  export your plugin entry points as C entry points using `@_cdecl`.
+  
+Legitimate uses may include:
+
+- Use `@_silgen_name` if you're implementing the Swift runtime.
+- Use `@_silgen_name` if you need to make a change to an ABI-stable
+  declaration's signature that would normally alter its mangled name, but you
+  need to preserve the old mangled name for ABI compatibility. You will need
+  to be careful that the change doesn't materially affect the actual calling
+  convention of the function in an incompatible way.
+- Use `@_silgen_name` if certain declarations need to have predictable symbol
+  names, such as to be easily referenced by linker scripts or other highly
+  customized build environments (and it's OK for those predictable symbols to
+  reference functions with a Swift ABI).
+- Use `@_silgen_name` to interface build products that must be linked
+  together but built completely separately, such that one can't import the other
+  normally. For this to work, the declaration(s) and definition must exactly
+  match, using the exact same definitions of any referenced types or other
+  declarations. The compiler can't help you if you mismatch.
 
 For more details, see the
 [Standard Library Programmer's Manual](https://github.com/swiftlang/swift/blob/main/docs/StandardLibraryProgrammersManual.md#_silgen_name).

include/swift/AST/DiagnosticsSema.def

@@ -1957,6 +1957,12 @@ ERROR(section_linkage_markers_disabled,none,
 ERROR(section_empty_name,none,
       "@_section section name cannot be empty", ())
 
+// @_silgen_name and friends
+WARNING(reserved_runtime_symbol_name,none,
+        "symbol name '%0' is reserved for the Swift runtime and cannot be "
+        "directly referenced without causing unpredictable behavior; "
+        "this will become an error", (StringRef))
+
 // @_extern
 ERROR(attr_extern_experimental,none,
       "@_extern requires '-enable-experimental-feature Extern'", ())

lib/Sema/TypeCheckAttr.cpp

@@ -144,7 +144,6 @@ class AttributeChecker : public AttributeVisitor<AttributeChecker> {
   IGNORED_ATTR(NoObjCBridging)
   IGNORED_ATTR(EmitAssemblyVisionRemarks)
   IGNORED_ATTR(ShowInInterface)
-  IGNORED_ATTR(SILGenName)
   IGNORED_ATTR(StaticInitializeObjCMetadata)
   IGNORED_ATTR(SynthesizedProtocol)
   IGNORED_ATTR(Testable)
@@ -364,6 +363,8 @@ class AttributeChecker : public AttributeVisitor<AttributeChecker> {
 
   void visitStaticExclusiveOnlyAttr(StaticExclusiveOnlyAttr *attr);
   void visitWeakLinkedAttr(WeakLinkedAttr *attr);
+  
+  void visitSILGenNameAttr(SILGenNameAttr *attr);
 };
 
 } // end anonymous namespace
@@ -2223,6 +2224,30 @@ void AttributeChecker::visitAvailableAttr(AvailableAttr *attr) {
   }
 }
 
+static bool canDeclareSymbolName(StringRef symbol, ModuleDecl *fromModule) {
+  // The Swift standard library needs to be able to define reserved symbols.
+  if (fromModule->isStdlibModule()
+      || fromModule->getName() == fromModule->getASTContext().Id_Concurrency
+      || fromModule->getName() == fromModule->getASTContext().Id_Distributed) {
+    return true;
+  }
+
+  // Swift runtime functions are a private contract between the compiler and
+  // runtime, and attempting to access them directly without going through
+  // builtins or proper language features breaks the compiler in various hard
+  // to predict ways. Warn when code attempts to do so; hopefully we can
+  // promote this to an error after a while.
+  
+  return llvm::StringSwitch<bool>(symbol)
+#define FUNCTION(_, Name, ...) \
+    .Case(#Name, false) \
+    .Case("_" #Name, false) \
+    .Case(#Name "_", false) \
+    .Case("_" #Name "_", false)
+#include "swift/Runtime/RuntimeFunctions.def"
+    .Default(true);
+}
+
 void AttributeChecker::visitCDeclAttr(CDeclAttr *attr) {
   // Only top-level func decls are currently supported.
   if (D->getDeclContext()->isTypeContext())
@@ -2231,6 +2256,12 @@ void AttributeChecker::visitCDeclAttr(CDeclAttr *attr) {
   // The name must not be empty.
   if (attr->Name.empty())
     diagnose(attr->getLocation(), diag::cdecl_empty_name);
+
+  // The standard library can use @_cdecl to implement runtime functions.
+  if (!canDeclareSymbolName(attr->Name, D->getModuleContext())) {
+      diagnose(attr->getLocation(), diag::reserved_runtime_symbol_name,
+               attr->Name);
+  }
 }
 
 void AttributeChecker::visitExposeAttr(ExposeAttr *attr) {
@@ -2338,12 +2369,14 @@ void AttributeChecker::visitExternAttr(ExternAttr *attr) {
     diagnoseAndRemoveAttr(attr, diag::attr_extern_experimental);
     return;
   }
+  
   // Only top-level func or static func decls are currently supported.
   auto *FD = dyn_cast<FuncDecl>(D);
   if (!FD || (FD->getDeclContext()->isTypeContext() && !FD->isStatic())) {
     diagnose(attr->getLocation(), diag::extern_not_at_top_level_func);
   }
 
+
   // C name must not be empty.
   if (attr->getExternKind() == ExternKind::C) {
     StringRef cName = attr->getCName(FD);
@@ -2358,6 +2391,14 @@ void AttributeChecker::visitExternAttr(ExternAttr *attr) {
       diagnose(attr->getLocation(), diag::extern_c_maybe_invalid_name, cName)
           .fixItInsert(attr->getRParenLoc(), (", \"" + cName + "\"").str());
     }
+    
+    // Diagnose reserved symbol names.
+    // The standard library can't use normal C interop so needs extern(c)
+    // for access to C standard library and ObjC/Swift runtime functions.
+    if (!canDeclareSymbolName(cName, D->getModuleContext())) {
+      diagnose(attr->getLocation(), diag::reserved_runtime_symbol_name,
+               cName);
+    }
 
     // Ensure the decl has C compatible interface. Otherwise it produces diagnostics.
     if (!isCCompatibleFuncDecl(FD)) {
@@ -2407,6 +2448,13 @@ static bool allowSymbolLinkageMarkers(ASTContext &ctx, Decl *D) {
   return false;
 }
 
+void AttributeChecker::visitSILGenNameAttr(SILGenNameAttr *A) {
+  if (!canDeclareSymbolName(A->Name, D->getModuleContext())) {
+    diagnose(A->getLocation(), diag::reserved_runtime_symbol_name,
+             A->Name);
+  }
+}
+
 void AttributeChecker::visitUsedAttr(UsedAttr *attr) {
   if (!allowSymbolLinkageMarkers(Ctx, D)) {
     diagnoseAndRemoveAttr(attr, diag::section_linkage_markers_disabled);

test/Generics/slice_test.swift

@@ -30,8 +30,8 @@ func testslice(_ s: Array<Int>) {
   _ = s[..<2]
 }
 
-@_silgen_name("malloc") func c_malloc(_ size: Int) -> UnsafeMutableRawPointer
-@_silgen_name("free") func c_free(_ p: UnsafeMutableRawPointer)
+@_silgen_name("c_malloc") func c_malloc(_ size: Int) -> UnsafeMutableRawPointer
+@_silgen_name("c_free") func c_free(_ p: UnsafeMutableRawPointer)
 
 class Vector<T> {
   var length : Int

test/attr/attr_silgen_name.swift

@@ -1,4 +1,4 @@
-// RUN: %target-typecheck-verify-swift
+// RUN: %target-typecheck-verify-swift -enable-experimental-feature Extern
 
 @_silgen_name("foo") // expected-note {{attribute already specified here}}
 @_silgen_name("bar") // expected-error {{duplicate attribute}}
@@ -14,3 +14,17 @@ func func_with_nested__silgen_name() {
    exit(0)
 }
 
+// Ensure that magic runtime symbol names can't be declared or defined through
+// various symbol-assigning attributes
+
+@_silgen_name("swift_retain") // expected-warning{{reserved}}
+func liveDangerously() {}
+
+@_silgen_name("swift_retain") // expected-warning{{reserved}}
+func liveRecklessly();
+
+@_extern(c, "swift_retain") // expected-warning{{reserved}}
+func liveEphemerally()
+
+@_cdecl("swift_retain") // expected-warning{{reserved}}
+func liveFrivolously() {}