Skip to content

[DRAFT] Add ub_checks for downcast_unchecked #145684

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Techcable
Copy link

@Techcable Techcable commented Aug 20, 2025

Right now debug_assert! is used, which will not trigger in user code.

This is likely unacceptable for performance reasons, since the optimizer cannot understand virtual Any::type_id() calls.

This could potentially be fixed by applying something like #[ffi_const] to the Any::type_id function (issue #58328), which would have wider reaching performance benefits. Unfortunately, #[ffi_const] is not possible right now because it is limited to FFI calls (as its name suggests).

Ignoring the performance issue, I wasn't quite sure how to implement the actual assertion. It cannot use the assert_unsafe_precondition! macro because that requires the assertion to work in a const context. The closest thing I could find in the stdlib seems to be debug_assert_fd_is_open, which uses rtabort!

// this is similar to assert_unsafe_precondition!() but it doesn't require const
if core::ub_checks::check_library_ub() {
if unsafe { libc::fcntl(fd, libc::F_GETFD) } == -1 && errno() == libc::EBADF {
rtabort!("IO Safety violation: owned file descriptor already closed");
}
}

However, use of rtabort! requires std. The current choice of assert! has the possibility of triggering unwinding, which is inconsistent with the behavior of the other UB checks. Another possibility would be to outline the check into a helper function annotated with #[rustc_nounwind]

I have verified the old assertion doesn't trigger in user code, but I have not tested this PR because it is a very early draft and I don't have much rust compiler experience.

Cross Reference #90850 and #123499

Right now debug_assert! is used, which will not trigger in user code.

This is likely unacceptable for performance reasons,
since the optimizer can not understand virtual Any::type_id() calls.
This could potentially be fixed if we apply something like #[ffi_const]
to the Any::type_id() function (which would have wierder reaching benefits).

This cannot use the assert_unsafe_precondition! macro,
because that requires the assertion to work in a const context.
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 20, 2025
@Techcable Techcable marked this pull request as ready for review August 20, 2025 23:58
@rustbot
Copy link
Collaborator

rustbot commented Aug 20, 2025

r? @ibraheemdev

rustbot has assigned @ibraheemdev.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 20, 2025
@Techcable Techcable changed the title Add ub_checks for downcast_unchecked [DRAFT] Add ub_checks for downcast_unchecked Aug 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants