Skip to content

Commit fade705

Browse files
vonakaFedor Ryabinintautschnig
authored
Ensuring that MIR constants are marked as static consts (#4233)
Currently, we rely on the `mutability` of the `Allocation` when marking constants. However, my understanding is that for various internal reasons the compiler may use mutable allocations even for constants. Instead, we should probably track whether a variable is constant ourselves. Resolves #3905 Resolves #4029 Also fixes `tests/expected/function-contract/valid_ptr.expected` (see comments below). By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses. --------- Co-authored-by: Fedor Ryabinin <[email protected]> Co-authored-by: Michael Tautschnig <[email protected]>
1 parent dd541be commit fade705

File tree

6 files changed

+97
-9
lines changed

6 files changed

+97
-9
lines changed

kani-compiler/src/codegen_cprover_gotoc/codegen/operand.rs

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ impl<'tcx> GotocCtx<'tcx> {
131131
// First try to generate the constant without allocating memory.
132132
let expr = self.try_codegen_constant(alloc, ty, loc).unwrap_or_else(|| {
133133
debug!("codegen_allocation try_fail");
134-
let mem_var = self.codegen_const_allocation(alloc, None, loc);
134+
let mem_var = self.codegen_const_allocation(alloc, None, loc, true);
135135
mem_var
136136
.cast_to(Type::unsigned_int(8).to_pointer())
137137
.cast_to(self.codegen_ty_stable(ty).to_pointer())
@@ -282,7 +282,7 @@ impl<'tcx> GotocCtx<'tcx> {
282282
let GlobalAlloc::Memory(data) = GlobalAlloc::from(alloc_id) else {
283283
unreachable!()
284284
};
285-
let mem_var = self.codegen_const_allocation(&data, None, loc);
285+
let mem_var = self.codegen_const_allocation(&data, None, loc, false);
286286

287287
// Extract identifier for static variable.
288288
// codegen_allocation_auto_imm_name returns the *address* of
@@ -323,7 +323,7 @@ impl<'tcx> GotocCtx<'tcx> {
323323
let GlobalAlloc::Memory(data) = GlobalAlloc::from(alloc_id) else {
324324
unreachable!()
325325
};
326-
let mem_var = self.codegen_const_allocation(&data, None, loc);
326+
let mem_var = self.codegen_const_allocation(&data, None, loc, false);
327327
let inner_typ = self.codegen_ty_stable(inner_ty);
328328
let len = data.bytes.len() / inner_typ.sizeof(&self.symbol_table) as usize;
329329
let data_expr = mem_var.cast_to(inner_typ.to_pointer());
@@ -394,7 +394,7 @@ impl<'tcx> GotocCtx<'tcx> {
394394
// crates do not conflict. The name alone is insufficient because Rust
395395
// allows different versions of the same crate to be used.
396396
let name = format!("{}::{alloc_id:?}", self.full_crate_name());
397-
self.codegen_const_allocation(&alloc, Some(name), loc)
397+
self.codegen_const_allocation(&alloc, Some(name), loc, false)
398398
}
399399
alloc @ GlobalAlloc::VTable(..) => {
400400
// This is similar to GlobalAlloc::Memory but the type is opaque to rust and it
@@ -404,7 +404,7 @@ impl<'tcx> GotocCtx<'tcx> {
404404
unreachable!()
405405
};
406406
let name = format!("{}::{alloc_id:?}", self.full_crate_name());
407-
self.codegen_const_allocation(&alloc, Some(name), loc)
407+
self.codegen_const_allocation(&alloc, Some(name), loc, false)
408408
}
409409
GlobalAlloc::TypeId { ty: _ } => todo!(),
410410
};
@@ -486,6 +486,7 @@ impl<'tcx> GotocCtx<'tcx> {
486486
alloc: &Allocation,
487487
name: Option<String>,
488488
loc: Location,
489+
is_definitely_const: bool,
489490
) -> Expr {
490491
debug!(?name, ?alloc, "codegen_const_allocation");
491492
let alloc_name = match self.alloc_map.get(alloc) {
@@ -497,6 +498,7 @@ impl<'tcx> GotocCtx<'tcx> {
497498
alloc_name.clone(),
498499
loc,
499500
has_interior_mutabity,
501+
is_definitely_const,
500502
);
501503
alloc_name
502504
}
@@ -517,7 +519,7 @@ impl<'tcx> GotocCtx<'tcx> {
517519
// The memory behind this allocation isn't constant, but codegen_alloc_in_memory (which codegen_const_allocation calls)
518520
// uses alloc's mutability field to set the const-ness of the allocation in CBMC's symbol table,
519521
// so we can reuse the code and without worrying that the allocation is set as immutable.
520-
self.codegen_const_allocation(alloc, name, loc)
522+
self.codegen_const_allocation(alloc, name, loc, false)
521523
}
522524

523525
/// Insert an allocation into the goto symbol table, and generate an init value.
@@ -530,6 +532,7 @@ impl<'tcx> GotocCtx<'tcx> {
530532
name: String,
531533
loc: Location,
532534
has_interior_mutabity: bool,
535+
is_definitely_const: bool,
533536
) {
534537
debug!(?name, ?alloc, "codegen_alloc_in_memory");
535538
let struct_name = &format!("{name}::struct");
@@ -583,7 +586,7 @@ impl<'tcx> GotocCtx<'tcx> {
583586
let _var = self.ensure_global_var_init(
584587
&name,
585588
false, //TODO is this correct?
586-
alloc.mutability == Mutability::Not && !has_interior_mutabity,
589+
(is_definitely_const || alloc.mutability == Mutability::Not) && !has_interior_mutabity,
587590
alloc_typ_ref.clone(),
588591
loc,
589592
init_fn,

kani-compiler/src/codegen_cprover_gotoc/codegen/static_var.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ impl GotocCtx<'_> {
2424
symbol_name,
2525
self.codegen_span_stable(def.span()),
2626
is_interior_mut(self.tcx, def.ty()),
27+
false,
2728
);
2829
}
2930

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
assertion\
2+
- Status: SUCCESS\
3+
- Description: "|result| *result == Enum::Second"\
4+
5+
assertion\
6+
- Status: SUCCESS\
7+
- Description: "|result| *result == Enum::First"\
8+
9+
VERIFICATION:- SUCCESSFUL
10+
11+
Complete - 5 successfully verified harnesses, 0 failures, 5 total.
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
// Copyright Kani Contributors
2+
// SPDX-License-Identifier: Apache-2.0 OR MIT
3+
// kani-flags: -Z function-contracts
4+
5+
//! Checking that constant blocks are correctly verified in contracts.
6+
//! See https://github.com/model-checking/kani/issues/3905
7+
8+
#![feature(ptr_alignment_type)]
9+
10+
use std::mem;
11+
use std::ptr::{Alignment, NonNull};
12+
13+
#[derive(PartialEq)]
14+
enum Enum {
15+
First,
16+
Second,
17+
}
18+
19+
#[kani::ensures(|result| *result == Enum::First)]
20+
const fn first() -> Enum {
21+
const { Enum::First }
22+
}
23+
24+
#[kani::ensures(|result| *result == Enum::Second)]
25+
const fn second() -> Enum {
26+
Enum::Second
27+
}
28+
29+
#[kani::proof_for_contract(first)]
30+
pub fn check_first() {
31+
let _ = first();
32+
}
33+
34+
#[kani::proof_for_contract(second)]
35+
pub fn check_second() {
36+
let _ = second();
37+
}
38+
39+
#[kani::ensures(|result| result.as_usize().is_power_of_two())]
40+
pub const fn Align_of<T>() -> Alignment {
41+
// This can't actually panic since type alignment is always a power of two.
42+
const { Alignment::new(mem::align_of::<T>()).unwrap() }
43+
}
44+
45+
#[kani::ensures(|result| result.as_usize().is_power_of_two())]
46+
pub const fn Align_of_no_const<T>() -> Alignment {
47+
// This can't actually panic since type alignment is always a power of two.
48+
Alignment::new(mem::align_of::<T>()).unwrap()
49+
}
50+
51+
#[kani::proof_for_contract(Align_of)]
52+
pub fn check_of_i32() {
53+
let _ = Align_of::<i32>();
54+
}
55+
56+
#[kani::proof_for_contract(Align_of_no_const)]
57+
pub fn check_of_i32_no_const() {
58+
let _ = Align_of_no_const::<i32>();
59+
}
60+
61+
#[kani::requires(true)]
62+
pub const unsafe fn byte_add_n<T>(s: NonNull<T>, count: usize) -> NonNull<T> {
63+
unsafe { NonNull::new_unchecked(s.as_ptr().byte_add(count)) }
64+
}
65+
66+
#[kani::proof_for_contract(byte_add_n)]
67+
pub fn non_null_byte_add_dangling_proof() {
68+
let ptr = NonNull::<i32>::dangling();
69+
assert!(ptr.as_ptr().addr() == 4);
70+
assert!(ptr.as_ptr().addr() <= (isize::MAX as usize));
71+
unsafe {
72+
let _ = byte_add_n(ptr, 0);
73+
}
74+
}

tests/expected/function-contract/valid_ptr.expected

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,7 @@ Failed Checks: |result| kani::mem::can_dereference(result.0)
44
VERIFICATION:- FAILED
55

66
Checking harness pre_condition::harness_invalid_ptr...
7-
Failed Checks: assertion failed: unsafe { read_ptr(ptr) } == -20
87
Failed Checks: Kani does not support reasoning about pointer to unallocated memory
9-
Failed Checks: dereference failure: pointer outside object bounds
108
VERIFICATION:- FAILED
119

1210
Checking harness pre_condition::harness_stack_ptr...

tests/kani/FunctionContracts/modify_slice_elem_fixme.rs renamed to tests/kani/FunctionContracts/modify_slice_elem.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
//!
55
//! This started failing with the 2025-04-05 toolchain upgrade
66
//! Tracking issue: https://github.com/model-checking/kani/issues/4029
7+
//! Was fixed with https://github.com/model-checking/kani/pull/4233
78
//!
89
//! Note that this test used to crash while parsing the annotations.
910
// kani-flags: -Zfunction-contracts

0 commit comments

Comments
 (0)