Skip to content
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

[BUG] OneWord*SBitSet.nextClearBit() is incorrect + safety issue #934

Open
fhermeni opened this issue Sep 8, 2022 · 1 comment
Open
Labels

Comments

@fhermeni
Copy link
Contributor

fhermeni commented Sep 8, 2022

The following snippet illustrates 2/3 issues with choco 4.10.8:

        final IEnvironment env = new EnvironmentTrailing();
        final IStateBitSet b64 = new OneWordS64BitSet(env, 12);
        final IStateBitSet b32 = new OneWordS32BitSet(env, 1);
        final IStateBitSet b = new S64BitSet(env, 1);
        b.set(63);
        b64.set(63);
        System.out.printf("nextClear(63): b64=%d, b=%d%n", b64.nextClearBit(63), b.nextClearBit(63));
        b32.set(31);
        System.out.printf("b32.nextClear(31): %d%n", b32.nextClearBit(31));
        b.clear();
        b64.clear();
        b32.clear();
        // Write outside the word length, I expect an error or at minimum no change.
        b32.set(32);
        b64.set(64);
        System.out.printf("b32: %s, b64: %s%n", b32, b64);
  1. nextClearBit() is incorrect when the highest bit is set. size() should be returned. The issue is clear when we compare the output between OneWord64 and S64BitSet.
  2. There are no guardrails to prevent from manipulating bits outside the word length. This may be for performance reasons but this is dangerous as other bits are set. In terms of consistency, there is a check for a negative value, accordingly an additional one wrt. the word length may be helpful as well.
  3. (Nit): the nBits argument in the constructor for OneWord*BitSet is useless.
@fhermeni fhermeni added the bug label Sep 8, 2022
@fhermeni
Copy link
Contributor Author

fhermeni commented Sep 8, 2022

Hello
One additional point:

        b64 = new OneWordS64BitSet(env, 12);
        b32 = new OneWordS32BitSet(env, 1);
        b32.set(3);
        b64.set(3);
        System.out.printf("b32:%s b64:%s equals:%s", b32, b64, b32.equals(b64));

equals() are implementation specific so despite the content may be the same wrt. the interface specification, the practical implementation impacts the result. Given users are likely to create them from an IEnvironment, the implementation is hidden which may lead to bugs in case equals() is required.

In theory, equals() should be robust against the interface implementation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant