Rollup merge of rust-lang#94270 - RalfJung:fn-ptrs, r=oli-obk

matthiaskrgr · web-flow · commit 3cd1dc1d6ec8 · 2022-02-24T07:48:06.000+01:00
Miri: relax fn ptr check As discussed in rust-lang/unsafe-code-guidelines#72 (comment), the function pointer check done by Miri is currently overeager: contrary to our usual principle of only checking rather uncontroversial validity invariants, we actually check that the pointer points to a real function. So, this relaxes the check to what the validity invariant probably will be (and what the reference already says it is): the function pointer must be non-null, and that's it. The check that CTFE does on the final value of a constant is unchanged -- CTFE recurses through references, so it makes some sense to also recurse through function pointers. We might still want to relax this in the future, but that would be a separate change. r? `@oli-obk`
diff --git a/compiler/rustc_const_eval/src/interpret/validity.rs b/compiler/rustc_const_eval/src/interpret/validity.rs
@@ -567,22 +567,27 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
             }
             ty::FnPtr(_sig) => {
                 let value = try_validation!(
-                    self.ecx.read_immediate(value),
+                    self.ecx.read_scalar(value).and_then(|v| v.check_init()),
                     self.path,
                     err_unsup!(ReadPointerAsBytes) => { "part of a pointer" } expected { "a proper pointer or integer value" },
+                    err_ub!(InvalidUninitBytes(None)) => { "uninitialized bytes" } expected { "a proper pointer or integer value" },
                 );
-                // Make sure we print a `ScalarMaybeUninit` (and not an `ImmTy`) in the error
-                // message below.
-                let value = value.to_scalar_or_uninit();
-                let _fn = try_validation!(
-                    value.check_init().and_then(|ptr| self.ecx.memory.get_fn(self.ecx.scalar_to_ptr(ptr))),
-                    self.path,
-                    err_ub!(DanglingIntPointer(..)) |
-                    err_ub!(InvalidFunctionPointer(..)) |
-                    err_ub!(InvalidUninitBytes(None)) =>
-                        { "{:x}", value } expected { "a function pointer" },
-                );
-                // FIXME: Check if the signature matches
+                let ptr = self.ecx.scalar_to_ptr(value);
+                // Ensure the pointer is non-null.
+                if self.ecx.memory.ptr_may_be_null(ptr) {
+                    throw_validation_failure!(self.path, { "a potentially null function pointer" });
+                }
+                // If we check references recursively, also check that this points to a function.
+                if let Some(_) = self.ref_tracking {
+                    let _fn = try_validation!(
+                        self.ecx.memory.get_fn(ptr),
+                        self.path,
+                        err_ub!(DanglingIntPointer(..)) |
+                        err_ub!(InvalidFunctionPointer(..)) =>
+                            { "{:x}", value } expected { "a function pointer" },
+                    );
+                    // FIXME: Check if the signature matches
+                }
                 Ok(true)
             }
             ty::Never => throw_validation_failure!(self.path, { "a value of the never type `!`" }),
diff --git a/src/test/ui/consts/const-eval/ub-ref-ptr.32bit.stderr b/src/test/ui/consts/const-eval/ub-ref-ptr.32bit.stderr
@@ -43,7 +43,7 @@ LL | const NULL_BOX: Box<u16> = unsafe { mem::transmute(0usize) };
            }
 
 error[E0080]: it is undefined behavior to use this value
-  --> $DIR/ub-ref-ptr.rs:30:1
+  --> $DIR/ub-ref-ptr.rs:31:1
    |
 LL | const REF_AS_USIZE: usize = unsafe { mem::transmute(&0) };
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc15, but expected initialized plain (non-pointer) bytes
@@ -54,7 +54,7 @@ LL | const REF_AS_USIZE: usize = unsafe { mem::transmute(&0) };
            }
 
 error[E0080]: it is undefined behavior to use this value
-  --> $DIR/ub-ref-ptr.rs:33:1
+  --> $DIR/ub-ref-ptr.rs:34:1
    |
 LL | const REF_AS_USIZE_SLICE: &[usize] = &[unsafe { mem::transmute(&0) }];
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed at .<deref>: encountered a pointer, but expected plain (non-pointer) bytes
@@ -65,7 +65,7 @@ LL | const REF_AS_USIZE_SLICE: &[usize] = &[unsafe { mem::transmute(&0) }];
            }
 
 error[E0080]: it is undefined behavior to use this value
-  --> $DIR/ub-ref-ptr.rs:36:1
+  --> $DIR/ub-ref-ptr.rs:37:1
    |
 LL | const REF_AS_USIZE_BOX_SLICE: Box<[usize]> = unsafe { mem::transmute::<&[usize], _>(&[mem::transmute(&0)]) };
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed at .<deref>: encountered a pointer, but expected plain (non-pointer) bytes
@@ -76,7 +76,7 @@ LL | const REF_AS_USIZE_BOX_SLICE: Box<[usize]> = unsafe { mem::transmute::<&[us
            }
 
 error[E0080]: it is undefined behavior to use this value
-  --> $DIR/ub-ref-ptr.rs:39:1
+  --> $DIR/ub-ref-ptr.rs:40:1
    |
 LL | const USIZE_AS_REF: &'static u8 = unsafe { mem::transmute(1337usize) };
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a dangling reference (address 0x539 is unallocated)
@@ -87,7 +87,7 @@ LL | const USIZE_AS_REF: &'static u8 = unsafe { mem::transmute(1337usize) };
            }
 
 error[E0080]: it is undefined behavior to use this value
-  --> $DIR/ub-ref-ptr.rs:42:1
+  --> $DIR/ub-ref-ptr.rs:43:1
    |
 LL | const USIZE_AS_BOX: Box<u8> = unsafe { mem::transmute(1337usize) };
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a dangling box (address 0x539 is unallocated)
@@ -98,7 +98,7 @@ LL | const USIZE_AS_BOX: Box<u8> = unsafe { mem::transmute(1337usize) };
            }
 
 error[E0080]: it is undefined behavior to use this value
-  --> $DIR/ub-ref-ptr.rs:45:1
+  --> $DIR/ub-ref-ptr.rs:46:1
    |
 LL | const UNINIT_PTR: *const i32 = unsafe { MaybeUninit { uninit: () }.init };
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized raw pointer
@@ -109,16 +109,49 @@ LL | const UNINIT_PTR: *const i32 = unsafe { MaybeUninit { uninit: () }.init };
            }
 
 error[E0080]: it is undefined behavior to use this value
-  --> $DIR/ub-ref-ptr.rs:47:1
+  --> $DIR/ub-ref-ptr.rs:49:1
+   |
+LL | const NULL_FN_PTR: fn() = unsafe { mem::transmute(0usize) };
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a potentially null function pointer
+   |
+   = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
+   = note: the raw bytes of the constant (size: 4, align: 4) {
+               00 00 00 00                                     │ ....
+           }
+
+error[E0080]: it is undefined behavior to use this value
+  --> $DIR/ub-ref-ptr.rs:51:1
    |
 LL | const UNINIT_FN_PTR: fn() = unsafe { MaybeUninit { uninit: () }.init };
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected a function pointer
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected a proper pointer or integer value
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
    = note: the raw bytes of the constant (size: 4, align: 4) {
                __ __ __ __                                     │ ░░░░
            }
 
-error: aborting due to 11 previous errors
+error[E0080]: it is undefined behavior to use this value
+  --> $DIR/ub-ref-ptr.rs:53:1
+   |
+LL | const DANGLING_FN_PTR: fn() = unsafe { mem::transmute(13usize) };
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0x0000000d, but expected a function pointer
+   |
+   = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
+   = note: the raw bytes of the constant (size: 4, align: 4) {
+               0d 00 00 00                                     │ ....
+           }
+
+error[E0080]: it is undefined behavior to use this value
+  --> $DIR/ub-ref-ptr.rs:55:1
+   |
+LL | const DATA_FN_PTR: fn() = unsafe { mem::transmute(&13) };
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc41, but expected a function pointer
+   |
+   = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
+   = note: the raw bytes of the constant (size: 4, align: 4) {
+               ╾─alloc41─╼                                     │ ╾──╼
+           }
+
+error: aborting due to 14 previous errors
 
 For more information about this error, try `rustc --explain E0080`.
diff --git a/src/test/ui/consts/const-eval/ub-ref-ptr.64bit.stderr b/src/test/ui/consts/const-eval/ub-ref-ptr.64bit.stderr
@@ -43,7 +43,7 @@ LL | const NULL_BOX: Box<u16> = unsafe { mem::transmute(0usize) };
            }
 
 error[E0080]: it is undefined behavior to use this value
-  --> $DIR/ub-ref-ptr.rs:30:1
+  --> $DIR/ub-ref-ptr.rs:31:1
    |
 LL | const REF_AS_USIZE: usize = unsafe { mem::transmute(&0) };
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc15, but expected initialized plain (non-pointer) bytes
@@ -54,7 +54,7 @@ LL | const REF_AS_USIZE: usize = unsafe { mem::transmute(&0) };
            }
 
 error[E0080]: it is undefined behavior to use this value
-  --> $DIR/ub-ref-ptr.rs:33:1
+  --> $DIR/ub-ref-ptr.rs:34:1
    |
 LL | const REF_AS_USIZE_SLICE: &[usize] = &[unsafe { mem::transmute(&0) }];
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed at .<deref>: encountered a pointer, but expected plain (non-pointer) bytes
@@ -65,7 +65,7 @@ LL | const REF_AS_USIZE_SLICE: &[usize] = &[unsafe { mem::transmute(&0) }];
            }
 
 error[E0080]: it is undefined behavior to use this value
-  --> $DIR/ub-ref-ptr.rs:36:1
+  --> $DIR/ub-ref-ptr.rs:37:1
    |
 LL | const REF_AS_USIZE_BOX_SLICE: Box<[usize]> = unsafe { mem::transmute::<&[usize], _>(&[mem::transmute(&0)]) };
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed at .<deref>: encountered a pointer, but expected plain (non-pointer) bytes
@@ -76,7 +76,7 @@ LL | const REF_AS_USIZE_BOX_SLICE: Box<[usize]> = unsafe { mem::transmute::<&[us
            }
 
 error[E0080]: it is undefined behavior to use this value
-  --> $DIR/ub-ref-ptr.rs:39:1
+  --> $DIR/ub-ref-ptr.rs:40:1
    |
 LL | const USIZE_AS_REF: &'static u8 = unsafe { mem::transmute(1337usize) };
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a dangling reference (address 0x539 is unallocated)
@@ -87,7 +87,7 @@ LL | const USIZE_AS_REF: &'static u8 = unsafe { mem::transmute(1337usize) };
            }
 
 error[E0080]: it is undefined behavior to use this value
-  --> $DIR/ub-ref-ptr.rs:42:1
+  --> $DIR/ub-ref-ptr.rs:43:1
    |
 LL | const USIZE_AS_BOX: Box<u8> = unsafe { mem::transmute(1337usize) };
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a dangling box (address 0x539 is unallocated)
@@ -98,7 +98,7 @@ LL | const USIZE_AS_BOX: Box<u8> = unsafe { mem::transmute(1337usize) };
            }
 
 error[E0080]: it is undefined behavior to use this value
-  --> $DIR/ub-ref-ptr.rs:45:1
+  --> $DIR/ub-ref-ptr.rs:46:1
    |
 LL | const UNINIT_PTR: *const i32 = unsafe { MaybeUninit { uninit: () }.init };
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized raw pointer
@@ -109,16 +109,49 @@ LL | const UNINIT_PTR: *const i32 = unsafe { MaybeUninit { uninit: () }.init };
            }
 
 error[E0080]: it is undefined behavior to use this value
-  --> $DIR/ub-ref-ptr.rs:47:1
+  --> $DIR/ub-ref-ptr.rs:49:1
+   |
+LL | const NULL_FN_PTR: fn() = unsafe { mem::transmute(0usize) };
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a potentially null function pointer
+   |
+   = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
+   = note: the raw bytes of the constant (size: 8, align: 8) {
+               00 00 00 00 00 00 00 00                         │ ........
+           }
+
+error[E0080]: it is undefined behavior to use this value
+  --> $DIR/ub-ref-ptr.rs:51:1
    |
 LL | const UNINIT_FN_PTR: fn() = unsafe { MaybeUninit { uninit: () }.init };
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected a function pointer
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected a proper pointer or integer value
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
    = note: the raw bytes of the constant (size: 8, align: 8) {
                __ __ __ __ __ __ __ __                         │ ░░░░░░░░
            }
 
-error: aborting due to 11 previous errors
+error[E0080]: it is undefined behavior to use this value
+  --> $DIR/ub-ref-ptr.rs:53:1
+   |
+LL | const DANGLING_FN_PTR: fn() = unsafe { mem::transmute(13usize) };
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0x000000000000000d, but expected a function pointer
+   |
+   = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
+   = note: the raw bytes of the constant (size: 8, align: 8) {
+               0d 00 00 00 00 00 00 00                         │ ........
+           }
+
+error[E0080]: it is undefined behavior to use this value
+  --> $DIR/ub-ref-ptr.rs:55:1
+   |
+LL | const DATA_FN_PTR: fn() = unsafe { mem::transmute(&13) };
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc41, but expected a function pointer
+   |
+   = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
+   = note: the raw bytes of the constant (size: 8, align: 8) {
+               ╾───────alloc41───────╼                         │ ╾──────╼
+           }
+
+error: aborting due to 14 previous errors
 
 For more information about this error, try `rustc --explain E0080`.
diff --git a/src/test/ui/consts/const-eval/ub-ref-ptr.rs b/src/test/ui/consts/const-eval/ub-ref-ptr.rs
@@ -24,6 +24,7 @@ const NULL: &u16 = unsafe { mem::transmute(0usize) };
 const NULL_BOX: Box<u16> = unsafe { mem::transmute(0usize) };
 //~^ ERROR it is undefined behavior to use this value
 
+
 // It is very important that we reject this: We do promote `&(4 * REF_AS_USIZE)`,
 // but that would fail to compile; so we ended up breaking user code that would
 // have worked fine had we not promoted.
@@ -44,7 +45,14 @@ const USIZE_AS_BOX: Box<u8> = unsafe { mem::transmute(1337usize) };
 
 const UNINIT_PTR: *const i32 = unsafe { MaybeUninit { uninit: () }.init };
 //~^ ERROR it is undefined behavior to use this value
+
+const NULL_FN_PTR: fn() = unsafe { mem::transmute(0usize) };
+//~^ ERROR it is undefined behavior to use this value
 const UNINIT_FN_PTR: fn() = unsafe { MaybeUninit { uninit: () }.init };
 //~^ ERROR it is undefined behavior to use this value
+const DANGLING_FN_PTR: fn() = unsafe { mem::transmute(13usize) };
+//~^ ERROR it is undefined behavior to use this value
+const DATA_FN_PTR: fn() = unsafe { mem::transmute(&13) };
+//~^ ERROR it is undefined behavior to use this value
 
 fn main() {}
