Skip to content
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

Swift: missing symbols in ios_framework bundle #332

Open
stristr opened this issue Feb 14, 2019 · 19 comments
Open

Swift: missing symbols in ios_framework bundle #332

stristr opened this issue Feb 14, 2019 · 19 comments
Labels
P3 Would be nice, but probably next quarter at the earliest type: bug Something isn't working

Comments

@stristr
Copy link

stristr commented Feb 14, 2019

With most of the heavy lifting done by @steeve (see #223), I made a working ios_framework rule for building dynamic frameworks that can be dropped into vanilla (non-Bazel) Xcode projects. At runtime, I discovered that certain symbols that are present in the object file created by the swift_library rule are missing from the framework binary created by the ios_framework rule.

I believe I traced this to the following weird phenomenon: if a Swift source file contains any ObjC classes, all of its symbols make it into the framework binary, even if they're not bridged to ObjC. But if a Swift source file does not contain any ObjC classes, (say, if it only contains protocols, structs, and classes that don't extend NSObject), the apple_binary compile subcommand is totally stripping out all symbols generated from that source file's corresponding object file. I was able to debug up to this point, but it wasn't obvious to me from the wrapped clang command why this would be happening.

I created a simple repro repository here. The ./repro.sh bash script should print:

WillBeIncluded is in object file
WillBeExcluded is in object file
WillBeIncluded is in framework binary
WillBeExcluded is NOT in framework binary

Is this a known issue?

@keith
Copy link
Member

keith commented Feb 21, 2019

This is an interesting bug. Thanks for the awesome repro case, without that I definitely wouldn't have looked into this.

So the core problem is this limitation of the linker when it comes to Swift code https://bugs.swift.org/browse/SR-6004 the tl;dr of that is that there is no equivalent of -ObjC for Swift. A quick workaround for this is to add alwayslink = 1 to your swift_library targets (note this breaks your repro script because the libraries when using this attribute have a .lo extension instead of .a). Another quick workaround is to pass --linkopt=-all_load (but the former is preferred).

The bazel specific issue here, and how it differs from xcodebuild, is that bazel first builds a static library target from all your .o files, and that's the only thing it passes in the -filelist path/to/params to the final dynamic linking step. In this case the file list contains just foo.a and the linker is lazy. Specifically this means that the linker won't go get all the object files in the archive if it doesn't see that they're used. There's a ld flag that's slightly related to this (details in man ld) called -export_dynamic, but this just inhibits LTO stripping (which Swift seems to even be opting out of, but I'm not sure about that), it doesn't force loading more unused object files (this is what -all_load) does. So the reason Xcode doesn't have this problem is its -filelist argument contains each object file explicitly, so the linker happily loads each one, and then keeps the symbols.

I think one possible thing we could do in bazel is pass -all_load for everyone using ios_framework since if you're expecting a dynamic framework, you likely don't want to lose any symbols like this because you don't know what your consumers will use. I could see some arguments against this, but obviously this case is a very good one for it. I can look at how much work this would be to add (if it would be accepted) but in the meantime you can use one of the workarounds mentioned above.

@stristr
Copy link
Author

stristr commented Feb 21, 2019

Brilliant! Thanks for the great explanation. I confirmed alwayslink does solve this, and after looking at what that flag does I even get why it works 😄!

Agreed that handling this automatically with default linkopts makes a lot of sense for my use case. In general, if it helps make the case: for teams using Swift, ensuring the binary outputs from ios_[static_]framework rules are suitable for dropping into arbitrary Xcode projects unlocks "gradual adoption" in a broad sense. Along those lines, the work from #223 (prebuilt_swift_static_framework) really should live in the core rules as well. ios_framework should "just work" with Swift, Objective-C, and mixed sources.

@sergiocampama
Copy link
Contributor

One thing I'd like to comment is that ios_framework is not a distribution solution, but a packaging solution, while ios_static_framework is meant only for distribution.

We already do a similar thing to -all_load but for Objc, which is the -Objc flag. This means that everything ObjC is kept, and was introduced because categories were being dropped, but over time we've found that we'd actually like to remove this and I think some people are looking into propagating .o files instead of .a.

Another thing to consider is that C code gets deadstripped from dylibs if not being used and not marked as public/exportable, so that adds another vector of issues for missing symbols.

We don't currently have proposed solution and I don't believe anyone's actively working on this given that workarounds are available.

@sergiocampama sergiocampama added P3 Would be nice, but probably next quarter at the earliest type: bug Something isn't working labels Mar 12, 2019
@kastiglione
Copy link
Contributor

we've found that we'd actually like to remove this

@sergiocampama what has made you want to remove use of -ObjC?

some people are looking into propagating .o files instead of .a

What's the difference between this and -all_load? Many apps are sensitive to size, and while these solve missing symbols they could cause added bloat.

@sergiocampama
Copy link
Contributor

We'd like to remove -ObjC because it also adds bloat since it imports all code inside an archive that has any ObjC code that could otherwise be dead stripped, but it allows the linking of category-only objects. Moving to .os instead of .as would allow removing the -ObjC flag and an avenue for possible bloat as well.

@kastiglione
Copy link
Contributor

kastiglione commented Mar 12, 2019

Passing .o files to ld causes them to be added to the binary. Passing all inputs as .o files would be the same as -all_load, which is worse for bloat than -ObjC.

@kastiglione
Copy link
Contributor

kastiglione commented Mar 12, 2019

FWIW, I previously came up with a technique to solve the ObjC category linking problem, which works for linking apps. The approach is to annotate both category declarations and definitions with a macro. The definition gets a symbol that can be linked to (A), and the declaration gets a weak symbol that links to the one in the definition (&A). Then, if any file imports the category header, it will cause the linker to load the object file that contains the category definition via the associated symbol (A). This allows .a files to contain categories, and be linked without -ObjC. Here's an example: https://github.com/Instagram/IGListKit/pull/957/files

@steeve
Copy link
Contributor

steeve commented Sep 11, 2020

I just got this issue too, and making the swift_library.alwayslink = True fixed it. Perhaps ios_framework should consume all depending libraries as alwayslink ?

@liuliu
Copy link

liuliu commented Nov 4, 2020

Any idea how to make alwayslink propagate such that ios_framework will do the right thing if it is the direct dependency? I do see the value to not propagate all the way back, but it seems just propagate to the direct dependency is useful.

@keith
Copy link
Member

keith commented Nov 5, 2020

I would expect setting alwayslink = True on the swift_library would correctly propagate to the ios_framework

@liuliu
Copy link

liuliu commented Nov 5, 2020

@keith it is a bit different in my case, I want it to propagate the other way, i.e., for the rule that will generate the final .so / .dylib, it will propagate back so any swift_library the final rule depends on has alwayslink = True set. Any examples how to make that happen?

@keith
Copy link
Member

keith commented Nov 5, 2020

Ah got it, adding -all_load to the linkopts of the dylib target will probably work then

@liuliu
Copy link

liuliu commented Nov 5, 2020

Yeah. Found this rabbit hole actually related to bigger Bazel issue: bazelbuild/bazel#7362

The -all_load would work on Apple platform but not on the Linux (yeah, I am doing some weird things with Swift on Linux :P). I think the Bazel team will resolve it with the decoupling of rules_cc. Ideally, ios_framework probably should default package all direct dependencies (i.e. whole archive all direct dependencies). And for transitive dependencies, dead code elimination should still work. That seems more "natural" for a static language like Swift.

@keith
Copy link
Member

keith commented Nov 5, 2020

Is there no equivalent flag for gold?

@keith
Copy link
Member

keith commented Nov 5, 2020

ios_framework isn't meant for packaging so it doesn't need to link all dependents. ios_static_framework does do a whole archive link.

@liuliu
Copy link

liuliu commented Nov 5, 2020

Somehow it doesn't work with: clang -fuse-ld=gold -all_load aSwiftPackaged.a.

Cool, thanks for the clarification on the ios_framework! I think my use case is niche enough will wait on how they work with rules_cc and we can make platform independent changes in rules_swift accordingly.

@keith
Copy link
Member

keith commented Nov 5, 2020

The flag probably differs, you probably want --whole-archive

@liuliu
Copy link

liuliu commented Nov 5, 2020

Yeah, that has some whole other issues (multiple definitions due to some transitive dependencies), and even just --allow-multiple-definitions, it still doesn't package properly.

@jerrymarino
Copy link
Contributor

jerrymarino commented Nov 23, 2021

I came across this issue and seemed to be having the same problem with a test using symbols in a test host .app that aren't used in the main app. Similar to the issue reported here, the app built under Xcode gets symbols in the test host's binary because sources were linked as object files.

For my case, it was possible workaround this in LD with the -force_load flag. I came up with a rule to force load the deps only. In other situations, I wouldn't use alwayslink because the deps are used in other places

swift_libary(name="app-bin", srcs=["some.swift", "main.swift"])
ios_application(name="app", deps=["app-bin"])

Also, in a large single module app, Bazel should also link the object files directly to the app for performance reasons e.g. not linking 2x. This would also be addressed with object file propagation!

You could probably take the rule in rules_ios as-is or re-work into a PR: bazel-ios/rules_ios#358

jerrymarino added a commit to bazel-ios/rules_ios that referenced this issue Nov 23, 2021
ld has different behavior when loading members of a static library VS objects
as far as visibility. Under `-dynamic`
- linked _swift object files_ can have public visibility
- symbols from _swift static libraries_ are omitted unless used, and not visible otherwise

By using `-force_load`, we can load static libraries in the attributes of an
application's direct depenencies. These args need go at the _front_ of the
linker invocation otherwise these arguments don't work with lds logic.

Why not put it into `rules_apple`? Ideally it could be, and perhaps consider a
PR to there .The underlying java rule, `AppleBinary.linkMultiArchBinary`
places `extraLinkopts` at the end of the linker invocation. At the time of
writing these args need to go into the current rule context where
`AppleBinary.linkMultiArchBinary` is called.

One use case of this is that iOS developers want to load above mentioned
symbols from applications. Another alternate could be to create an aspect, that
actually generates a different application and linker invocation instead of
force loading symbols. This could be more complicated from an integration
perspective so it isn't used. Another case is present in dynamic frameworks in
rules_apple ( bazelbuild/rules_apple#332 )
jerrymarino added a commit to bazel-ios/rules_ios that referenced this issue Nov 24, 2021
* A rule to link with `-force_load` for direct`deps`

ld has different behavior when loading members of a static library VS objects
as far as visibility. Under `-dynamic`
- linked _swift object files_ can have public visibility
- symbols from _swift static libraries_ are omitted unless used, and not visible otherwise

By using `-force_load`, we can load static libraries in the attributes of an
application's direct depenencies. These args need go at the _front_ of the
linker invocation otherwise these arguments don't work with lds logic.

Why not put it into `rules_apple`? Ideally it could be, and perhaps consider a
PR to there .The underlying java rule, `AppleBinary.linkMultiArchBinary`
places `extraLinkopts` at the end of the linker invocation. At the time of
writing these args need to go into the current rule context where
`AppleBinary.linkMultiArchBinary` is called.

One use case of this is that iOS developers want to load above mentioned
symbols from applications. Another alternate could be to create an aspect, that
actually generates a different application and linker invocation instead of
force loading symbols. This could be more complicated from an integration
perspective so it isn't used. Another case is present in dynamic frameworks in
rules_apple ( bazelbuild/rules_apple#332 )
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 Would be nice, but probably next quarter at the earliest type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants