Skip to content

[libc++] {std, ranges}::equal algorithms for vector<bool>::iterator fail with small storage types #126369

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
winner245 opened this issue Feb 8, 2025 · 1 comment · Fixed by #130394
Assignees
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Comments

@winner245
Copy link
Contributor

winner245 commented Feb 8, 2025

When using std::equal with vector<bool>::iterator in libc++, the function fails to compare vectors correctly for vector<bool> using storage types smaller than int, e.g., unsigned char, unsigned short, uint8_t and uint16_t. The issue arises when these small integral types are used as the underlying storage types for vector<bool>, where intermediate bitwise operations involving these small integral types are subject to integral promotions, leading to incorrect final equality comparison results.

Bug Reproduction

I have prepared a carefully written demo that illustrates the incorrect equality comparisons of std::equal when comparing two vector<bool> instances. With carefully chosen storage types, input sizes, and bit offsets for the two input vectors, we've captured incorrect comparisons in every possible code path:

  • Error processing in the first, last, or middle words;
  • Error processing in aligned or unaligned code paths.

For comparison purposes, the demo also provides the correct equality comparisons obtained from the standard general-form std::equal which works with all general input_iterators. To ensure that the standard std::equal is invoked, I wrap the input vector<bool>::iterators into standard input_iterators before calling std::equal (iterator unwrapping currently doesn't work, which is another issue to be fixed). Furthermore, we also provide the comparison results obtained from other implementations like MSVC-STL or libstdc++.

Key observations

  • While the std::equal optimization for vector<bool>::iterator fails with small integral types in libc++, the problem does not appear in other implementations such as MSVC-STL or libstdc++, nor does it occur with the standard std::equal implementation in libc++.

  • The same issue also carries over to the std::ranges::equal algorithm with the __bit_iterator optimization in libc++, as the range version algorithm boils down to calling the non-range version.

This behavior is clearly a bug in libc++. It is crucial to address this issue to ensure correct functionality across all storage types for vector<bool>.

Related issues: #121713, #122410, #122528

@winner245 winner245 added the bug Indicates an unexpected problem or unintended behavior label Feb 8, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 8, 2025

@llvm/issue-subscribers-bug

Author: Peng Liu (winner245)

When using `std::equal` with `vector<bool>::iterator` in libc++, the function fails to compare vectors correctly for `vector<bool>` using storage types smaller than `int`, e.g., `unsigned char`, `unsigned short`, `uint8_t` and `uint16_t`. The issue arises when these small integral types are used as the underlying storage types for `vector<bool>`, where intermediate bitwise operations involving these small integral types are subject to integral promotions, leading to incorrect final equality comparison results.

Bug Reproduction

I have prepared a carefully written demo that illustrates the incorrect equality comparisons of std::equal when comparing two vector&lt;bool&gt; instances. With carefully chosen storage types, input sizes, and bit offsets for the two input vectors, we've captured incorrect comparisons in every possible code path:

  • Error processing in the first, last, or middle words;
  • Error processing in aligned or unaligned code paths.

For comparison purposes, the demo also provides the correct equality comparisons obtained from the standard general-form std::equal which works with all general input_iterators. To ensure that the standard std::equal is invoked, I wrap the input vector&lt;bool&gt;::iterators into standard input_iterators before calling std::equal. Furthermore, we also provide the comparison results obtained from other implementations like MSVC-STL or libstdc++.

Key observations

  • While the std::equal optimization for vector&lt;bool&gt;::iterator fails with small integral types in libc++, the problem does not appear in other implementations such as MSVC-STL or libstdc++, nor does it occur with the standard std::equal implementation in libc++.

  • The same issue also carries over to the std::ranges::equal algorithm with the __bit_iterator optimization in libc++, as the range version algorithm boils down to calling the non-range version.

This behavior is clearly a bug in libc++. It is crucial to address this issue to ensure correct functionality across all storage types for vector&lt;bool&gt;.

Related issues: #122528, #121713

@winner245 winner245 added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Feb 8, 2025
@EugeneZelenko EugeneZelenko removed the bug Indicates an unexpected problem or unintended behavior label Feb 8, 2025
winner245 added a commit to winner245/llvm-project that referenced this issue Feb 8, 2025
@winner245 winner245 self-assigned this Feb 20, 2025
winner245 added a commit to winner245/llvm-project that referenced this issue Feb 20, 2025
ldionne pushed a commit that referenced this issue Mar 19, 2025
… types (#130394)

The current implementation of `{std, ranges}::equal` fails to correctly
compare `vector<bool>`s when the underlying storage type is smaller than
`int` (e.g., `unsigned char`, `unsigned short`, `uint8_t` and
`uint16_t`). See [demo](https://godbolt.org/z/j4s87s6b3)). The problem
arises due to integral promotions on the intermediate bitwise
operations, leading to incorrect final equality comparison results. This
patch fixes the issue by ensuring that `{std, ranges}::equal` operate
properly for both aligned and unaligned bits.
 
Fixes #126369.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants