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

Make VaList contain a VaListImpl directly. #238

Closed
wants to merge 35 commits into from

Conversation

jothan
Copy link

@jothan jothan commented Oct 19, 2024

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:

pub type __gnuc_va_list = [u32; 3usize];
pub type va_list = __gnuc_va_list;

BoxyUwU and others added 30 commits October 16, 2024 14:00
(cherry picked from commit 4c8b84a)
(cherry picked from commit 7c692e1)
…nikic"

This reverts commit 8c7a7e3, reversing
changes made to a00bd75.

(cherry picked from commit 472fef6)
(cherry picked from commit 12f2bcd)
…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 9c91a4e)
(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 <brian@tarricone.org>
* Updates uses of object::Architecture within the compiler
@jothan
Copy link
Author

jothan commented Oct 19, 2024

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
Copy link
Member

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 :)

@MabezDev MabezDev closed this Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.