`<valarray>`: self-assignment by `slice_array` causes dangerous behavior

[achabense]

Look at this example. If v = v[std::slice{0, 4, 1}] isn't UB itself(I'm not too sure), then it is dangerously implemented currently.

#include<valarray>

int main() {
	std::valarray v{1, 2, 3, 4, 5};
	v = v[std::slice{0, 4, 1}];
}

In the current implementation, this means v will firstly do _Tidy_deallocate, which deallocates _Myptr; then do _Grow, reading data from _Myptr.

template <class _Ty>
valarray<_Ty>& valarray<_Ty>::operator=(const slice_array<_Ty>& _Slicearr) {
    _Tidy_deallocate();
    _Grow(_Slicearr.size(), &_Slicearr._Data(_Slicearr.start()), _Slicearr.stride());
    return *this;
}

Take the above code for example, a fixed version would be:

template <class _Ty>
valarray<_Ty>& valarray<_Ty>::operator=(const slice_array<_Ty>& _Slicearr) {
    valarray<_Ty> _Val(_Slicearr);
    swap(_Val);
    return *this;
}

All the following four functions have the same problem.

STL/stl/inc/valarray

Lines 187 to 193 in a621095

	valarray& operator=(const slice_array<_Ty>& _Slicearr); // defined below
	valarray& operator=(const gslice_array<_Ty>& _Gslicearr); // defined below
	valarray& operator=(const mask_array<_Ty>& _Maskarr); // defined below
	valarray& operator=(const indirect_array<_Ty>& _Indarr); // defined below

[frederick-vs-ja]

I think v = v[std::slice{0, 4, 1}] is UB itself, see [valarray.assign]/10. It might be better to diagnose erroneous uses rather than support them.

[achabense]

Maybe they are still worth some changes, either to do some diagnostics or to model the behavior of =(const valarray&). What's more, a failed _Grow will cause v to lose its data.

[frederick-vs-ja]

What's more, a failed _Grow will cause v to lose its data.

It seems to me that MSVC STL hasn't implemented these assigment operators as expected. I think the intent of [valarray.assign]/10 is allowing in-place assignment, and thus allocation might be unexpected.

[CaseyCarter]

It's not clear to me that there's a bug here, tagging this "info needed" until we can investigate.

[StephanTLavavej]

We believe that this is indeed UB for the Standardese quoted by #3728 (comment) .

While we merged debug checks to detect forbidden aliasing in vector operations, valarray usage is much rarer (especially in new code) so the usefulness of adding such checks doesn't seem to be worth the effort, especially due to the complexity of how many slices etc. valarray supports.