Skip to content

Silent transformation of __CPROVER_{r,w,rw}_ok causing surprising behaviour #7377

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

Closed
tautschnig opened this issue Nov 23, 2022 · 4 comments · Fixed by #7395
Closed

Silent transformation of __CPROVER_{r,w,rw}_ok causing surprising behaviour #7377

tautschnig opened this issue Nov 23, 2022 · 4 comments · Fixed by #7395
Assignees
Labels
aws Bugs or features of importance to AWS CBMC users aws-high pending merge

Comments

@tautschnig
Copy link
Collaborator

Posting an analysis by @nwetzler, @jimgrundy and @remi-delmas-3000 The attached a gzipped tarball contains a single C file and a script to demonstrate the problem.

The following function takes a single pointer and asserts that it is read/write okay:

void foo (char* x) {
    __CPROVER_assert(__CPROVER_rw_ok(x, 1), "");
    return;
}

We can compile this to goto (to create a.out) and then run goto-instrument on the resulting binary to create a new binary (which is b.out). Off the bat, you’ll notice that the size of these two binaries is different. However, if you show-goto-functions or program-only (for the SSA) and compare the two, they will be identical.

But if you now enable pointer-check and pointer-primitive-check, you’ll see that they produce different show-goto-functions and program-only output. Furthermore, we think the assertions produced by the checks are even semantically different between the two, which seems like a bug in CBMC.

83c83
<         ASSERT pointer_object(NULL) = pointer_object(foo::x) ∨ ¬(is_invalid_pointer(foo::x)) // pointer invalid in RW_OK(x, (unsigned long int)1)
---
>         ASSERT (¬(pointer_object(foo::x) = pointer_object(NULL)) ∧ ¬(is_invalid_pointer(foo::x)) ∧ ¬(pointer_object(foo::x) = pointer_object(__CPROVER_deallocated)) ∧ ¬(pointer_object(foo::x) = pointer_object(__CPROVER_dead_object)) ∧ pointer_offset(foo::x) ≥ 0) ⇒ (pointer_object(NULL) = pointer_object(foo::x) ∨ ¬(is_invalid_pointer(foo::x))) // pointer invalid in OBJECT_SIZE(x)

Several issues appear to arise:

  1. There isn't actually a way to just dump the contents of a GOTO binary for goto-instrument will always call into instrumentation steps.
  2. Just like you cannot just dump a GOTO binary, running goto-instrument a.out b.out will perform transformations despite no such transformation being requested.
  3. A particular example of such a transformation (maybe the only one, maybe there are others) is the expansion of __CPROVER_{r,w,rw}_ok that goto_check_ct does (irrespective of whether a property instrumentation was requested).
  4. goto_check_ct is not idempotent: doing one round of goto_check_ct followed by another with --pointer-primitive-check enabled is not the same as doing just one round of goto_check_ct with --pointer-primitive-check enabled.
@tautschnig tautschnig added aws Bugs or features of importance to AWS CBMC users aws-high labels Nov 23, 2022
@tautschnig tautschnig self-assigned this Nov 23, 2022
@jimgrundy
Copy link
Collaborator

There is some urgency to this one as it looks like the extra complication introduced into the assertion is part of a performance problem we are seeing (in a more complex example, of course). Removing this complication is important.

@remi-delmas-3000
Copy link
Collaborator

the related issue is decribed here : #7378

@tautschnig
Copy link
Collaborator Author

I started to work on the issues noticed in here (as well as in #7378). #7395 should be part of the puzzle (once complete). I am still trying to figure out, however, what is really causing this performance problem (the issues listed in this ticket are to be addressed, but don't necessarily explain why performance should be so much worse).

@jimgrundy
Copy link
Collaborator

Can we get a status update?

tautschnig added a commit to tautschnig/cbmc that referenced this issue Dec 9, 2022
These expressions are now handled by the back-ends. Unconditional
rewrites by goto_check_ct (even when no checks are enabled) are no
longer necessary.

Fixes: diffblue#7377
@tautschnig tautschnig removed their assignment Dec 9, 2022
tautschnig added a commit to tautschnig/cbmc that referenced this issue Dec 20, 2022
These expressions are now handled by the back-ends. Unconditional
rewrites by goto_check_ct (even when no checks are enabled) are no
longer necessary.

Fixes: diffblue#7377
tautschnig added a commit to tautschnig/cbmc that referenced this issue Dec 20, 2022
These expressions are now handled by the back-ends. Unconditional
rewrites by goto_check_ct (even when no checks are enabled) are no
longer necessary.

Fixes: diffblue#7377
@tautschnig tautschnig self-assigned this Mar 13, 2023
tautschnig added a commit to tautschnig/cbmc that referenced this issue May 19, 2023
These expressions are now handled by the back-ends. Unconditional
rewrites by goto_check_ct (even when no checks are enabled) are no
longer necessary.

Fixes: diffblue#7377
tautschnig added a commit to tautschnig/cbmc that referenced this issue May 19, 2023
These expressions are now handled by the back-ends. Unconditional
rewrites by goto_check_ct (even when no checks are enabled) are no
longer necessary.

Fixes: diffblue#7377
tautschnig added a commit to tautschnig/cbmc that referenced this issue May 22, 2023
These expressions are now rewritten to their prophecy_* counterparts by
goto-program conversion, and back-ends directly handle these prophecy_*
expressions. Unconditional rewrites by goto_check_ct (even when no
checks are enabled) are no longer necessary.

Fixes: diffblue#7377
tautschnig added a commit to tautschnig/cbmc that referenced this issue May 23, 2023
These expressions are now rewritten to their prophecy_* counterparts by
goto-program conversion, and back-ends directly handle these prophecy_*
expressions. Unconditional rewrites by goto_check_ct (even when no
checks are enabled) are no longer necessary.

Fixes: diffblue#7377
tautschnig added a commit to tautschnig/cbmc that referenced this issue May 29, 2023
These expressions are now rewritten to their prophecy_* counterparts by
goto-program conversion, and back-ends directly handle these prophecy_*
expressions. Unconditional rewrites by goto_check_ct (even when no
checks are enabled) are no longer necessary.

Fixes: diffblue#7377
tautschnig added a commit to tautschnig/cbmc that referenced this issue May 29, 2023
These expressions are now rewritten to their prophecy_* counterparts by
goto-program conversion, and back-ends directly handle these prophecy_*
expressions. Unconditional rewrites by goto_check_ct (even when no
checks are enabled) are no longer necessary.

Fixes: diffblue#7377
tautschnig added a commit to tautschnig/cbmc that referenced this issue May 30, 2023
These expressions are now rewritten to their prophecy_* counterparts by
goto-program conversion, and back-ends directly handle these prophecy_*
expressions. Unconditional rewrites by goto_check_ct (even when no
checks are enabled) are no longer necessary.

Fixes: diffblue#7377
tautschnig added a commit to tautschnig/cbmc that referenced this issue May 30, 2023
These expressions are now rewritten to their prophecy_* counterparts by
goto-program conversion, and back-ends directly handle these prophecy_*
expressions. Unconditional rewrites by goto_check_ct (even when no
checks are enabled) are no longer necessary.

Fixes: diffblue#7377
tautschnig added a commit to tautschnig/cbmc that referenced this issue May 31, 2023
These expressions are now rewritten to their prophecy_* counterparts by
goto-program conversion, and back-ends directly handle these prophecy_*
expressions. Unconditional rewrites by goto_check_ct (even when no
checks are enabled) are no longer necessary.

Fixes: diffblue#7377
tautschnig added a commit to tautschnig/cbmc that referenced this issue Jul 19, 2023
These expressions are now rewritten to their prophecy_* counterparts by
goto-program conversion, and back-ends directly handle these prophecy_*
expressions. Unconditional rewrites by goto_check_ct (even when no
checks are enabled) are no longer necessary.

Fixes: diffblue#7377
tautschnig added a commit to tautschnig/cbmc that referenced this issue Aug 16, 2023
These expressions are now rewritten to their prophecy_* counterparts by
goto-program conversion, and back-ends directly handle these prophecy_*
expressions. Unconditional rewrites by goto_check_ct (even when no
checks are enabled) are no longer necessary.

Fixes: diffblue#7377
tautschnig added a commit to tautschnig/cbmc that referenced this issue Aug 21, 2023
These expressions are now rewritten to their prophecy_* counterparts by
goto-program conversion, and back-ends directly handle these prophecy_*
expressions. Unconditional rewrites by goto_check_ct (even when no
checks are enabled) are no longer necessary.

Fixes: diffblue#7377
tautschnig added a commit to tautschnig/cbmc that referenced this issue Aug 23, 2023
These expressions are now rewritten to their prophecy_* counterparts by
goto-program conversion, and back-ends directly handle these prophecy_*
expressions. Unconditional rewrites by goto_check_ct (even when no
checks are enabled) are no longer necessary.

Fixes: diffblue#7377
tautschnig added a commit to tautschnig/cbmc that referenced this issue Sep 7, 2023
These expressions are now rewritten to their prophecy_* counterparts by
goto-program conversion, and back-ends directly handle these prophecy_*
expressions. Unconditional rewrites by goto_check_ct (even when no
checks are enabled) are no longer necessary.

Fixes: diffblue#7377
tautschnig added a commit to tautschnig/cbmc that referenced this issue Sep 7, 2023
These expressions are now rewritten to their prophecy_* counterparts by
goto-program conversion, and back-ends directly handle these prophecy_*
expressions. Unconditional rewrites by goto_check_ct (even when no
checks are enabled) are no longer necessary.

Fixes: diffblue#7377
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws Bugs or features of importance to AWS CBMC users aws-high pending merge
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants