Avoid ICE when is_val_statically_known is not of a supported type #120484

Teapot4195 · 2024-01-30T02:15:03Z

2 ICE with 1 stone!

Implement llvm.is.constant.ptr to avoid first ICE in linked issue.
return false when the argument is not one of i*/f*/ptr to avoid second ICE.

fixes #120480

rustbot · 2024-01-30T02:15:11Z

r? @TaKO8Ki

(rustbot has picked a reviewer for you, use r? to override)

compiler-errors · 2024-01-30T03:19:48Z

@bors r+

bors · 2024-01-30T03:19:51Z

📌 Commit 7bdf705 has been approved by compiler-errors

It is now in the queue for this repository.

compiler-errors · 2024-01-30T03:20:12Z

@bors r-

hold up, this needs a test

compiler-errors · 2024-01-30T03:21:26Z

You can put it into tests/ui/intrinsics and please give them a useful name describing what it does and not just issue-###.rs.

Teapot4195 · 2024-01-30T03:22:36Z

Ah, let me write a test for this.

Actually tests/codegen/is_val_statically_known.rs already exists, extend it for these two cases?

compiler-errors · 2024-01-30T03:25:15Z

Up to you

Teapot4195 · 2024-01-30T20:25:08Z

@rustbot ready

compiler-errors · 2024-01-30T21:12:52Z

@bors r+

bors · 2024-01-30T21:12:54Z

📌 Commit a97ff2a has been approved by compiler-errors

It is now in the queue for this repository.

…piler-errors Avoid ICE when is_val_statically_known is not of a supported type 2 ICE with 1 stone! 1. Implement `llvm.is.constant.ptr` to avoid first ICE in linked issue. 2. return `false` when the argument is not one of `i*`/`f*`/`ptr` to avoid second ICE. fixes rust-lang#120480

NCGThompson · 2024-02-01T01:10:02Z

tests/codegen/is_val_statically_known.rs

+
+#[inline]
+pub fn _iref(a: &u8) -> i32 {
+    if unsafe { is_val_statically_known(a) } { 5 } else { 4 }
+}
+
+// CHECK-LABEL: @_iref_borrow(
+#[no_mangle]
+pub fn _iref_borrow() -> i32 {
+    // CHECK: ret i32 4
+    _iref(&0)
+}
+
+// CHECK-LABEL: @_iref_arg(
+#[no_mangle]
+pub fn _iref_arg(a: &u8) -> i32 {
+    // CHECK: ret i32 4
+    _iref(a)
+}
+
+#[inline]
+pub fn _slice_ref(a: &[u8]) -> i32 {
+    if unsafe { is_val_statically_known(a) } { 7 } else { 6 }
+}
+
+// CHECK-LABEL: @_slice_ref_borrow(
+#[no_mangle]
+pub fn _slice_ref_borrow() -> i32 {
+    // CHECK: ret i32 6
+    _slice_ref(&[0;3])
+}
+
+// CHECK-LABEL: @_slice_ref_arg(
+#[no_mangle]
+pub fn _slice_ref_arg(a: &[u8]) -> i32 {
+    // CHECK: ret i32 6
+    _slice_ref(a)
+}


If it isn't too late, can you add some test cases where the function should return true? Currently, it only returns false for pointers.

Sure 😀

I'm not sure if it would make it into the tree though

Please make a follow-up PR rather than pushing to this one

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#120484 (Avoid ICE when is_val_statically_known is not of a supported type) - rust-lang#120516 (pattern_analysis: cleanup manual impls) - rust-lang#120517 (never patterns: It is correct to lower `!` to `_`.) - rust-lang#120523 (Improve `io::Read::read_buf_exact` error case) - rust-lang#120528 (Store SHOULD_CAPTURE as AtomicU8) - rust-lang#120529 (Update data layouts in custom target tests for LLVM 18) - rust-lang#120530 (Be less confident when `dyn` suggestion is not checked for object safety) - rust-lang#120531 (Remove a bunch of `has_errors` checks that have no meaningful or the wrong effect) - rust-lang#120533 (Correct paths for hexagon-unknown-none-elf platform doc) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#120484 (Avoid ICE when is_val_statically_known is not of a supported type) - rust-lang#120516 (pattern_analysis: cleanup manual impls) - rust-lang#120517 (never patterns: It is correct to lower `!` to `_`.) - rust-lang#120523 (Improve `io::Read::read_buf_exact` error case) - rust-lang#120528 (Store SHOULD_CAPTURE as AtomicU8) - rust-lang#120529 (Update data layouts in custom target tests for LLVM 18) - rust-lang#120531 (Remove a bunch of `has_errors` checks that have no meaningful or the wrong effect) - rust-lang#120533 (Correct paths for hexagon-unknown-none-elf platform doc) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#120484 - Teapot4195:issue-120480-fix, r=compiler-errors Avoid ICE when is_val_statically_known is not of a supported type 2 ICE with 1 stone! 1. Implement `llvm.is.constant.ptr` to avoid first ICE in linked issue. 2. return `false` when the argument is not one of `i*`/`f*`/`ptr` to avoid second ICE. fixes rust-lang#120480

NCGThompson · 2024-02-05T20:33:57Z

You may need to edit documentation as well.

I tried playing around with it, and is_val_statically_known is rarely true. Since memory locations are typically relative, the only way I found to make it return true is by creating a reference from an address:

// CHECK-LABEL: @_iref_addr(
#[no_mangle]
pub fn _iref_addr() -> i32 {
    let my_num_ptr = 16 as *mut u8;
    // CHECK: ret i32 5
    unsafe { _iref(my_num_ptr.as_ref().unwrap()) }
}

So it seems is_val_statically_known(&a) is equivalent to is_val_statically_known(&a as *const _ as usize). Someone may instead expect it to be equivalent too is_val_statically_known(a). This is a raw intrinsic, so there is no expectation that it is user-friendly or intuitive. If someone wants to, they can create a wrapper that does the dereferencing. However, since we explicitly handle pointers, I think it would be prudent to clarify in the documentation.

Avoid ICE when is_val_statically_known is not of a supported type

7bdf705

rustbot assigned TaKO8Ki Jan 30, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 30, 2024

compiler-errors approved these changes Jan 30, 2024

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 30, 2024

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 30, 2024

compiler-errors assigned compiler-errors and unassigned TaKO8Ki Jan 30, 2024

Add additional test cases for is_val_statically_known

a97ff2a

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 Jan 30, 2024

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 30, 2024

matthiaskrgr mentioned this pull request Jan 31, 2024

Rollup of 9 pull requests #120534

Closed

matthiaskrgr mentioned this pull request Jan 31, 2024

Rollup of 9 pull requests #120536

Closed

NCGThompson reviewed Feb 1, 2024

View reviewed changes

matthiaskrgr mentioned this pull request Feb 3, 2024

Rollup of 8 pull requests #120624

Merged

bors merged commit 6f24836 into rust-lang:master Feb 4, 2024
11 checks passed

rustbot added this to the 1.78.0 milestone Feb 4, 2024

This was referenced Feb 14, 2024

ICE when compiling this core change #121064

Closed

Document is_val_statically_known only supports primitives (ICE otherwise) #121115

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid ICE when is_val_statically_known is not of a supported type #120484

Avoid ICE when is_val_statically_known is not of a supported type #120484

Teapot4195 commented Jan 30, 2024

rustbot commented Jan 30, 2024

compiler-errors commented Jan 30, 2024

bors commented Jan 30, 2024

compiler-errors commented Jan 30, 2024

compiler-errors commented Jan 30, 2024

Teapot4195 commented Jan 30, 2024

compiler-errors commented Jan 30, 2024

Teapot4195 commented Jan 30, 2024

compiler-errors commented Jan 30, 2024

bors commented Jan 30, 2024

NCGThompson Feb 1, 2024

Teapot4195 Feb 1, 2024

compiler-errors Feb 1, 2024

NCGThompson commented Feb 5, 2024

Avoid ICE when is_val_statically_known is not of a supported type #120484

Avoid ICE when is_val_statically_known is not of a supported type #120484

Conversation

Teapot4195 commented Jan 30, 2024

rustbot commented Jan 30, 2024

compiler-errors commented Jan 30, 2024

bors commented Jan 30, 2024

compiler-errors commented Jan 30, 2024

compiler-errors commented Jan 30, 2024

Teapot4195 commented Jan 30, 2024

compiler-errors commented Jan 30, 2024

Teapot4195 commented Jan 30, 2024

compiler-errors commented Jan 30, 2024

bors commented Jan 30, 2024

NCGThompson Feb 1, 2024

Choose a reason for hiding this comment

Teapot4195 Feb 1, 2024

Choose a reason for hiding this comment

compiler-errors Feb 1, 2024

Choose a reason for hiding this comment

NCGThompson commented Feb 5, 2024