Skip to content

Support separate modules for static and dynamic exporting symbols for Windows #8049

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 7, 2024

Conversation

dschaefer2
Copy link
Member

On Windows, there is a limit of around 64K to the number of symbols a DLL/EXE can export. We hit that regularly with large projects like SwiftPM itself. This hides those symbols and only exposes the ones requested for a DLL.

For Windows triples only, creates a parallel build graph for Swift modules, one for static linking using -static, and one for exporting linking which is the default. DLL products consume their direct target dependencies as exporting. All other dependencies use the static versions to eliminate unnecessary symbol exports.

The bulk of this is managed by the SwiftModuleBuildDescription which will create a duplicate of itself for the exporting case and set its own type to static linking. Both modules are fed to the planner to create llbuild swift command tasks. Code is added for dynamic libraries to hook up the correct inputs for exporting the symbols in the libraries targets.

This is WIP as we need to do a lot of testing to ensure we didn't break anything. Ensuring this only affects builds for Windows triples helps mitigate that.

if self.windowsTargetType == .exporting {
path = path.appending("exporting")
}
return path
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really don't like how we're putting all the modules in a single directory and adding it to the include path. Since the exporting modules have the same name, I had to create another directory to put them in and add that include path for the products that consume them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah putting all the modules in the same directory is extra annoying because you can end up importing things you haven't declared a dependency on.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question is, who do I break if I change this?

@dschaefer2
Copy link
Member Author

@swift-ci please test

@dschaefer2
Copy link
Member Author

One other issue I'm fighting is when building SwiftPM, I'm still getting the exporting imported symbols warning for the yaml_parser functions, despite having the YAML_DECLARE_STATIC flag being passed to the CYaml target.

@compnerd
Copy link
Member

-DYAML_DECLARE_STATIC should do the trick IIRC. Are we sure that it is getting to the C compiler and is also getting to the clang importer as -Xcc -DYAML_DECLARE_STATIC?

@rauhul
Copy link
Member

rauhul commented Oct 20, 2024

if I recall correctly, CSettings on a C target are not passed to Swift targets which depend on that target as -Xcc flags

@dschaefer2
Copy link
Member Author

if I recall correctly, CSettings on a C target are not passed to Swift targets which depend on that target as -Xcc flags

The warnings I'm seeing are:

lld-link: warning: C:\Users\dschaefer2\swift\swiftpm2\.build\x86_64-unknown-windows-msvc\debug\Yams.build\Parser.swift.o: locally defined symbol imported: yaml_event_delete (defined in C:\Users\dschaefer2\swift\swiftpm2\.build\x86_64-unknown-windows-msvc\debug\CYaml.build\src\api.c.o) [LNK4217]

Which does imply the issue is on the Swift side. That's something worth trying.

@dschaefer2
Copy link
Member Author

if I recall correctly, CSettings on a C target are not passed to Swift targets which depend on that target as -Xcc flags

Yup, @DougGregor ran into that with swift-java too. I've created #8095 to track that. (I'm sure that's another issue on that somewhere but I didn't spend enough time trying to find it :))

For Windows triples only, creates a parallel build graph for Swift
modules, one for static linking using -static, and one for
exporting linking which is the default. DLL products consume their
direct target dependencies as exporting. All other dependencies
use the static versions to eliminate unnecessary symbol exports.

The bulk of this is managed by the SwiftModuleBuildDescription
which will create a duplicate of itself for the exporting case
and set it's own type to static linking. Both modules are fed to
the planner to create llbuild swift command tasks. Code is added
for dynamic libraries to hook up the correct inputs for exporting
the symbols in the libraries targets.
@dschaefer2 dschaefer2 changed the title [WIP] Static versus exporting symbols for Windows Static versus exporting symbols for Windows Nov 6, 2024
@dschaefer2 dschaefer2 changed the title Static versus exporting symbols for Windows Support separate modules for static and dynamic exporting symbols for Windows Nov 6, 2024
@dschaefer2
Copy link
Member Author

@swift-ci please test

@dschaefer2
Copy link
Member Author

@swift-ci please test windows

@dschaefer2
Copy link
Member Author

This is ready for review. I'd like to start giving it some soak time with the snapshot builds once approved.

@dschaefer2 dschaefer2 merged commit f34907c into swiftlang:main Nov 7, 2024
5 checks passed
@dschaefer2 dschaefer2 deleted the win.linking branch November 7, 2024 15:35
@hjyamauchi
Copy link

Hi @dschaefer2 it seems like the following issue started after this PR/commit: #8110

I'm not sure if it's an issue on the user side or the spm side.

Wdyt?

@dschaefer2
Copy link
Member Author

Hi @dschaefer2 it seems like the following issue started after this PR/commit: swiftlang/swift-package-manager#8110

I'm not sure if it's an issue on the user side or the spm side.

Wdyt?

Thanks! I'll take a look. I am worried about side effects I didn't think about. This could be one.

@compnerd
Copy link
Member

compnerd commented Nov 8, 2024

I think that this is a modelling issue. The problem is that there are two libraries SwiftWin32UI and SwiftWin32. SwiftWin32UI depends on SwiftWin32. SwiftWin32 however has a dependency on SwiftCOM which is package external and has a dynamic library product. When the SwiftWin32 product is built (dynamic library), we link dynamically to SwiftCOM and everything is happy. When building SwiftWin32UI, we statically link against SwiftWin32. This uses the object library for SwiftWin32. However, this object library was built against the static library (object library) for SwiftCOM. We now attempt to link with SwiftCOM.lib (the import library for SwiftCOM) which assumes dynamic linking for symbols that we are expecting statically as we are building against the static library.

@dschaefer2
Copy link
Member Author

I've brought that issue over here as #8110 . We can continue diagnosis there.

grynspan added a commit to swiftlang/swift-testing that referenced this pull request Nov 11, 2024
Works around a link-time bug in SwiftPM that appears to have been introduced in
swiftlang/swift-package-manager#8049.
grynspan added a commit to swiftlang/swift-testing that referenced this pull request Nov 11, 2024
Works around a link-time bug in SwiftPM that appears to have been
introduced in
swiftlang/swift-package-manager#8049.

Works around
swiftlang/swift-package-manager#8111 until
swiftlang/swift-package-manager#8112 is in the
build.

### Checklist:

- [x] Code and documentation should follow the style of the [Style
Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md).
- [x] If public symbols are renamed or modified, DocC references should
be updated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants