Skip to content

Commit 5efd8b6

Browse files
author
Carolyn Zech
authored
Verify contracts/stubs for generic types with multiple inherent implementations (#3829)
Extends Kani's contracts/stubbing logic to handle simple paths with generic arguments. I'll explain the changes commit-by-commit: ## Commit 690ba97: Error for multiple implementations Prior to this PR, given the code from #3773: ```rust struct NonZero<T>(T); impl NonZero<u32> { #[kani::requires(true)] fn unchecked_mul(self, x: u32) {} } impl NonZero<i32> { #[kani::requires(true)] fn unchecked_mul(self, x: i32) {} } #[kani::proof_for_contract(NonZero::unchecked_mul)] fn verify_unchecked_mul() { let x: NonZero<i32> = NonZero(-1); x.unchecked_mul(-2); } ``` When resolving the target, Kani would return the first `unchecked_mul` implementation that it found for NonZero. If that happens to be the `u32` implementation, then we get an error later that the target `NonZero::unchecked_mul` isn't reachable from the harness, which is confusing because the harness does call `unchecked_mul`. The real problem is that the target needs to specify which implementation it wants to verify, so now we throw this error: ``` Failed to resolve checking function NonZero::unchecked_mul because there are multiple implementations of unchecked_mul in struct `NonZero`. Found: NonZero::<u32>::unchecked_mul NonZero::<i32>::unchecked_mul | | #[kani::proof_for_contract(NonZero::unchecked_mul)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: Replace NonZero::unchecked_mul with a specific implementation. ``` ## Commit 62b66de: Path resolution for simple paths with generic arguments If the user took our suggestion and changed their proof for contract to be `NonZero::<i32>::unchecked_mul`, they would still get the same error. The `syn::Path` is this: ``` Path { leading_colon: None, segments: [ PathSegment { ident: Ident( NonZero, ), arguments: PathArguments::AngleBracketed { colon2_token: Some( PathSep, ), lt_token: Lt, args: [ GenericArgument::Type( Type::Path { qself: None, path: Path { leading_colon: None, segments: [ PathSegment { ident: Ident( i32, ), arguments: PathArguments::None, }, ], }, }, ), ], gt_token: Gt, }, }, PathSep, PathSegment { ident: Ident( unchecked_mul, ), arguments: PathArguments::None, }, ], ``` Kani's path resolution would skip over the `PathArguments` for the base type NonZero, so it would still try to find `NonZero::unchecked_mul` and run into the same problem. So, this commit adds a `base_path_args` field to our `Path` representation to store these arguments, which we use to select the specified implementation. Resolves #3773 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 6fda222 commit 5efd8b6

File tree

9 files changed

+277
-19
lines changed

9 files changed

+277
-19
lines changed

kani-compiler/src/kani_middle/attributes.rs

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -213,13 +213,20 @@ impl<'tcx> KaniAttributes<'tcx> {
213213
let def = self
214214
.resolve_from_mod(name.as_str())
215215
.map_err(|e| {
216-
self.tcx.dcx().span_err(
216+
let mut err = self.tcx.dcx().struct_span_err(
217217
attr.span,
218218
format!(
219219
"Failed to resolve replacement function {}: {e}",
220220
name.as_str()
221221
),
222-
)
222+
);
223+
if let ResolveError::AmbiguousPartialPath { .. } = e {
224+
err = err.with_help(format!(
225+
"Replace {} with a specific implementation.",
226+
name.as_str()
227+
));
228+
}
229+
err.emit();
223230
})
224231
.ok()?;
225232
Some((name, def, attr.span))
@@ -242,13 +249,20 @@ impl<'tcx> KaniAttributes<'tcx> {
242249
self.resolve_from_mod(name.as_str())
243250
.map(|ok| (name, ok, target.span))
244251
.map_err(|resolve_err| {
245-
self.tcx.dcx().span_err(
252+
let mut err = self.tcx.dcx().struct_span_err(
246253
target.span,
247254
format!(
248255
"Failed to resolve checking function {} because {resolve_err}",
249256
name.as_str()
250257
),
251-
)
258+
);
259+
if let ResolveError::AmbiguousPartialPath { .. } = resolve_err {
260+
err = err.with_help(format!(
261+
"Replace {} with a specific implementation.",
262+
name.as_str()
263+
));
264+
}
265+
err.emit();
252266
})
253267
.ok()
254268
})

kani-compiler/src/kani_middle/resolve.rs

Lines changed: 120 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use stable_mir::ty::{FnDef, RigidTy, Ty, TyKind};
2424
use std::collections::HashSet;
2525
use std::fmt;
2626
use std::iter::Peekable;
27-
use syn::{PathSegment, QSelf, TypePath};
27+
use syn::{PathArguments, PathSegment, QSelf, TypePath};
2828
use tracing::{debug, debug_span};
2929

3030
mod type_resolution;
@@ -153,7 +153,7 @@ fn resolve_path<'tcx>(
153153
let next_item = match def_kind {
154154
DefKind::ForeignMod | DefKind::Mod => resolve_in_module(tcx, base, &name),
155155
DefKind::Struct | DefKind::Enum | DefKind::Union => {
156-
resolve_in_type_def(tcx, base, &name)
156+
resolve_in_type_def(tcx, base, &path.base_path_args, &name)
157157
}
158158
DefKind::Trait => resolve_in_trait(tcx, base, &name),
159159
kind => {
@@ -170,6 +170,8 @@ fn resolve_path<'tcx>(
170170
pub enum ResolveError<'tcx> {
171171
/// Ambiguous glob resolution.
172172
AmbiguousGlob { tcx: TyCtxt<'tcx>, name: String, base: DefId, candidates: Vec<DefId> },
173+
/// Ambiguous partial path (multiple inherent impls, c.f. https://github.com/model-checking/kani/issues/3773)
174+
AmbiguousPartialPath { tcx: TyCtxt<'tcx>, name: String, base: DefId, candidates: Vec<DefId> },
173175
/// Use super past the root of a crate.
174176
ExtraSuper,
175177
/// Invalid path.
@@ -208,6 +210,18 @@ impl fmt::Display for ResolveError<'_> {
208210
.collect::<String>()
209211
)
210212
}
213+
ResolveError::AmbiguousPartialPath { tcx, base, name, candidates } => {
214+
let location = description(*tcx, *base);
215+
write!(
216+
f,
217+
"there are multiple implementations of {name} in {location}. Found:\n{}",
218+
candidates
219+
.iter()
220+
.map(|def_id| tcx.def_path_str(*def_id))
221+
.intersperse("\n".to_string())
222+
.collect::<String>()
223+
)
224+
}
211225
ResolveError::InvalidPath { msg } => write!(f, "{msg}"),
212226
ResolveError::UnexpectedType { tcx, item: def_id, expected } => write!(
213227
f,
@@ -232,12 +246,13 @@ impl fmt::Display for ResolveError<'_> {
232246
/// The segments of a path.
233247
type Segments = Vec<PathSegment>;
234248

235-
/// A path consisting of a starting point and a bunch of segments. If `base`
249+
/// A path consisting of a starting point, any PathArguments for that starting point, and a bunch of segments. If `base`
236250
/// matches `Base::LocalModule { id: _, may_be_external_path : true }`, then
237251
/// `segments` cannot be empty.
238252
#[derive(Debug, Hash)]
239253
struct Path {
240254
pub base: DefId,
255+
pub base_path_args: PathArguments,
241256
pub segments: Segments,
242257
}
243258

@@ -271,7 +286,11 @@ fn resolve_prefix<'tcx>(
271286
let next_name = segment.ident.to_string();
272287
let result = resolve_external(tcx, &next_name);
273288
if let Some(def_id) = result {
274-
Ok(Path { base: def_id, segments: segments.cloned().collect() })
289+
Ok(Path {
290+
base: def_id,
291+
base_path_args: segment.arguments.clone(),
292+
segments: segments.cloned().collect(),
293+
})
275294
} else {
276295
Err(ResolveError::MissingItem {
277296
tcx,
@@ -292,7 +311,11 @@ fn resolve_prefix<'tcx>(
292311
None => current_module,
293312
Some((hir_id, _)) => hir_id.owner.def_id,
294313
};
295-
Ok(Path { base: crate_root.to_def_id(), segments: segments.cloned().collect() })
314+
Ok(Path {
315+
base: crate_root.to_def_id(),
316+
base_path_args: segment.arguments.clone(),
317+
segments: segments.cloned().collect(),
318+
})
296319
}
297320
// Path starting with "self::"
298321
(None, Some(segment)) if segment.ident == SELF => {
@@ -320,7 +343,11 @@ fn resolve_prefix<'tcx>(
320343
Err(err)
321344
}
322345
})?;
323-
Ok(Path { base: def_id, segments: segments.cloned().collect() })
346+
Ok(Path {
347+
base: def_id,
348+
base_path_args: segment.arguments.clone(),
349+
segments: segments.cloned().collect(),
350+
})
324351
}
325352
_ => {
326353
unreachable!("Empty path: `{path:?}`")
@@ -350,7 +377,11 @@ where
350377
}
351378
}
352379
debug!("base: {base_module:?}");
353-
Ok(Path { base: base_module.to_def_id(), segments: segments.cloned().collect() })
380+
Ok(Path {
381+
base: base_module.to_def_id(),
382+
base_path_args: PathArguments::None,
383+
segments: segments.cloned().collect(),
384+
})
354385
}
355386

356387
/// Resolves an external crate name.
@@ -529,26 +560,86 @@ where
529560
if segments.next().is_some() {
530561
Err(ResolveError::UnexpectedType { tcx, item: def_id, expected: "module" })
531562
} else {
532-
resolve_in_type_def(tcx, def_id, &name.ident.to_string())
563+
resolve_in_type_def(tcx, def_id, &PathArguments::None, &name.ident.to_string())
533564
}
534565
}
535566

567+
fn generic_args_to_string<T: ToTokens>(args: &T) -> String {
568+
args.to_token_stream().to_string().chars().filter(|c| !c.is_whitespace()).collect::<String>()
569+
}
570+
536571
/// Resolves a function in a type given its `def_id`.
537572
fn resolve_in_type_def<'tcx>(
538573
tcx: TyCtxt<'tcx>,
539574
type_id: DefId,
575+
base_path_args: &PathArguments,
540576
name: &str,
541577
) -> Result<DefId, ResolveError<'tcx>> {
542578
debug!(?name, ?type_id, "resolve_in_type");
543-
let missing_item_err =
544-
|| ResolveError::MissingItem { tcx, base: type_id, unresolved: name.to_string() };
545579
// Try the inherent `impl` blocks (i.e., non-trait `impl`s).
546-
tcx.inherent_impls(type_id)
580+
let candidates: Vec<DefId> = tcx
581+
.inherent_impls(type_id)
547582
.iter()
548583
.flat_map(|impl_id| tcx.associated_item_def_ids(impl_id))
549584
.cloned()
550-
.find(|item| is_item_name(tcx, *item, name))
551-
.ok_or_else(missing_item_err)
585+
.filter(|item| is_item_name(tcx, *item, name))
586+
.collect();
587+
588+
match candidates.len() {
589+
0 => Err(ResolveError::MissingItem { tcx, base: type_id, unresolved: name.to_string() }),
590+
1 => Ok(candidates[0]),
591+
_ => {
592+
let invalid_path_err = |generic_args, candidates: Vec<DefId>| -> ResolveError {
593+
ResolveError::InvalidPath {
594+
msg: format!(
595+
"the generic arguments {} are invalid. The available implementations are: \n{}",
596+
&generic_args,
597+
&candidates
598+
.iter()
599+
.map(|def_id| tcx.def_path_str(def_id))
600+
.intersperse("\n".to_string())
601+
.collect::<String>()
602+
),
603+
}
604+
};
605+
// If there are multiple implementations, we need generic arguments on the base type to refine our options.
606+
match base_path_args {
607+
// If there aren't such arguments, report the ambiguity.
608+
PathArguments::None => Err(ResolveError::AmbiguousPartialPath {
609+
tcx,
610+
name: name.into(),
611+
base: type_id,
612+
candidates,
613+
}),
614+
// Otherwise, use the provided generic arguments to refine our options.
615+
PathArguments::AngleBracketed(args) => {
616+
let generic_args = generic_args_to_string(&args);
617+
let refined_candidates: Vec<DefId> = candidates
618+
.iter()
619+
.cloned()
620+
.filter(|item| {
621+
is_item_name_with_generic_args(tcx, *item, &generic_args, name)
622+
})
623+
.collect();
624+
match refined_candidates.len() {
625+
0 => Err(invalid_path_err(&generic_args, candidates)),
626+
1 => Ok(refined_candidates[0]),
627+
// since is_item_name_with_generic_args looks at the entire item path after the base type, it shouldn't be possible to have more than one match
628+
_ => unreachable!(
629+
"Got multiple refined candidates {:?}",
630+
refined_candidates
631+
.iter()
632+
.map(|def_id| tcx.def_path_str(def_id))
633+
.collect::<Vec<String>>()
634+
),
635+
}
636+
}
637+
PathArguments::Parenthesized(args) => {
638+
Err(invalid_path_err(&generic_args_to_string(args), candidates))
639+
}
640+
}
641+
}
642+
}
552643
}
553644

554645
/// Resolves a function in a trait.
@@ -601,7 +692,11 @@ where
601692
base: ty,
602693
unresolved: name.to_string(),
603694
})?;
604-
Ok(Path { base: item, segments: segments.cloned().collect() })
695+
Ok(Path {
696+
base: item,
697+
base_path_args: PathArguments::None,
698+
segments: segments.cloned().collect(),
699+
})
605700
} else {
606701
Err(ResolveError::InvalidPath { msg: format!("Unexpected primitive type `{ty}`") })
607702
}
@@ -612,3 +707,14 @@ fn is_item_name(tcx: TyCtxt, item: DefId, name: &str) -> bool {
612707
let last = item_path.split("::").last().unwrap();
613708
last == name
614709
}
710+
711+
fn is_item_name_with_generic_args(
712+
tcx: TyCtxt,
713+
item: DefId,
714+
generic_args: &str,
715+
name: &str,
716+
) -> bool {
717+
let item_path = tcx.def_path_str(item);
718+
let all_but_base_type = item_path.find("::").map_or("", |idx| &item_path[idx..]);
719+
all_but_base_type == format!("{}::{}", generic_args, name)
720+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// Copyright Kani Contributors
2+
// SPDX-License-Identifier: Apache-2.0 OR MIT
3+
// kani-flags: -Zfunction-contracts
4+
5+
// Check that Kani can verify contracts on methods where the base type has multiple inherent impls,
6+
// c.f. https://github.com/model-checking/kani/issues/3773
7+
8+
struct NonZero<T>(T);
9+
10+
impl NonZero<u32> {
11+
#[kani::requires(self.0.checked_mul(x).is_some())]
12+
fn unchecked_mul(self, x: u32) -> u32 {
13+
self.0 * x
14+
}
15+
}
16+
17+
impl NonZero<i32> {
18+
#[kani::requires(self.0.checked_mul(x).is_some())]
19+
fn unchecked_mul(self, x: i32) -> i32 {
20+
self.0 * x
21+
}
22+
}
23+
24+
#[kani::proof_for_contract(NonZero::<i32>::unchecked_mul)]
25+
fn verify_unchecked_mul_ambiguous_path() {
26+
let x: NonZero<i32> = NonZero(-1);
27+
x.unchecked_mul(-2);
28+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
Failed to resolve checking function NonZero::<g32>::unchecked_mul because the generic arguments ::<g32> are invalid. The available implementations are:
2+
NonZero::<u32>::unchecked_mul
3+
NonZero::<i32>::unchecked_mul
4+
|
5+
| #[kani::proof_for_contract(NonZero::<g32>::unchecked_mul)]
6+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
7+
8+
Failed to resolve checking function NonZero::unchecked_mul because there are multiple implementations of unchecked_mul in struct `NonZero`. Found:
9+
NonZero::<u32>::unchecked_mul
10+
NonZero::<i32>::unchecked_mul
11+
|
12+
| #[kani::proof_for_contract(NonZero::unchecked_mul)]
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
14+
|
15+
= help: Replace NonZero::unchecked_mul with a specific implementation.
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Copyright Kani Contributors
2+
// SPDX-License-Identifier: Apache-2.0 OR MIT
3+
// kani-flags: -Zfunction-contracts
4+
5+
// Test that Kani errors if the target of the proof_for_contract is missing a generic argument that's required to disambiguate between multiple implementations
6+
// or if the generic argument is invalid.
7+
// c.f. https://github.com/model-checking/kani/issues/3773
8+
9+
struct NonZero<T>(T);
10+
11+
impl NonZero<u32> {
12+
#[kani::requires(self.0.checked_mul(x).is_some())]
13+
fn unchecked_mul(self, x: u32) -> u32 {
14+
self.0 * x
15+
}
16+
}
17+
18+
impl NonZero<i32> {
19+
#[kani::requires(self.0.checked_mul(x).is_some())]
20+
fn unchecked_mul(self, x: i32) -> i32 {
21+
self.0 * x
22+
}
23+
}
24+
25+
// Errors because there is more than one unchecked_mul implementation
26+
#[kani::proof_for_contract(NonZero::unchecked_mul)]
27+
fn verify_unchecked_mul_ambiguous_path() {
28+
let x: NonZero<i32> = NonZero(-1);
29+
x.unchecked_mul(-2);
30+
}
31+
32+
// Errors because the g32 implementation doesn't exist
33+
#[kani::proof_for_contract(NonZero::<g32>::unchecked_mul)]
34+
fn verify_unchecked_mul_invalid_impl() {
35+
let x: NonZero<i32> = NonZero(-1);
36+
NonZero::<i32>::unchecked_mul(x, -2);
37+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
error: Failed to resolve replacement function NonZero::unchecked_mul: there are multiple implementations of unchecked_mul in struct `NonZero`. Found:
2+
NonZero::<u32>::unchecked_mul
3+
NonZero::<i32>::unchecked_mul
4+
invalid_inherent_impls.rs
5+
|
6+
| #[kani::stub_verified(NonZero::unchecked_mul)]
7+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
8+
|
9+
= help: Replace NonZero::unchecked_mul with a specific implementation.
10+
11+
error: Failed to resolve replacement function NonZero::<g32>::unchecked_mul: the generic arguments ::<g32> are invalid. The available implementations are:
12+
NonZero::<u32>::unchecked_mul
13+
NonZero::<i32>::unchecked_mul
14+
invalid_inherent_impls.rs
15+
|
16+
| #[kani::stub_verified(NonZero::<g32>::unchecked_mul)]
17+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
18+
|

0 commit comments

Comments
 (0)