-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
compiler: fix llvmcall segfault with zero-sized types #60111
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
base: master
Are you sure you want to change the base?
Conversation
|
Thanks! CI is failing with errors related to this PR, so maybe have a look at that. @vchuravy, would you be willing to give a closer review? |
|
I would defer to @vtjnash for the ABI part |
413e519 to
b955f00
Compare
| bool retboxed; | ||
| Type *rettype = julia_type_to_llvm(ctx, rtt, &retboxed); | ||
| if (jl_is_datatype(rtt) && jl_datatype_size((jl_datatype_t*)rtt) == 0) { | ||
| emit_error(ctx, "llvmcall does not support zero-sized return types"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about void functions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added a check for that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The semantics here of checking if the user made an egregious typo are still wrong. This is also well formed:
julia> mutable struct A end
julia> f() = Core.Intrinsics.llvmcall("ret ptr addrspace(10) null", A, Tuple{})
This sort of random checking for user error doesn't really make sense to me. The point of llvmcall is an unsafe escape hatch into unsafe territory. If you don't like that, just don't use it.
Fixes the segmentation fault that occurred when passing zero-sized types to
llvmcall. The compiler now validates argument and return types and throws a clear error message instead of crashing.Before
After
Changes
fixes: #60063