IRGen: special case VWT emission linkage computation

[compnerd]

The well known builtin and structural types are strongly defined in the
runtime which is compacted into the standard library. Given that the VWT
is defined in the runtime, it is not visible to the Swift compilation
process and as we do not provide a Swift definition, we would previously
compute the linkage as being module external (dllimport for shared
library builds). This formed incorrect references to these variables and
would require thunking to adjust the references.

One special case that we add here is the "any function" type
representation (@escaping () -> ()) as we do use the VWT for this type
in the standard library but do not consider it part of the well known
builtin or structural type enumeration.

These errors were previously being swallowed by the build system and
thus escaped from being fixed when the other cases of incorrect DLL
storage were.

[compnerd]

@swift-ci please test

[compnerd]

@swift-ci please test

[rjmccall]

The VWTs for well-known types are strong definitions in the standard library. That is a common situation arising with linkage that we really shouldn't need a special case for here. What is getAddrOfLLVMVariable doing that's wrong for PE?

[compnerd]

I'm not sure about all of them, but some are being given dllimport DLL storage (which then percolates to the VWT references into getAddrOfLLVMVariableOrGOTEquivalent which miscreates the variable). I was just in the process of sinking this check into getAddrOfLLVMVariable as well because we will need that to be present to fix the handling of the GOT equivalent case. I believe that this is happening as we do not have forward declarations from a header that properly annotates the DLL storage for the references to the (strong) definition in the runtime. One case that is particularly annoying is the @escaping () -> () VWT for some reason is failing to be marked correctly as it does not get accounted for in the well known set.

[rjmccall]

I didn't quite follow that. If we're emitting code for a different DLL than the standard library, we need these references to be dllimport, don't we? Can you back up and explain the bug you're seeing in basic terms?

[compnerd]

Okay, so, the situation is this:

1. The runtime strongly defines the value witness tables for the well known types. The runtime is implemented as a C/C++ library that is compacted into the standard library.
2. The standard library defines the well known structural and builtin types. This is implemented in Swift and is, as far as the compiler is concerned, a separate library from the runtime, but is a single DLL from the linker's perspective.

When we build the standard library, we need to ensure that we special case the references for the runtime functions and the runtime defined VWTs. We previously only special cased the runtime functions and because we were swallowing the linker reported issues due to the build system we never realized that we were only partially correctly building the runtime and relying on thunks.

The types enumerated by "well well builtin and structural types" is relatively small:

• ()
• AnyObject
• Builtin.Object
• Builtin.RawPointer
• Builtin.BridgedObject
• Int, UInt, Int[8|16|32|64|128|256], UInt[8|16|32|64|128|256]

This requires that we special case @escaping () -> () as well.

The reality is that just because it is a separate library does not tell us enough about whether it is statically or dynamically linked. For Swift entities, we now track this and have this serialised into the swiftmodule, however, for C/C++ libraries we rely on CPP macros and the headers. The runtime VWTs are not declared and imported through a clang module that would allow us to use CPP macros to inform the compiler that these external declarations are not dllimport but rather dllexport.

I'm using a slightly kludgy workaround here of checking these types and changing the linkage to say that the symbol is not dllimport or dllexport but rather just a local symbol and then rely on the C/C++ code to perform the dllexport to get the proper ABI surface.

Edit: I should mention that I have no real attachment to this change - if there is a better way to handle this, I'd be happy to change this as I suspect that we are going to face the complexity in the dll storage computation in order to enable the static standard library builds.

[compnerd]

As to the concrete bug that I am observing: we form references to the VWTs going through the IAT (import address table) - __imp_... rather than referencing the local symbol. You are not supposed to use that for the symbols that are marked as dllexport. The linker flags this and it just is something that has finally been exposed allowing us to try to correct this code generation issue.

[compnerd]

Yeah, that would be ideal, but the problem is that we don't always have the ability to compute that at the right time. I've been trying to get that isKnownLocal pinned down perfectly, but never managed to do that. We already run into another problematic situation that has a point solution as well: the runtime functions. Because we don't have the IGM always, we cannot identify if the current module is the standard library or not. The point solution there is to add a metadata tag into the LLVM module to identify it as a Swift module.

[rjmccall]

Why don't we have the IGM? That does not seem like something that would difficult to pass down.

[compnerd]

There are too many call sites to adjust for that IMO. We would effectively need to remove
LinkInfo LinkInfo::get(const UniversalLinkageInfo &,ModuleDecl *,const LinkEntity &,ForDefinition_t) to ensure that the IGM is always available. There is also llvm::Constant *swift::getRuntimeFn(llvm::Module &, llvm::Constant *&, const char *, llvm::CallingConv::ID, RuntimeAvailability, llvm::ArrayRef<llvm::Type *>, llvm::ArrayRef<llvm::Type *>, ArrayRef<Attribute::AttrKind>, ArrayRef<llvm::MemoryEffects>, IRGenModule *) which has an optional IGM which is only available when swift_willThrow is formed and not otherwise. I do think that fixing all the generation call sites seems quite a bit larger of an effort. This workaround is quite localised and at least lets us fix this issue.

Separately starting to remove the first signature for LinkInfo::get seems like a good follow up.

[compnerd]

Re-tracing the steps through this, I believe that even wiring in IGM alone is insufficient. getAddrOfLLVMVariableOrGOTEquivalent is given a LinkEntity which is then used to create the forward declaration of the variable via getAddOrLLVMVariable. The current version of this change is working on exactly that area - it is adjusting the IRLinkage of the created variable. This is the earliest point at which we can adjust the attributes of the declaration.

[compnerd]

@rjmccall - okay to merge with with the last comment?

[compnerd]

@swift-ci please test

[compnerd]

@swift-ci please test

[compnerd]

@swift-ci please test

[compnerd]

Ugh, the test failures are due to the missing type definition for Void in the test cases.

[compnerd]

@swift-ci please test

[compnerd]

@swift-ci please test

[compnerd]

@swift-ci please test Linux platform