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

naked_asm: add cfi_startproc/cfi_endproc to all platforms #42

Merged
merged 1 commit into from
Dec 26, 2024

Conversation

xobs
Copy link
Contributor

@xobs xobs commented Dec 17, 2024

Since 2024-12-12 the Rust compiler now requires all naked ASM functions to include .cfi_startproc and .cfi_endproc. Without these directives, the build will fail.

The x86_64 target already had these directives, so it was unaffected by this change.

Add these directives to all other supported platforms.

This closes #41.

@xobs xobs force-pushed the add-cfi_startproc-cfi_endproc branch 2 times, most recently from 8dc3d9d to 7e0d1ee Compare December 17, 2024 01:37
@xobs
Copy link
Contributor Author

xobs commented Dec 17, 2024

The failure in riscv64gc is unrelated to this patch, since that file is not changed. I haven't managed to get it to fail in my local build yet.

@xobs xobs force-pushed the add-cfi_startproc-cfi_endproc branch from 7e0d1ee to 385687b Compare December 17, 2024 01:40
@xobs
Copy link
Contributor Author

xobs commented Dec 17, 2024

Oh interesting, it only breaks in --release

@xobs
Copy link
Contributor Author

xobs commented Dec 17, 2024

This is failing due to rust-lang/rust#80608

@xobs
Copy link
Contributor Author

xobs commented Dec 17, 2024

I've worked around it by manually adding the +d feature flag, as suggested in rust-lang/rust#134403 (comment)

Comment on lines 139 to 140
.option push // Work around Rust issue #80608
.option arch, +d // Work around Rust issue #80608
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this isn't necessary, this is only used in normal asm which doesn't have the global_asm/naked_asm bug.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does seem that way. Let me remove that (and in riscv64) and see if it passes.

@xobs xobs force-pushed the add-cfi_startproc-cfi_endproc branch from f8aa6bf to c6f2860 Compare December 26, 2024 04:43
@nbdd0121 nbdd0121 force-pushed the add-cfi_startproc-cfi_endproc branch from c6f2860 to 4b6f428 Compare December 26, 2024 14:44
Since 2024-12-12 the Rust compiler now requires all naked ASM functions
to include `.cfi_startproc` and `.cfi_endproc`. Without these
directives, the build will fail.

Add these directives to all supported platforms.

This also works around rust-lang/rust#80608 by forcing LLVM to consider that
code with the "d" extension has an FPU.

Signed-off-by: Sean Cross <[email protected]>
@nbdd0121 nbdd0121 force-pushed the add-cfi_startproc-cfi_endproc branch from 4b6f428 to 8f3beee Compare December 26, 2024 14:45
@nbdd0121 nbdd0121 merged commit 8f3beee into nbdd0121:trunk Dec 26, 2024
5 checks passed
@nbdd0121
Copy link
Owner

Thank you!

@xobs xobs deleted the add-cfi_startproc-cfi_endproc branch December 26, 2024 14:47
@xobs
Copy link
Contributor Author

xobs commented Dec 26, 2024

Thanks for merging! Would it be possible to bump the version number and release a new version so I can unbreak the nightly builds for riscv32imac-unknown-xous-elf?

@nbdd0121
Copy link
Owner

A new version should have already been published.

@xobs
Copy link
Contributor Author

xobs commented Dec 26, 2024

Thank you, I see that now.

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.

error: <inline asm>:11:1: this directive must appear between .cfi_startproc and .cfi_endproc directives
2 participants