swiftlang / swift-package-manager

The Package Manager for the Swift Programming Language
Apache License 2.0
9.75k stars 1.35k forks source link

Windows linker changes cause some of our symbols not to be emitted. #8111

Open grynspan opened 5 days ago

grynspan commented 5 days ago

Swift Testing has a symbol:

@_cdecl("swt_abiv0_getEntryPoint")
@usableFromInline func abiv0_getEntryPoint() -> UnsafeRawPointer {
  ...
}

With the changes in #8049, it looks like this symbol is no longer being exported and our tests around it have started failing.

dschaefer2 commented 5 days ago

If you declare that public does it help?

grynspan commented 5 days ago

Workin' on it as a workaround. @usableFromInline should be sufficient though.

dschaefer2 commented 5 days ago

I assume the compiler only exports public symbols on Windows.

grynspan commented 5 days ago

That would defeat the purpose of @usableFromInline though. :|

dschaefer2 commented 5 days ago

I'm wondering if this is a compiler bug. If @usableFromInline implies the symbol is going to be dllexport'ed, is that really happening?

grynspan commented 5 days ago

From discussing on Slack, it looks like this is failing because Swift Testing is being linked statically instead of dynamically into the test target, which means there aren't any exported symbols for us to look up at runtime.

We can add type: .dynamic to our package manifest, but we'll need SwiftPM to honour that and link to our dynamic product instead of the individual object files, otherwise it doesn't help.

dschaefer2 commented 4 days ago

Do you have a small reproducible sample? I did a package init enabling swift-testing and added the main branch of the swift-testing repo as a dependency to my test. That seems to have avoided using the DLL from the toolchain and it worked fine.

Or is this only for swift-testings tests?

grynspan commented 4 days ago

You should be able to repro by doing the following, approximately:

  1. Add Swift Testing as a package dependency so it's getting built from source and linked into your target.
  2. In your target, call GetProcAddress(GetModuleHandleW(nil), "swt_abiv0_getEntryPoint").
  3. The result of that call should not be nil, however it is showing up as nil for me with #8049 in place.

This is not a typical workflow for most Swift developers, but it is a supported one.

dschaefer2 commented 4 days ago

What do you do on Linux? I noticed this comment:

// NOTE: The standard Linux linker does not allow exporting symbols from // executables, so dlsym() does not let us find this function on that // platform when built as an executable rather than a dynamic library.

With my change that becomes true on Windows as well.

dschaefer2 commented 4 days ago

Also the reason changing Testing into a DLL doesn't work with this test is that you can't depend on a product in your own package (still not sure why that's not allowed). So you end up depending on the static object files.

dschaefer2 commented 4 days ago

So my current thinking is to pull back on the two build artifacts per module and instead check whether a module is included in a DLL product and decide whether to add -static based on that. The double artifact thing had opened the door to other grief anyway.

grynspan commented 4 days ago

So if we make Swift Testing's library target type: .dynamic it would opt out of -static, then?