The newest version of Visual Studio C++ compiler broke our code. Our application uses
STL algorithm like std::minmax_element on measured samples. Sometimes the devices do not deliver a valid sample and a NaN element is inserted instead.
Up to now the STL algorithms ignored NaN completely (GCC, CLANG, Visual Studio)
The crash is reproduce able with this minimum example:
#include <algorithm>
#include <array>
#include <limits>
#include <iostream>
int main()
{
const double inf = std::numeric_limits<double>::infinity();
const double nan = std::numeric_limits<double>::quiet_NaN();
double values[] = {0, -inf, nan};
const auto mm = std::minmax_element(std::begin(values), std::end(values));
std::cout << "Min: " << *mm.first << std::endl;
std::cout << "Max: " << *mm.second << std::endl;
return std::distance(mm.first, mm.second);
}
Playground: https://gcc.godbolt.org/z/GhTPsfj5q
Expected:
example.cpp
ASM generation compiler returned: 0
example.cpp
Execution build compiler returned: 0
Program returned: 1
Min: -inf
Max: nan
VS 2022 >= 17.10
example.cpp
ASM generation compiler returned: 0
example.cpp
Execution build compiler returned: 0
Program returned: 4294967295
Min: -inf
Max: 0
The STL expert from MS arguments that STL algorithms expect "strictly-weak-ordering". A sequence containing NaN does not fulfill this requirement and therefore we wander the world of undefined behavior.
A STL expert from my team understands the standard so that strictly-weak-ordering is only needed in C++-20 when using std::less.
Still I need to know, if we have to cleanup our data sequences upfront, before calling STL algorithms in future to be save.
Thank you a 1000 times!
Bye Gunther
2
Answers
From C++17 final draft:
28.7.3 lists a number of binary search algorithms (
lower_bound
,upper_bound
,equal_range
andbinary_search
) andminmax_element
is not among them so it’s not exempt from the strict weak ordering requirement which means that includingNaN
s in the range makes your program have undefined behavior.They understand wrong. The change between C++17 and C++20 was changing the default comparison from
<
tostd::less
. It has always been a requirement for whichever comparison is used to be a strict-weak-order.This change only affects algorithms applied to ranges of pointers, as
std::less
is defined to use<
for everything apart from pointers, where it synthesizes a strict-weak-order. For historical reasons, it is undefined to apply<
to pointers to elements of distinct arrays.