Skip to content

Unexpected result when calling count intrinsics with zero #6702

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
adpaco-aws opened this issue Feb 28, 2022 · 2 comments · Fixed by #6703
Closed

Unexpected result when calling count intrinsics with zero #6702

adpaco-aws opened this issue Feb 28, 2022 · 2 comments · Fixed by #6703

Comments

@adpaco-aws
Copy link
Contributor

CBMC version: 5.50.0
Operating system: Ubuntu 20.04
Exact command line resulting in the issue: cbmc example.c, where example.c is

#include <assert.h>

int main() {
    unsigned char var = 0;
    assert(__builtin_clz(var) == 8);
}

What behaviour did you expect: Successful verification
What happened instead: Verification failed.
The result is unexpected because the commit that originally added support says "The result is always defined, even for zero (where the result is the bit width)". A similar thing happens if __builtin_clz is used instead.

@tautschnig
Copy link
Collaborator

I believe the behaviour is as expected: __builtin_clz (or __builtin_ctz) are undefined for zero. That's GCC's semantics, not ones of our own choosing. What is, however, defined, is the value of the underlying expression count_leading_zeros_exprt and count_trailing_zeros_exprt, respectively -- which is all that the comment is referring to.

@tautschnig
Copy link
Collaborator

Re-opening as it turns out there actually is an issue in count_trailing_zeros_exprt, which did mishandle zero. Bugfix forthcoming.

@tautschnig tautschnig reopened this Mar 1, 2022
tautschnig added a commit to tautschnig/cbmc that referenced this issue Mar 1, 2022
The previous implementation would produce an off-by-one error in case of
an argument 0, which count_trailing_zeros is defined for (even when
__builtin_ctz is undefined in that case).

Fixes: diffblue#6702
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants