Merged
Conversation
Contributor
|
r? @jroesch (rust_highfive has picked a reviewer for you, use r? to override) |
We use a 64bit integer to pass the set of attributes that is to be removed, but the called C function expects a 32bit integer. On most platforms this doesn't cause any problems other than being unable to unset some attributes, but on ARM even the lower 32bit aren't handled correctly because the 64bit value is passed in different registers, so the C function actually sees random garbage. So we need to fix the relevant functions to use 32bit integers instead. Additionally we need an implementation that actually accepts 64bit integers because some attributes can only be unset that way. Fixes rust-lang#32360
This reverts commit afbbb74.
| pub fn LLVMGetFunctionAttr(Fn: ValueRef) -> c_ulonglong; | ||
| pub fn LLVMRemoveFunctionAttr(Fn: ValueRef, val: c_ulonglong); | ||
| pub fn LLVMGetFunctionAttr(Fn: ValueRef) -> c_uint; | ||
| pub fn LLVMRemoveFunctionAttr(Fn: ValueRef, val: c_uint); |
Contributor
There was a problem hiding this comment.
@alexcrichton would it be possible to use ctest to check the validity of these LLVM bindings?
Member
There was a problem hiding this comment.
In theory yeah, but may be nontrivial to integrate
Member
|
@bors r+ |
Collaborator
|
📌 Commit 1eacb4a has been approved by |
Collaborator
Collaborator
| pub fn LLVMAddFunctionAttrStringValue(Fn: ValueRef, index: c_uint, | ||
| Name: *const c_char, | ||
| Value: *const c_char); | ||
| pub fn LLVMRemoveFunctionAttributes(Fn: ValueRef, index: c_uint, attr: uint64_t); |
Contributor
There was a problem hiding this comment.
how come attr doens't need to be c_ulonglong?
Contributor
Author
There was a problem hiding this comment.
The C side uses uint64_t not unsigned long long.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.