-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Unreferenced exports are removed when using .addAssemblyFile() #22234
Comments
Since this only happens in release modes, my immediate suspicion here is LTO issues. Please try setting |
@alexrp: You are correct; adding |
Can you show the assembly file that has the exports in question? |
@alexrp: The exports are in a Zig file. If memory serves, even an empty assembly file can trigger this issue. If it's helpful I can put together a test-case. |
Ah. Well, LTO probably figures that it's fine to remove them because you're building an executable. Can you try setting And by the way, the reason an assembly file triggers this issue is that LTO only happens if "C" source files are included in the compilation. (For the purposes of this, "C" means anything passed to Clang, which currently includes assembly files.) |
Hi @alexrp. |
Ok, I'm inclined to say that this isn't a bug then. If you build an executable without |
Isn't it a little odd (from the end-user's perspective) that the behaviour would change just by adding an assembly file (or C file) though? Is In my case, I'm trying to build a small OS for use with a boot-loader that supports multiboot. (https://github.com/andrewrk/HellOS is a good example of that) In that scenario you'd want to compile to an executable ELF but with the multiboot header included for the bootloader to detect (but otherwise unused by the program). If Zig aims the replace C then it strikes me that these sorts of use cases should be considered, and Zig should make it easy to avoid such 'gotchas'. Could/should the removal of unused symbols not be handled by the linker? (In my case I had Otherwise, I was wondering if it would be worth having some sort of annotation to force LTO to keep a particular export/symbol? |
Yes, but that quirk goes away with #22230 (diff).
All of this ultimately comes down to what LLD does in LTO. We're most likely dropping the LLD dependency in the next release cycle (0.15.0), and LTO along with it, so it's probably not worth spending too much time thinking about solutions to this (unless similar bugs can be encountered w/o LTO). |
Zig Version
0.14.0-dev.2370+5c6b25d9b
Steps to Reproduce and Observed Behavior
In recent versions of Zig (since v0.11?) there is some unintuitive behaviour when using
.addAssemblyFile()
as opposed to not using it.When not using
.addAssemblyFile()
the variables that areexport
ed from your.zig
files are present in the resulting binary. For example a multiboot header: https://github.com/andrewrk/HellOS/blob/68efb199d96bcd1c104e54c21bd8802f2612ec68/hellos.zig#L14`However, if you make use of
addAssemblyFile()
then theexport
ed variables will be removed unless they are referenced in your assembly file. (Which is a particular nuisance for multiboot headers, as there is no reason to reference it.)This behaviour is NOT seen in Debug builds.
This does not appear to be an issue with the final linking step, but rather an earlier step, as the data is missing from the
.o
file.If this is the intended behaviour, then please accept my apologies; it wasn't intuitive to me.
Rough steps to reproduce
export
s some data that is otherwise unreferencedhexdump .zig-cache/o/*/Proj.o | grep -i 'b002'
; you should find it.addAssemblyFile()
Expected Behavior
export
ed variables in.zig
source files should be present in the final binary, regardless of whetheraddAssemblyFile()
was used.I hope that the above is clear. If not, I will try to put together a test case, in the future. For now though, I wanted to get this written down while it's fresh in my mind, and also on the off chance that someone is able to easily understand why this is happening.
The text was updated successfully, but these errors were encountered: