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

Problems with C variadics #177

Closed
bjoernQ opened this issue May 17, 2023 · 4 comments
Closed

Problems with C variadics #177

bjoernQ opened this issue May 17, 2023 · 4 comments

Comments

@bjoernQ
Copy link

bjoernQ commented May 17, 2023

This relates to esp-rs/esp-wifi-sys#16

In https://github.com/bjoernQ/xtensa-rust-variadics-problem-repo I have some very simple code to reproduce the problem.

What I do there

  • Rust calls function in C
  • That function calls a function in Rust with variadics

On RISCV it works as expected and outputs

42 "TAG" "FORMAT"
1 = 1
2 = 2
3 = 3
4 = 4
5 = 5
6 = 6
7 = 7
8 = 8
9 = 9

On ESP32 it outputs

42 "TAG" "FORMAT"
1 = 1061160316
2 = 1
3 = 2
4 = 3
5 = 2148373693
6 = 1073594208
7 = 1074266112
8 = 1073414160
9 = 4
@karlri
Copy link

karlri commented Sep 18, 2023

I implemented the shortest solution I could come up with. It's not a full solution and it's not implemented in the proper place but it's provided in the case someone finds it useful.

esp-1.72.1.0...karlri:rust:esp-1.72.1.0

Ideally the compiler should be updated to not use the llvm intrinsic va_arg, or the va_arg intrinsic in llvm should be fixed, but I'm not competent enough to implement it where it should be implemented.

@karlri
Copy link

karlri commented Oct 4, 2023

I think I traced this bug back to its source.

I believe the bug to originate from here on line 340:
https://github.com/espressif/llvm-project/blob/xtensa_release_16.0.4/llvm/lib/Target/Xtensa/XtensaISelLowering.cpp

Can't Expand ISD::VAARG since we have a custom struct, we need Custom implementation.
It would need to routed around line 1924

  case ISD::VAARG:
    return LowerVAARG(Op, DAG);

finally, the big job would be to implement
SDValue XtensaTargetLowering::LowerVAARG(SDValue Op, SelectionDAG &DAG) const

@plietar
Copy link

plietar commented Oct 18, 2023

I've opened a PR at #201 which should fix this. With it the code in the reproduction repository works as intended, and so does the esp-wifi syslog facility.

@MabezDev
Copy link
Member

Closed with @plietar's patch in 1.73.0.1, thanks again @plietar! Note that not all builds are available, but by the end of the day they should be available :).

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 a pull request may close this issue.

4 participants