[libcxx][ios] initialize __fill_val_ in _FillHelper

[daltenty]

This is a small fix to #89305. In the __init function of _FillHelper, __fill_val_ was left uninitialized. This worked for the implementation in the PR because we always checked __set_ before trying to read it, and would initialize if it was unset.

However it turns out in earlier versions of the header (at least on AIX which followed this path), we do a read of __fill_val_ even if __set_ was false before initializing, to check if it matched the sentinel value, so this causes undesired behaviour and UB.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: David Tenty (daltenty)

Changes

This is a small fix to #89305. In the __init function of _FillHelper, __fill_val_ was left uninitialized. This worked for the implementation in the PR because we always checked __set_ before trying to read it, and would initialize if it was unset.

However it turns out in earlier versions of the header (at least on AIX which followed this path), we do a read of __fill_val_ even if __set_ was false before initializing to check if it matched the sentinel value, so this causes undesired behaviour and UB.

---

Full diff: https://github.com/llvm/llvm-project/pull/110279.diff

1 Files Affected:

• (modified) libcxx/include/ios (+4-1)

diff --git a/libcxx/include/ios b/libcxx/include/ios
index 61a05fadd29a17..d4f15a269a11a6 100644
--- a/libcxx/include/ios
+++ b/libcxx/include/ios
@@ -524,7 +524,10 @@ template <class _Traits>
 // Attribute 'packed' is used to keep the layout compatible with the previous
 // definition of the '__fill_' and '_set_' pair in basic_ios on AIX & z/OS.
 struct _LIBCPP_PACKED _FillHelper {
-  _LIBCPP_HIDE_FROM_ABI void __init() { __set_ = false; }
+  _LIBCPP_HIDE_FROM_ABI void __init() {
+    __set_      = false;
+    __fill_val_ = _Traits::eof();
+  }
   _LIBCPP_HIDE_FROM_ABI _FillHelper& operator=(typename _Traits::int_type __x) {
     __set_      = true;
     __fill_val_ = __x;

[ldionne]

How did you catch the bug? Is it possible to add a test that triggers that?

[daltenty]

How did you catch the bug? Is it possible to add a test that triggers that?

We have internal runs that test using the old headers with the new dylib (I guess it would be nice to have such runs for at least a few releases in the community CI ideally)

Adding a legitimate test against the current code doesn't seem straight forward, this depends on the old internal behaviour of basic_ios<_CharT, _Traits>::fill() in the header

[xingxue-ibm]

LGTM, Thanks for the fix!