Fix #121126: index out of bounds exceeds max value

[c4rrao]

When indexing an array with an index (u32) that exceeds the maximum value allowed by FieldIdx (default: 0xFFFF_FF00), although the compiler would detect the error, it would also cause a panic, which is a bug. I fixed it by adding a verification before calling the FieldIdx::from_u32(idx) method. This check ensures that if the idx value is greater than the maximum allowed value, it returns Option::None, similar to how other functions handle errors during the call to the project method of type Value.

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @TaKO8Ki (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[rustbot]

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

[oli-obk]

Please implement this as a method on all index types instead. Like try_from_u32 and then implement from_u32 by using that

[c4rrao]

Can we have from_u32 returning Option instead of Self? I can't see a way to inform the caller that the operation was unsuccessful without using Option, Result or panicking. And I guess Result is not the way and panicking was what we had before with the assert()

[oli-obk]

Hmm... we could just always return an option, good point. Yea lets do that instead of my idea of having both a try_from and a from method

[c4rrao]

I have a doubt here. In this match inside project method from Value type, why do we always cast the usize idx to a u32 just to call then from_u32 when we have also a from_usize method?

[oli-obk]

probably just an oversight. using from_usize is better

[rustbot]

There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

You can start a rebase with the following commands:

$ # rebase
$ git pull --rebase https://github.com/rust-lang/rust.git master
$ git push --force-with-lease

The following commits are merge commits:

• 57a5b75

[rust-log-analyzer]

The job x86_64-gnu-llvm-17 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

#16 exporting to docker image format
#16 sending tarball 29.5s done
#16 DONE 32.9s
##[endgroup]
Setting extra environment values for docker:  --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-17]
---
sccache: Starting the server...
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-17', '--enable-llvm-link-shared', '--set', 'rust.thin-lto-import-instr-limit=10', '--set', 'change-id=99999999', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--enable-new-symbol-mangling']
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-17/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.thin-lto-import-instr-limit := 10
configure: change-id            := 99999999
---
1 error: this operation will panic at runtime
-   --> $DIR/issue-121126-index-out-of-bounds-exceeds-max-value.rs:8:5
+   --> $DIR/index-out-of-bounds-exceeds-max-value-issue-121126.rs:8:5
3    |
4 LL |     [0][0xFFFF_FF01];
5    |     ^^^^^^^^^^^^^^^^ index out of bounds: the length is 1 but the index is 4294967041

The actual stderr differed from the expected stderr.
The actual stderr differed from the expected stderr.
Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/indexing/index-out-of-bounds-exceeds-max-value-issue-121126/index-out-of-bounds-exceeds-max-value-issue-121126.stderr
To only update this specific test, also pass `--test-args indexing/index-out-of-bounds-exceeds-max-value-issue-121126.rs`

error: 1 errors occurred comparing output.
status: exit status: 1
status: exit status: 1
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/indexing/index-out-of-bounds-exceeds-max-value-issue-121126.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2" "--target=x86_64-unknown-linux-gnu" "--check-cfg" "cfg(FALSE)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/indexing/index-out-of-bounds-exceeds-max-value-issue-121126" "-A" "unused" "-A" "internal_features" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/indexing/index-out-of-bounds-exceeds-max-value-issue-121126/auxiliary"
--- stderr -------------------------------
error: this operation will panic at runtime
##[error]  --> /checkout/tests/ui/indexing/index-out-of-bounds-exceeds-max-value-issue-121126.rs:8:5
   |
   |
LL |     [0][0xFFFF_FF01];
   |     ^^^^^^^^^^^^^^^^ index out of bounds: the length is 1 but the index is 4294967041
   = note: `#[deny(unconditional_panic)]` on by default

error: aborting due to 1 previous error
------------------------------------------

[bors]

☔ The latest upstream changes (presumably #125821) made this pull request unmergeable. Please resolve the merge conflicts.

[c4rrao]

A similar PR #125821 was already accepted, so I'm closing this one.