IRGen: attribute correct linkage to Windows DSO handle

Partially address the incorrect handling for the `#dsohandle` on
Windows.

We were previously emitting a local definition for this external
constant, and worse yet, not marking the definition for COMDAT.  It is
unclear what definition would win ultimately (implementation defined),
as we had a definition as well as the linker synthesized value. We can
change the SIL linkage for this type to `DefaultForDeclaration` which
will give it `available_externally` and default visibility and storage
which is closer to what we desire.  However, because we do not track the
LLVM variables and apply heuristics for lowering the
`SILGlobalVariable`, we would attribute it with imported DLL Storage.
This would then cause us to fail at link time (amusingly enough link.exe
will report a LNK1000).  Special case the variable and track that we are
targeting a windows environment in the `UniversalLinkageInfo` so that we
do not special case this on other platforms.

This also has the nice side effect of allowing us to remove the special
case in the TBD handling.

Fixes: #64741

Author: compnerd

include/swift/IRGen/Linking.h

@@ -45,6 +45,7 @@ bool useDllStorage(const llvm::Triple &triple);
 class UniversalLinkageInfo {
 public:
   bool IsELFObject;
+  bool IsMSVCEnvironment;
   bool UseDLLStorage;
   bool Internalize;
 

lib/FrontendTool/TBD.cpp

@@ -56,28 +56,6 @@ bool swift::writeTBD(ModuleDecl *M, StringRef OutputFilename,
   return false;
 }
 
-/// Determine if a symbol name is ignored when validating the TBD's contents
-/// against the IR's.
-///
-/// \param name The name of the symbol in question.
-/// \param IRModule The module being validated.
-///
-/// \returns Whether or not the presence or absence of the symbol named \a name
-///   should be ignored (instead of potentially producing a diagnostic.)
-static bool isSymbolIgnored(const StringRef& name,
-                            const llvm::Module &IRModule) {
-  if (llvm::Triple(IRModule.getTargetTriple()).isOSWindows()) {
-    // https://github.com/apple/swift/issues/58199
-    // Error when referencing #dsohandle in a Swift test on Windows.
-    // On Windows, ignore the lack of __ImageBase in the TBD file.
-    if (name == "__ImageBase") {
-      return true;
-    }
-  }
-
-  return false;
-}
-
 static bool validateSymbols(DiagnosticEngine &diags,
                             const std::vector<std::string> &symbols,
                             const llvm::Module &IRModule,
@@ -99,11 +77,6 @@ static bool validateSymbols(DiagnosticEngine &diags,
     // with what TBDGen created.
     auto unmangledName = nameValue.getKey();
 
-    if (isSymbolIgnored(unmangledName, IRModule)) {
-      // This symbol should not affect validation. Skip it.
-      continue;
-    }
-
     SmallString<128> name;
     llvm::Mangler::getNameWithPrefix(name, unmangledName,
                                      IRModule.getDataLayout());

lib/IRGen/GenDecl.cpp

@@ -2167,14 +2167,20 @@ void IRGenerator::emitEntryPointInfo() {
 }
 
 static IRLinkage
-getIRLinkage(const UniversalLinkageInfo &info, SILLinkage linkage,
-             ForDefinition_t isDefinition, bool isWeakImported,
-             bool isKnownLocal = false) {
+getIRLinkage(StringRef name, const UniversalLinkageInfo &info,
+             SILLinkage linkage, ForDefinition_t isDefinition,
+             bool isWeakImported, bool isKnownLocal = false) {
 #define RESULT(LINKAGE, VISIBILITY, DLL_STORAGE)                               \
   IRLinkage{llvm::GlobalValue::LINKAGE##Linkage,                               \
             llvm::GlobalValue::VISIBILITY##Visibility,                         \
             llvm::GlobalValue::DLL_STORAGE##StorageClass}
 
+  // This is a synthetic symbol that is referenced for `#dsohandle` and is never
+  // a definition but needs to be handled as a definition as it will be provided
+  // by the linker. This is a MSVC extension that is honoured by lld as well.
+  if (info.IsMSVCEnvironment && name == "__ImageBase")
+    return RESULT(External, Default, Default);
+
   // Use protected visibility for public symbols we define on ELF.  ld.so
   // doesn't support relative relocations at load time, which interferes with
   // our metadata formats.  Default visibility should suffice for other object
@@ -2208,7 +2214,7 @@ getIRLinkage(const UniversalLinkageInfo &info, SILLinkage linkage,
 
   case SILLinkage::Private: {
     if (info.forcePublicDecls() && !isDefinition)
-      return getIRLinkage(info, SILLinkage::PublicExternal, isDefinition,
+      return getIRLinkage(name, info, SILLinkage::PublicExternal, isDefinition,
                           isWeakImported, isKnownLocal);
 
     auto linkage = info.needLinkerToMergeDuplicateSymbols()
@@ -2262,8 +2268,9 @@ void irgen::updateLinkageForDefinition(IRGenModule &IGM,
       isKnownLocal = IGM.getSwiftModule() == MD || MD->isStaticLibrary();
 
   auto IRL =
-      getIRLinkage(linkInfo, entity.getLinkage(ForDefinition),
-                   ForDefinition, weakImported, isKnownLocal);
+      getIRLinkage(global->hasName() ? global->getName() : StringRef(),
+                   linkInfo, entity.getLinkage(ForDefinition), ForDefinition,
+                   weakImported, isKnownLocal);
   ApplyIRLinkage(IRL).to(global);
 
   LinkInfo link = LinkInfo::get(IGM, entity, ForDefinition);
@@ -2297,8 +2304,9 @@ LinkInfo LinkInfo::get(const UniversalLinkageInfo &linkInfo,
   }
 
   bool weakImported = entity.isWeakImported(swiftModule);
-  result.IRL = getIRLinkage(linkInfo, entity.getLinkage(isDefinition),
-                            isDefinition, weakImported, isKnownLocal);
+  result.IRL = getIRLinkage(result.Name, linkInfo,
+                            entity.getLinkage(isDefinition), isDefinition,
+                            weakImported, isKnownLocal);
   result.ForDefinition = isDefinition;
   return result;
 }
@@ -2308,8 +2316,8 @@ LinkInfo LinkInfo::get(const UniversalLinkageInfo &linkInfo, StringRef name,
                        bool isWeakImported) {
   LinkInfo result;
   result.Name += name;
-  result.IRL = getIRLinkage(linkInfo, linkage, isDefinition, isWeakImported,
-                            linkInfo.Internalize);
+  result.IRL = getIRLinkage(name, linkInfo, linkage, isDefinition,
+                            isWeakImported, linkInfo.Internalize);
   result.ForDefinition = isDefinition;
   return result;
 }

lib/IRGen/Linking.cpp

@@ -88,6 +88,7 @@ UniversalLinkageInfo::UniversalLinkageInfo(const llvm::Triple &triple,
                                            bool forcePublicDecls,
                                            bool isStaticLibrary)
     : IsELFObject(triple.isOSBinFormatELF()),
+      IsMSVCEnvironment(triple.isWindowsMSVCEnvironment()),
       UseDLLStorage(useDllStorage(triple)), Internalize(isStaticLibrary),
       HasMultipleIGMs(hasMultipleIGMs), ForcePublicDecls(forcePublicDecls) {}
 

lib/SILGen/SILGenExpr.cpp

@@ -4163,8 +4163,9 @@ visitMagicIdentifierLiteralExpr(MagicIdentifierLiteralExpr *E, SGFContext C) {
       auto ImageBase = M.lookUpGlobalVariable("__ImageBase");
       if (!ImageBase)
         ImageBase =
-            SILGlobalVariable::create(M, SILLinkage::Public, IsNotSerialized,
-                                      "__ImageBase", BuiltinRawPtrTy);
+            SILGlobalVariable::create(M, SILLinkage::DefaultForDeclaration,
+                                      IsNotSerialized, "__ImageBase",
+                                      BuiltinRawPtrTy);
       ModuleBase = B.createGlobalAddr(SILLoc, ImageBase);
     } else {
       auto DSOHandle = M.lookUpGlobalVariable("__dso_handle");