forked from rust-lang/rust
-
Notifications
You must be signed in to change notification settings - Fork 39
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
Make VaList contain a VaListImpl directly. #238
Closed
Closed
Conversation
This file contains 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
(cherry picked from commit 5b7be14)
(cherry picked from commit 8966207)
(cherry picked from commit 4c8b84a)
(cherry picked from commit 7c692e1)
(cherry picked from commit 12f2bcd)
(cherry picked from commit c3f9c4f)
…check_expr_struct_fields (cherry picked from commit 95b9ecd)
…ency and we have a trait error (cherry picked from commit 9d5d03b)
The library path is needed when the toolchain has been configured with `[rust] rpath = false`. Otherwise, building the reference book will get an error when it tries to run rustdoc, like: rustdoc: error while loading shared libraries: librustc_driver-2ec457c3b8826b72.so (cherry picked from commit de4c897)
(cherry picked from commit f46057b)
(cherry picked from commit 9cb540a)
(cherry picked from commit 9c91a4e)
(cherry picked from commit f0bfba2)
(cherry picked from commit e1c0f04)
(cherry picked from commit 7358429)
(cherry picked from commit 2f6307d)
(cherry picked from commit 0d94e6b)
(cherry picked from commit ebe4fc4)
LLVM does not include an implementation of the va_arg instruction for Xtensa. From what I understand, this is a conscious decision and instead language frontends are encouraged to implement it themselves. The rationale seems to be that loading values correctly requires language and ABI-specific knowledge that LLVM lacks. This is true of most architectures, and rustc already provides implementation for a number of them. This commit extends the support to include Xtensa. See https://lists.llvm.org/pipermail/llvm-dev/2017-August/116337.html for some discussion on the topic. Unfortunately there does not seem to be a reference document for the semantics of the va_list and va_arg on Xtensa. The most reliable source is the GCC implementation, which this commit tries to follow. Clang also provides its own compatible implementation. This was tested for all the types that rustc allows in variadics. Co-authored-by: Brian Tarricone <[email protected]>
* Updates uses of object::Architecture within the compiler
This is my current workaround for this issue: extern "C" {
fn vsnprintf(s: *mut c_char, n: usize, format: *const c_char, ap: VaListImpl) -> c_int;
} Replacing what should be VaList by VaListImpl works correctly. |
MabezDev
force-pushed
the
esp-1.82.0.0
branch
from
October 21, 2024 14:26
e698506
to
4144e1f
Compare
Thank you for the fix! I actually ran into this issue a few weeks ago and didn't realize it was this. I've applied your patch to the 1.82 branch and added you as a co-author of the vaarg patch so you still get credit :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
On Xtensa, va_list on the C side is clearly the size of 3 pointers.
It then follows that VaList should contain those pointers directly instead of containing a pointer to a VaListImpl.
This PR corrects which VaList implementation variant is used.
esp-idf-sys bindings: