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

Set linkage of __crystal_* funs to internal #15439

Conversation

ysbaddaden
Copy link
Contributor

Unlike the usual fun for which we might expect to have external linkage, the __crystal_* funs are an internal implementation detail to abstract calls to stdlib from the codegen pass (e.g. __crystal_once, __crystal_raise, ...).

When compiling to a single module, we can mark them to have internal linkage, so they're only considered inside the object file. We already do that for all globals and def.

Related to #921

When compiling to a single module we can set the `__crystal_main`
function linkage to internal.
…e module

Unlike the usual fun that create a non mangled symbols, and is usually
expected to have external linkage, the `__crystal_*` funs are an
internal implementation detail to abstract calls to stdlib from the
codegen pass (e.g. `__crystal_once`, `__crystal_raise`, ...).

We should compile them to be internal when compiling to a single module.
@ysbaddaden
Copy link
Contributor Author

ysbaddaden commented Feb 9, 2025

Interesting, darwin fails to link, for aarch64 and x86_64 alike:

         Undefined symbols for architecture arm64:
           "___crystal_personality", referenced from:
               _~proc2Proc(Pointer(Void), Pointer(Void), Nil)@src/gc/boehm.cr:331 in _main.o3.o
               _*IO::FileDescriptor#unbuffered_write<Slice(UInt8)>:Nil in _main.o3.o
               _*Crystal::System::Thread::current_thread:Thread in _main.o3.o
               _*Crystal::EventLoop::Polling::Arena(Crystal::EventLoop::Polling::PollDescriptor, 65536)@Crystal::EventLoop::Polling::Arena(T, BLOCK_BYTESIZE)#at<Int32, Bool>:Pointer(Crystal::EventLoop::Polling::Arena::Entry(Crystal::EventLoop::Polling::PollDescriptor)) in _main.o3.o
               _*Crystal::Scheduler#reschedule:Nil in _main.o3.o
               _*Exception::CallStack#printable_backtrace:Array(String) in _main.o3.o
               _*Exception::CallStack::decode_line_number<UInt64>:Tuple(String, Int32, Int32) in _main.o3.o
               ...
         ld: symbol(s) not found for architecture arm64
         clang-11: error: linker command failed with exit code 1 (use -v to see invocation)

All symbols are declared in the _main.o3.o object file, so it should also declare __crystal_personality and I don't see why it wouldn't be able to find it if it's internal 😕

@straight-shoota straight-shoota removed this from the 1.16.0 milestone Feb 9, 2025
@ysbaddaden
Copy link
Contributor Author

$ nm ~/.cache/crystal/Users-runner-work-crystal-crystal-spec-std-data-collect_within_ensure/_main.o3.o | grep __crystal
0000000000000000 t ___crystal_main
0000000000000cb8 t ___crystal_personality
00000000000011dc t ___crystal_raise
0000000000001224 t ___crystal_raise_overflow

The symbol is in the object file, with internal linkage t). I'll try to list all the failing symbols. Maybe there's an external symbol in the list?

@ysbaddaden
Copy link
Contributor Author

ysbaddaden commented Feb 10, 2025

That seems to be a bug in macos ld 🤦

https://stackoverflow.com/questions/72566667/darwin-ld-stubbornly-wont-find-symbol-thats-defined-in-object-file-while-l

Is macos ruining support for #921? Yes.

Apparently it would only work with lld.

@ysbaddaden
Copy link
Contributor Author

ysbaddaden commented Feb 10, 2025

Since we use internal linkage for all defs, it might just be a bug with the personality function? 🤔

Confirmed: not making __crystal_personality internal fixes the ld issue on Darwin. Other funs don't have this issue.

I fail to link using cc -fuse-ld=ldd:

ld64.lld: warning: ignoring unknown argument: -platform_version
ld64.lld: warning: ignoring unknown argument: -no_uuid
ld64.lld: warning: -sdk_version is required when emitting min version load command.  Setting sdk version to match provided min version
Cannot open macos: No such file or directory

I notice that clang-11 injected -platform_version macos 11.0.0 0.0.0 into the ld command. That seems like the problem. Maybe it's working better with a newer llvm release but CI nix environment is still stuck to LLVM 11.

@straight-shoota
Copy link
Member

That SO question is also about a personality function. So perhaps it's related to some quirk of exception handling on macos?

@ysbaddaden ysbaddaden force-pushed the fix/set-internal-linkage-to-generated-crystal-main-fun branch from e615ba8 to 1938467 Compare February 10, 2025 13:41
@straight-shoota straight-shoota added this to the 1.16.0 milestone Feb 10, 2025
@ysbaddaden
Copy link
Contributor Author

ysbaddaden commented Feb 10, 2025

@straight-shoota straight-shoota changed the title Set internal linkage to __crystal_* fun Set linkage of __crystal_* funs to internal Feb 11, 2025
@straight-shoota straight-shoota merged commit ade1e6f into crystal-lang:master Feb 11, 2025
71 checks passed
@ysbaddaden ysbaddaden deleted the fix/set-internal-linkage-to-generated-crystal-main-fun branch February 11, 2025 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants