IRGen: weak linkage is not supported on COFF, avoid it

[compnerd]

Disable weak linkage on COFF as it is not supported. Everything must be
fully resolved at link time. This disables the use of weak linkage if
that file format is in use.

Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.

Resolves SR-NNNN.

[compnerd]

CC: @rjmccall @jckarter

[compnerd]

@swift-ci please test

[rjmccall]

So, the obvious concern here is that this isn't an equivalent implementation — we were using weak linkage for a reason (presumably, that we don't have a guarantee that the symbol will be there), and using strong linkage instead just means that the program will crash.

[jckarter]

When would we try to weak-link things when targeting COFF to begin with? I thought that was an Apple-SDK-specific feature.

[compnerd]

@rjmccall - I agree, thats why I felt like I needed a review/discussion on this.

I think that it is actually going to fail to build period. The linker must statically resolve all symbols (the two level namespaces requires that the linker know what DLL is going to provide the external symbol), so it will fail to link entirely due to the required strong symbol.

Now, we could use an undocumented supported thing to provide a local definition that we default to that does nothing, but, I'm not sure how common it is to use the weak linkage in swift. How does one write something in swift that uses weak linkage (sorry, I don't immediately see how to write that, I suppose I could go spelunking into the code base).

[jrose-apple]

Weak linking is mainly for backwards-deployment, although we haven't actually wired that up properly for anything that isn't imported yet. Even on Apple platforms, though, the linker still statically resolves all symbols; it's just that they are permitted to be absent at load time. I guess there's no such thing as "dllimport-or-nullptr" on Windows?

[compnerd]

@jrose-apple - no, there is no dllimport or nullptr on Windows :-(. It must be available at link time and load time.

[jrose-apple]

Then as Joe points out, we're probably okay because we should never encounter such a situation to begin with. Maybe we should leave it as a fatal error for now rather than falling back to strong linkage.

[compnerd]

We need to fall back to strong linkage as the module itself is invalid otherwise. I think that we shouldn't be generating invalid IR.

[jrose-apple]

No, I mean that there shouldn't ever be a case where we generate weak linkage on Windows to begin with. That is, there should be no declarations for which the existing isWeakImported would return true.

[compnerd]

Yeah, that is what this change does :-).

[jckarter]

Our point is that the change seems unnecessary to begin with, since these conditions should never arise on a non-Apple platform.

[jrose-apple]

If you wanted to do something to catch this, you could immediately fail here with llvm::report_fatal_error rather than go on to generate an invalid module.

[rjmccall]

Yeah, that is what this change does :-).

If you're saying that there's some code today where we currently generate weak imports on COFF, and so without this change we'll produce an invalid module, then that's a bug we should fix — not by suppressing weak imports, but by implementing the feature relying on weak imports so that it stops trying to request them on COFF.

I agree with Jordan that it seems totally reasonable to assert right here that we're not on COFF, so that it crashes in the place that would be generating bad code instead of much later in a verifier or in the backend.