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.

Sign up for GitHub

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

Jump to bottom

fix undefined symbol errors #1180

Closed
wants to merge 1 commit into from
Closed

fix undefined symbol errors #1180

wants to merge 1 commit into from

Conversation

bnoordhuis
Copy link
Contributor

After commit fd75e73 ("add f16 type"), the Ubuntu CI machine started
failing with the following link-time errors:

lld: error: undefined symbol: __gnu_f2h_ieee
>>> referenced by math.zig:9(/home/travis/build/ziglang/zig/test/cases/math.zig:9)
>>>               ./zig-cache/test.o:(testDivision)

lld: error: undefined symbol: __gnu_h2f_ieee
>>> referenced by math.zig:9(/home/travis/build/ziglang/zig/test/cases/math.zig:9)
>>>               ./zig-cache/test.o:(testDivision)

Define the symbols unconditionally except on MacOS. This follows the
logic in LLVM's lib/CodeGen/TargetLoweringBase.cpp.

(This works for me locally but let's see how it fares on the CI.)

@bnoordhuis bnoordhuis mentioned this pull request Jun 30, 2018
Merged
@bnoordhuis
fix undefined symbol errors
8b6fb58 
After commit fd75e73 ("add f16 type"), the Ubuntu CI machine started
failing with the following link-time errors:

		lld: error: undefined symbol: __gnu_f2h_ieee
		>>> referenced by math.zig:9(/home/travis/build/ziglang/zig/test/cases/math.zig:9)
		>>>               ./zig-cache/test.o:(testDivision)

		lld: error: undefined symbol: __gnu_h2f_ieee
		>>> referenced by math.zig:9(/home/travis/build/ziglang/zig/test/cases/math.zig:9)
		>>>               ./zig-cache/test.o:(testDivision)

Export the symbols unconditionally except on MacOS.  This follows the
logic in LLVM's lib/CodeGen/TargetLoweringBase.cpp.

Don't export the symbols when testing on Windows because it results in
duplicate symbol errors when linking the compiler_rt tests (but not
other tests.)
@bnoordhuis
Copy link
Contributor Author

It's a bit of a puzzle but I hope I have it right now. To summarize:

  1. Weakly export the symbols everywhere but Windows.

  2. On Windows, export them iff builtin.is_test==false.

  3. Use the __extendhfsf2 / __truncsfhf2 names on MacOS and the __gnu_h2f_ieee / __gnu_f2h_ieee names everywhere else.

andrewrk
andrewrk approved these changes Jun 30, 2018
View reviewed changes
Copy link
Member

@andrewrk andrewrk left a comment

Choose a reason for hiding this comment

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

Looks good to me

@bnoordhuis
Copy link
Contributor Author

Still failing. Back to the drawing board...

It's weird I can't reproduce on a local trusty system with the packages from apt.llvm.org. Will need to study this more.

@andrewrk
Copy link
Member

andrewrk commented Jul 2, 2018 •
edited

I think I know what's going on here. Here's the failing test with --verbose-link:

[nix-shell:~/downloads/zig/build]$ ./zig test ../test/behavior.zig --library c --verbose-link
lld --gc-sections -m elf_x86_64 -o ./zig-cache/test /nix/store/84h2zni7h805k0i1ys2bba3dsp1cqnhh-glibc-2.26-131/lib/Scrt1.o /nix/store/84h2zni7h805k0i1ys2bba3dsp1cqnhh-glibc-2.26-131/lib/crti.o /nix/store/cm5znbhvylrh702hhsy9hnad7wk3v7xv-gcc-7.3.0/lib/gcc/x86_64-unknown-linux-gnu/7.3.0/crtbegin.o -L /nix/store/84h2zni7h805k0i1ys2bba3dsp1cqnhh-glibc-2.26-131/lib -L /nix/store/cm5znbhvylrh702hhsy9hnad7wk3v7xv-gcc-7.3.0/lib/gcc/x86_64-unknown-linux-gnu/7.3.0 -dynamic-linker /nix/store/84h2zni7h805k0i1ys2bba3dsp1cqnhh-glibc-2.26-131/lib/ld-linux-x86-64.so.2 ./zig-cache/test.o -lgcc --as-needed -lgcc_s --no-as-needed -lc -lm -lgcc --as-needed -lgcc_s --no-as-needed /nix/store/cm5znbhvylrh702hhsy9hnad7wk3v7xv-gcc-7.3.0/lib/gcc/x86_64-unknown-linux-gnu/7.3.0/crtend.o /nix/store/84h2zni7h805k0i1ys2bba3dsp1cqnhh-glibc-2.26-131/lib/crtn.o

On linux we don't link compiler_rt.o when we link against libc, because when we link against libc we also link with -lgcc which supposedly has all the compiler_rt symbols. Well, looks like the libgcc on travis does not. So we'll have to do the thing we already do for windows:

zig/src/link.cpp

Lines 551 to 560 in 0206b76

if (g->out_type == OutTypeExe || g->out_type == OutTypeLib) {
if (g->libc_link_lib == nullptr) {
Buf *builtin_o_path = build_o(g, "builtin");
lj->args.append(buf_ptr(builtin_o_path));
}
// msvc compiler_rt is missing some stuff, so we still build it and rely on LinkOnce
Buf *compiler_rt_o_path = build_compiler_rt(g);
lj->args.append(buf_ptr(compiler_rt_o_path));
}

The comment is a bit outdated - we've since changed the linkage of compiler-rt symbols to Weak. So I think we can change the ELF linking logic to be the same as this windows linking logic.

@andrewrk
Copy link
Member

andrewrk commented Jul 2, 2018

I believe this is fixed by 2759c79. Tests passed for me locally.

@andrewrk andrewrk closed this Jul 2, 2018
@andrewrk
Copy link
Member

andrewrk commented Jul 2, 2018

I don't know what's going on with the macos failure. I'll have access to my mac laptop in 2 days.

@bnoordhuis
Copy link
Contributor Author

Nice work, Andrew! Glad to see linux is working again. I'll try to take a look at macos tomorrow.

@bnoordhuis bnoordhuis deleted the fix-f16-link-errors branch July 3, 2018 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewrk andrewrk andrewrk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@bnoordhuis @andrewrk