[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

Jump to bottom

Draft

Techcable wants to merge 1 commit into rust-lang:master

from Techcable:fix/downcast-unchecked-check-ub

Draft

[DRAFT] Add ub_checks for downcast_unchecked #145684

Techcable wants to merge 1 commit into rust-lang:master from Techcable:fix/downcast-unchecked-check-ub

+21 −5

Conversation

Techcable

Copy link

@Techcable Techcable commented Aug 20, 2025 •

edited by rustbot

Loading

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!

rust/library/std/src/sys/fs/unix.rs

Lines 848 to 853 in 040a98a

// 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

@Techcable


 Add ub_checks for downcast_unchecked

42e03cc

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 T-libs labels

Aug 20, 2025

@Techcable Techcable marked this pull request as ready for review

August 20, 2025 23:58

@rustbot rustbot assigned ibraheemdev

Aug 20, 2025

@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 and removed S-waiting-on-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

@tgross35

Copy link

Contributor