[libcxx-commits] [libcxx] [libc++] Fix possible out of range access in bitset::to_ullong implementation (PR #121348)

[Peng Liu via libcxx-commits]

================
@@ -381,8 +382,9 @@ __bitset<_N_words, _Size>::to_ullong(true_type, true_type) const {
   unsigned long long __r = __first_[0];
   _LIBCPP_DIAGNOSTIC_PUSH
   _LIBCPP_GCC_DIAGNOSTIC_IGNORED("-Wshift-count-overflow")
-  for (size_t __i = 1; __i < sizeof(unsigned long long) / sizeof(__storage_type); ++__i)
-    __r |= static_cast<unsigned long long>(__first_[__i]) << (sizeof(__storage_type) * CHAR_BIT);
+  size_t __n_words = std::min<size_t>(_N_words, sizeof(unsigned long long) / sizeof(__storage_type));
----------------
winner245 wrote:

It seems that the Stage 1 CI failure I mentioned earlier has been resolved. To identify the root cause, I had to revert all changes and then push incremental modifications. I apologize for any frequent notifications you may have received due to my numerous pushes. 

The issue originated from the changes I made. I added a pre-truncation step in the `std::bitset` ULL constructor's member initializer list to ensure that the ULL parameter `__v` fits within _Size bits:

```cpp
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR bitset(unsigned long long __v) _NOEXCEPT
    : __base(sizeof(unsigned long long) * CHAR_BIT <= _Size ? __v : __v & ((1ULL << _Size) - 1)) {}
```

The purpose of this change is to ensure that the base class `__bitset<_N_words, _Size>` ULL constructor no longer needs to handle any truncation for out-of-range bit widths, thus simplifying the implementation of the ULL constructor of the base class (otherwise, the constructor would be even more ugly).

Similarly, the ULL constructor for the 1-word specialization `__bitset<1, _Size>` also no longer needs truncation, which was previously handled as follows:

```cpp
template <size_t _Size>
inline _LIBCPP_CONSTEXPR __bitset<1, _Size>::__bitset(unsigned long long __v) _NOEXCEPT
    : __first_(_Size == __bits_per_word ? static_cast<__storage_type>(__v)
                                        : static_cast<__storage_type>(__v) & ((__storage_type(1) << _Size) - 1)) {}
```

By removing the uncesary truncation, I simplified the above constructor to:

```cpp
template <size_t _Size>
inline _LIBCPP_CONSTEXPR __bitset<1, _Size>::__bitset(unsigned long long __v) _NOEXCEPT
    : __first_(static_cast<__storage_type>(__v)) {}
```

Unexpectedly, this last simplification for the 1-word specialization has caused the error `gdb.error: There is no member or method named __bits_per_word`. After reverting this simplification, the error disappeared. I am still trying to understand why this change caused the gdb error, while the same simplification applied in the constructor of the primary template `__bitset<_N_words, _Size>` compiled successfully.

https://github.com/llvm/llvm-project/pull/121348