Skip to content

Commit 2ad7573

Browse files
Fix the bug: Static union values can panic Kani (#4112)
This PR fixes the bug: Static union values can panic Kani. The reason behind the bug is Kani's translation of Union type is not consistent with its layout defined by Rust. Specifically, the size of the Union is not just the max of its fields but that maximum should be round up to a multiple of its alignment. See: https://web.mit.edu/rust-lang_v1.25/arch/amd64_ubuntu1404/share/doc/rust/html/reference/type-layout.html#:~:text=The%20layout%20of%20a%20type,also%20part%20of%20type%20layout In this PR, I add another variant for DataComponent: DataComponent::UnionField, with a padded_type of a Union (a struct which contains a padding and the real field type) to fix the translated layout and support reasoning about transmute function for union type. Resolves #4097 By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.
1 parent 0209e8e commit 2ad7573

File tree

9 files changed

+138
-7
lines changed

9 files changed

+138
-7
lines changed

cprover_bindings/src/goto_program/typ.rs

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ pub enum CIntType {
9999
pub enum DatatypeComponent {
100100
Field { name: InternedString, typ: Type },
101101
Padding { name: InternedString, bits: u64 },
102+
UnionField { name: InternedString, typ: Type, padded_typ: Type },
102103
}
103104

104105
/// The formal parameters of a function.
@@ -120,40 +121,45 @@ impl DatatypeComponent {
120121
pub fn field_typ(&self) -> Option<&Type> {
121122
match self {
122123
Field { typ, .. } => Some(typ),
124+
UnionField { typ, .. } => Some(typ),
123125
Padding { .. } => None,
124126
}
125127
}
126128

127129
pub fn is_field(&self) -> bool {
128130
match self {
129131
Field { .. } => true,
132+
UnionField { .. } => true,
130133
Padding { .. } => false,
131134
}
132135
}
133136

134137
pub fn is_padding(&self) -> bool {
135138
match self {
136139
Field { .. } => false,
140+
UnionField { .. } => false,
137141
Padding { .. } => true,
138142
}
139143
}
140144

141145
pub fn name(&self) -> InternedString {
142146
match self {
143-
Field { name, .. } | Padding { name, .. } => *name,
147+
Field { name, .. } | UnionField { name, .. } | Padding { name, .. } => *name,
144148
}
145149
}
146150

147151
pub fn sizeof_in_bits(&self, st: &SymbolTable) -> u64 {
148152
match self {
149153
Field { typ, .. } => typ.sizeof_in_bits(st),
154+
UnionField { padded_typ, .. } => padded_typ.sizeof_in_bits(st),
150155
Padding { bits, .. } => *bits,
151156
}
152157
}
153158

154159
pub fn typ(&self) -> Type {
155160
match self {
156161
Field { typ, .. } => typ.clone(),
162+
UnionField { typ, .. } => typ.clone(),
157163
Padding { bits, .. } => Type::unsigned_int(*bits),
158164
}
159165
}
@@ -203,6 +209,15 @@ impl DatatypeComponent {
203209
Field { name, typ }
204210
}
205211

212+
pub fn unionfield<T: Into<InternedString>>(name: T, typ: Type, padded_typ: Type) -> Self {
213+
let name = name.into();
214+
assert!(
215+
Self::typecheck_datatype_field(&typ),
216+
"Illegal field.\n\tName: {name}\n\tType: {typ:?}"
217+
);
218+
UnionField { name, typ, padded_typ }
219+
}
220+
206221
pub fn padding<T: Into<InternedString>>(name: T, bits: u64) -> Self {
207222
let name = name.into();
208223
Padding { name, bits }
@@ -384,7 +399,7 @@ impl Type {
384399
StructTag(tag) => st.lookup(*tag).unwrap().typ.sizeof_in_bits(st),
385400
TypeDef { .. } => unreachable!("Expected concrete type."),
386401
Union { components, .. } => {
387-
components.iter().map(|x| x.typ().sizeof_in_bits(st)).max().unwrap_or(0)
402+
components.iter().map(|x| x.sizeof_in_bits(st)).max().unwrap_or(0)
388403
}
389404
UnionTag(tag) => st.lookup(*tag).unwrap().typ.sizeof_in_bits(st),
390405
Unsignedbv { width } => *width,
@@ -809,6 +824,7 @@ impl Type {
809824
match &components[0] {
810825
Padding { .. } => None,
811826
Field { typ, .. } => recurse(typ, st),
827+
UnionField { typ, .. } => recurse(typ, st),
812828
}
813829
} else {
814830
None

cprover_bindings/src/irep/to_irep.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,13 @@ impl ToIrep for DatatypeComponent {
136136
(IrepId::CPrettyName, Irep::just_string_id(name.to_string())),
137137
(IrepId::Type, typ.to_irep(mm)),
138138
]),
139+
DatatypeComponent::UnionField { name, typ: _, padded_typ } => {
140+
Irep::just_named_sub(linear_map![
141+
(IrepId::Name, Irep::just_string_id(name.to_string())),
142+
(IrepId::CPrettyName, Irep::just_string_id(name.to_string())),
143+
(IrepId::Type, padded_typ.to_irep(mm)),
144+
])
145+
}
139146
DatatypeComponent::Padding { name, bits } => Irep::just_named_sub(linear_map![
140147
(IrepId::CIsPadding, Irep::one()),
141148
(IrepId::Name, Irep::just_string_id(name.to_string())),

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

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,11 @@ impl GotocCtx<'_> {
161161
debug_write_type(ctx, typ, out, indent + 2)?;
162162
writeln!(out, ",")?;
163163
}
164+
DatatypeComponent::UnionField { name, typ, .. } => {
165+
write!(out, "{:indent$}{name}: ", "", indent = indent + 2)?;
166+
debug_write_type(ctx, typ, out, indent + 2)?;
167+
writeln!(out, ",")?;
168+
}
164169
DatatypeComponent::Padding { bits, .. } => {
165170
writeln!(
166171
out,
@@ -197,6 +202,11 @@ impl GotocCtx<'_> {
197202
debug_write_type(ctx, typ, out, indent + 2)?;
198203
writeln!(out, ",")?;
199204
}
205+
DatatypeComponent::UnionField { name, typ, .. } => {
206+
write!(out, "{:indent$}{name}: ", "", indent = indent + 2)?;
207+
debug_write_type(ctx, typ, out, indent + 2)?;
208+
writeln!(out, ",")?;
209+
}
200210
DatatypeComponent::Padding { bits, .. } => {
201211
writeln!(
202212
out,
@@ -1195,15 +1205,36 @@ impl<'tcx> GotocCtx<'tcx> {
11951205
def: &'tcx AdtDef,
11961206
subst: &'tcx GenericArgsRef<'tcx>,
11971207
) -> Type {
1208+
let union_size = rustc_internal::stable(ty).layout().unwrap().shape().size.bits();
1209+
let union_name = self.ty_mangled_name(ty);
1210+
let union_pretty_name = self.ty_pretty_name(ty);
11981211
self.ensure_union(self.ty_mangled_name(ty), self.ty_pretty_name(ty), |ctx, _| {
1199-
def.variants().raw[0]
1212+
let fields_info: Vec<(String, Type, u64)> = def.variants().raw[0]
12001213
.fields
12011214
.iter()
12021215
.map(|f| {
1203-
DatatypeComponent::field(
1204-
f.name.to_string(),
1205-
ctx.codegen_ty(f.ty(ctx.tcx, subst)),
1206-
)
1216+
let ty = rustc_internal::stable(f.ty(ctx.tcx, subst));
1217+
let ty_size = ty.layout().unwrap().shape().size.bits();
1218+
let padding_size = union_size - ty_size;
1219+
(f.name.to_string(), ctx.codegen_ty_stable(ty), padding_size as u64)
1220+
})
1221+
.collect();
1222+
fields_info
1223+
.iter()
1224+
.map(|(name, ty, padding)| {
1225+
let struct_name = format!("{union_name}::{name}");
1226+
let pretty_struct_name = format!("{union_pretty_name}::{name}");
1227+
let pad_name = format!("{name}_padding");
1228+
let padded_typ: Type = if *padding == 0 {
1229+
ty.clone()
1230+
} else {
1231+
let pad =
1232+
DatatypeComponent::Padding { name: pad_name.into(), bits: *padding };
1233+
ctx.ensure_struct(struct_name, pretty_struct_name, |_ctx, _| {
1234+
vec![DatatypeComponent::field(name, ty.clone()), pad.clone()]
1235+
})
1236+
};
1237+
DatatypeComponent::unionfield(name, ty.clone(), padded_typ)
12071238
})
12081239
.collect()
12091240
})
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
main.assertion.2\
2+
- Status: SUCCESS\
3+
- Description: "assertion failed: unsafe { FOO.a[1] } == 0"
4+
5+
6+
main.assertion.3\
7+
- Status: SUCCESS\
8+
- Description: "assertion failed: unsafe { BAR.b } == 3"\
9+
10+
VERIFICATION:- SUCCESSFUL

tests/expected/union/static_union.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Copyright Kani Contributors
2+
// SPDX-License-Identifier: Apache-2.0 OR MIT
3+
4+
static FOO: Data = Data { a: [0; 3] };
5+
6+
static BAR: Data = Data { b: 3 };
7+
8+
union Data {
9+
a: [u8; 3],
10+
b: u16,
11+
}
12+
13+
#[kani::proof]
14+
fn main() {
15+
let _x = &FOO;
16+
assert!(unsafe { FOO.a[1] } == 0);
17+
assert!(unsafe { BAR.b } == 3);
18+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
main.assertion.1\
2+
- Status: SUCCESS\
3+
- Description: "index out of bounds: the length is less than or equal to the given index"\
4+
5+
Check 2: main.assertion.2\
6+
- Status: FAILURE\
7+
- Description: "assertion failed: unsafe { FOO.a[1] } == 5"\
8+
9+
Failed Checks: assertion failed: unsafe { FOO.a[1] } == 5
10+
11+
VERIFICATION:- FAILED

tests/expected/union/static_union2.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// Copyright Kani Contributors
2+
// SPDX-License-Identifier: Apache-2.0 OR MIT
3+
4+
static FOO: Data = Data { a: [0; 3] };
5+
6+
union Data {
7+
a: [u8; 3],
8+
b: u16,
9+
}
10+
11+
#[kani::proof]
12+
fn main() {
13+
let _x = &FOO;
14+
assert!(unsafe { FOO.a[1] } == 5);
15+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
main.assertion.1\
2+
- Status: SUCCESS\
3+
- Description: "assertion failed: y == 256"
4+
5+
VERIFICATION:- SUCCESSFUL
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Copyright Kani Contributors
2+
// SPDX-License-Identifier: Apache-2.0 OR MIT
3+
4+
use std::mem::transmute;
5+
6+
static FOO: Data = Data { a: [0, 1, 0] };
7+
8+
#[derive(Copy, Clone)]
9+
union Data {
10+
a: [u8; 3],
11+
b: u16,
12+
}
13+
14+
#[kani::proof]
15+
fn main() {
16+
let y: u32 = unsafe { transmute(FOO) };
17+
assert!(y == 256);
18+
}

0 commit comments

Comments
 (0)