Merge pull request #78464 from eeckstein/sil-linker-error

SILLinker: convert an assert to a compiler error message

Author: eeckstein

include/swift/AST/DiagnosticsSIL.def

@@ -408,6 +408,8 @@ ERROR(embedded_swift_allocating,none,
       "cannot use allocating operation in -no-allocations mode", ())
 ERROR(embedded_capture_of_generic_value_with_deinit,none,
       "capturing generic non-copyable type with deinit in escaping closure not supported in embedded Swift", ())
+ERROR(wrong_linkage_for_serialized_function,none,
+      "function has wrong linkage to be called from %0", (StringRef))
 NOTE(performance_called_from,none,
       "called from here", ())
 

lib/SIL/IR/Linker.cpp

@@ -57,6 +57,7 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/Debug.h"
+#include "swift/AST/DiagnosticsSIL.h"
 #include "swift/AST/ProtocolConformance.h"
 #include "swift/AST/SubstitutionMap.h"
 #include "swift/Basic/Assertions.h"
@@ -100,12 +101,32 @@ void SILLinkerVisitor::deserializeAndPushToWorklist(SILFunction *F) {
 
 /// Deserialize a function and add it to the worklist for processing.
 void SILLinkerVisitor::maybeAddFunctionToWorklist(
-    SILFunction *F, SerializedKind_t callerSerializedKind) {
+    SILFunction *F, SerializedKind_t callerSerializedKind, SILFunction *caller) {
   SILLinkage linkage = F->getLinkage();
-  ASSERT((callerSerializedKind == IsNotSerialized ||
+
+  // Originally this was an assert. But it can happen if the user "re-defines"
+  // an existing function with a wrong linkage, e.g. using `@_cdecl`.
+  if(!(callerSerializedKind == IsNotSerialized ||
             F->hasValidLinkageForFragileRef(callerSerializedKind) ||
-            hasSharedVisibility(linkage) || F->isExternForwardDeclaration()) &&
-         "called function has wrong linkage for serialized function");
+            hasSharedVisibility(linkage) || F->isExternForwardDeclaration())) {
+    StringRef name = "a serialized function";
+    llvm::SmallVector<char> scratch;
+
+    if (caller) {
+      name = caller->getName();
+      if (SILDeclRef declRef = caller->getDeclRef()) {
+        if (auto *decl = declRef.getDecl()) {
+          name = decl->getName().getString(scratch);
+        }
+      }
+    }
+    F->getModule().getASTContext().Diags.diagnose(
+      F->getLocation().getSourceLoc(),
+      diag::wrong_linkage_for_serialized_function, name);
+    hasError = true;
+    return;
+  }
+
   if (!F->isExternalDeclaration()) {
     // The function is already in the module, so no need to de-serialized it.
     // But check if we need to set the IsSerialized flag.
@@ -218,19 +239,22 @@ void SILLinkerVisitor::visitPartialApplyInst(PartialApplyInst *PAI) {
 
 void SILLinkerVisitor::visitFunctionRefInst(FunctionRefInst *FRI) {
   maybeAddFunctionToWorklist(FRI->getReferencedFunction(),
-                             FRI->getFunction()->getSerializedKind());
+                             FRI->getFunction()->getSerializedKind(),
+                             FRI->getFunction());
 }
 
 void SILLinkerVisitor::visitDynamicFunctionRefInst(
     DynamicFunctionRefInst *FRI) {
   maybeAddFunctionToWorklist(FRI->getInitiallyReferencedFunction(),
-                             FRI->getFunction()->getSerializedKind());
+                             FRI->getFunction()->getSerializedKind(),
+                             FRI->getFunction());
 }
 
 void SILLinkerVisitor::visitPreviousDynamicFunctionRefInst(
     PreviousDynamicFunctionRefInst *FRI) {
   maybeAddFunctionToWorklist(FRI->getInitiallyReferencedFunction(),
-                             FRI->getFunction()->getSerializedKind());
+                             FRI->getFunction()->getSerializedKind(),
+                             FRI->getFunction());
 }
 
 // Eagerly visiting all used conformances leads to a large blowup
@@ -470,6 +494,9 @@ void SILLinkerVisitor::process() {
     for (auto &BB : *Fn) {
       for (auto &I : BB) {
         visit(&I);
+
+        if (hasError)
+          return;
       }
     }
   }

lib/SIL/IR/Linker.h

@@ -91,6 +91,8 @@ class SILLinkerVisitor : public SILInstructionVisitor<SILLinkerVisitor, void> {
   /// Whether any functions were deserialized.
   bool Changed;
 
+  bool hasError = false;
+
 public:
   SILLinkerVisitor(SILModule &M, SILModule::LinkingMode LinkingMode)
       : Mod(M), Worklist(), Mode(LinkingMode), Changed(false) {}
@@ -143,7 +145,8 @@ class SILLinkerVisitor : public SILInstructionVisitor<SILLinkerVisitor, void> {
   /// If `callerSerializedKind` is IsSerialized, then all shared
   /// functions which are referenced from `F` are set to be serialized.
   void maybeAddFunctionToWorklist(SILFunction *F,
-                                  SerializedKind_t callerSerializedKind);
+                                  SerializedKind_t callerSerializedKind,
+                                  SILFunction *caller = nullptr);
 
   /// Is the current mode link all? Link all implies we should try and link
   /// everything, not just transparent/shared functions.

lib/SILOptimizer/UtilityPasses/Link.cpp

@@ -51,7 +51,9 @@ class SILLinker : public SILModuleTransform {
     using namespace RuntimeConstants;
 #define FUNCTION(ID, NAME, CC, AVAILABILITY, RETURNS, ARGS, ATTRS, EFFECT,     \
                  MEMORY_EFFECTS)                                               \
-  linkEmbeddedRuntimeFunctionByName(#NAME, EFFECT);
+  linkEmbeddedRuntimeFunctionByName(#NAME, EFFECT);                            \
+  if (getModule()->getASTContext().hadError())                                  \
+    return;
 
 #define RETURNS(...)
 #define ARGS(...)

test/embedded/wrong-linkage.swift

@@ -0,0 +1,8 @@
+// RUN: %target-swift-emit-ir %s -enable-experimental-feature Embedded -verify -wmo
+
+// REQUIRES: swift_feature_Embedded
+
+@_cdecl("posix_memalign")
+func posix_memalign(_ resultPtr:UnsafeMutablePointer<UnsafeMutableRawPointer?>, _ :Int, _ :Int) -> Int32 { // expected-error {{function has wrong linkage to be called from}}
+  return 0
+}