Skip to content

Prefix property descriptions of panics with "Panic:" #4045

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tautschnig
Copy link
Member

This will reduce the ambiguity between cases of Kani reporting a safety (undefined behavior) violation vs Kani demonstrating the possibility of a panic (defined behavior).

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.

This will reduce the ambiguity between cases of Kani reporting a safety
(undefined behavior) violation vs Kani demonstrating the possibility of
a panic (defined behavior).
@tautschnig tautschnig requested a review from a team as a code owner April 25, 2025 12:25
@github-actions github-actions bot added the Z-EndToEndBenchCI Tag a PR to run benchmark CI label Apr 25, 2025
@zhassan-aws
Copy link
Contributor

Is it not possible to rely on the property class for this disambiguation? We've been trying to avoid modifying assertion messages that come from the compiler.

@tautschnig
Copy link
Member Author

Is it not possible to rely on the property class for this disambiguation? We've been trying to avoid modifying assertion messages that come from the compiler.

I guess that would require also printing the property class when reporting results?

@zhassan-aws
Copy link
Contributor

I guess what I'm missing is the target audience of this change. Is it for human consumption? If not, Kani's CBMC property parser processes the property class:

pub fn property_class(&self) -> String {

@tautschnig
Copy link
Member Author

I guess what I'm missing is the target audience of this change.

My current reason for putting forth this change proposal is that we were reviewing results on the standard library and didn't know whether we were seeing reports of undefined behavior or possible panics.

@zhassan-aws
Copy link
Contributor

My current reason for putting forth this change proposal is that we were reviewing results on the standard library and didn't know whether we were seeing reports of undefined behavior or possible panics.

And this cannot be inferred from the first line of a check's output? e.g.:

Check 94: std::ptr::write::<std::mem::MaybeUninit<usize>>.pointer_dereference.5
         - Status: SUCCESS
         - Description: "dereference failure: pointer outside object bounds"
         - Location: ../.rustup/toolchains/nightly-2025-01-28-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:1582:9 in function std::ptr::write::<std::mem::MaybeUninit<usize>>

which shows that this is a pointer_dereference check.

(Sorry for asking too many questions. I just want to make sure this change is warranted.)

@carolynzech carolynzech marked this pull request as draft May 13, 2025 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Z-EndToEndBenchCI Tag a PR to run benchmark CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants