From 0e5ce18cff2bc63c4f53ece06dae1a0e9ed44527 Mon Sep 17 00:00:00 2001 From: Saran Tunyasuvunakool Date: Thu, 16 Jun 2022 11:57:19 -0700 Subject: [PATCH] Manually parse inf and nan. The C++ standard library `std::istringstream` is not guaranteed to parse "inf" and "nan" as valid floating point numbers. In our testing, libc++ does this, but libstdc++ and MSVCRT do not. PiperOrigin-RevId: 455435431 Change-Id: I99c00303b08c2e4e62acfa3ca52a21a3008f4545 --- src/xml/xml_util.cc | 43 ++++++++++++++++++++++++--- test/xml/xml_native_reader_test.cc | 47 +++++++++++++++++++++++------- 2 files changed, 76 insertions(+), 14 deletions(-) diff --git a/src/xml/xml_util.cc b/src/xml/xml_util.cc index 6372c85b..7263764d 100644 --- a/src/xml/xml_util.cc +++ b/src/xml/xml_util.cc @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -42,6 +43,32 @@ using tinyxml2::XMLElement; namespace mju = ::mujoco::util; +template +std::optional ParseInfOrNan(const std::string& s) { + const char* str = s.c_str(); + if constexpr (std::is_floating_point_v) { + T sign = 1; + if (s.size() == 4 && s[0] == '-') { + sign = -1; + ++str; + } else if (s.size() != 3) { + return std::nullopt; + } + if (std::numeric_limits::has_infinity && + (str[0] == 'i' || str[0] == 'I') && + (str[1] == 'n' || str[1] == 'N') && + (str[2] == 'f' || str[2] == 'F')) { + return sign * std::numeric_limits::infinity(); + } else if (std::numeric_limits::has_quiet_NaN && + (str[0] == 'n' || str[0] == 'N') && + (str[1] == 'a' || str[1] == 'A') && + (str[2] == 'n' || str[2] == 'N')) { + return sign * std::numeric_limits::quiet_NaN(); + } + } + return std::nullopt; +} + } // namespace @@ -565,14 +592,22 @@ int mjXUtil::ReadAttr(XMLElement* elem, const char* attr, const int len, while (!strm.eof() && i < len) { strm >> token; istringstream token_strm(token); - token_strm >> data[i++]; + token_strm >> data[i]; if (token_strm.fail() || !token_strm.eof()) { - throw mjXError(elem, "problem reading attribute '%s'", attr); - } else if constexpr (std::is_floating_point_v) { - if (std::isnan(data[i-1])) { + // C++ standard libraries do not always parse inf and nan as valid floating point values. + std::optional maybe_result = ParseInfOrNan(token); + if (maybe_result.has_value()) { + data[i] = *maybe_result; + } else { + throw mjXError(elem, "problem reading attribute '%s'", attr); + } + } + if constexpr (std::is_floating_point_v) { + if (std::isnan(data[i])) { mju_warning("XML contains a 'NaN'. Please check it carefully."); } } + ++i; } strm >> std::ws; diff --git a/test/xml/xml_native_reader_test.cc b/test/xml/xml_native_reader_test.cc index 04ea596c..0d6e8121 100644 --- a/test/xml/xml_native_reader_test.cc +++ b/test/xml/xml_native_reader_test.cc @@ -16,6 +16,7 @@ #include #include +#include #include #include @@ -32,6 +33,7 @@ namespace { using ::std::string; using ::testing::Eq; using ::testing::HasSubstr; +using ::testing::IsNan; using ::testing::IsNull; using ::testing::NotNull; @@ -133,12 +135,36 @@ TEST_F(UserDataTest, InvalidNUserSensor) { EXPECT_THAT(error.data(), HasSubstr("nuser_sensor")); } -TEST_F(UserDataTest, RaiseNanWarning) { +TEST_F(UserDataTest, CanParseInf) { static constexpr char xml[] = R"( - + + + + + + )"; + const double inf = std::numeric_limits::infinity(); + mjModel* model = LoadModelFromString(xml); + ASSERT_THAT(model, NotNull()); + EXPECT_EQ(model->geom_pos[0], 0.5); + EXPECT_EQ(model->geom_pos[1], -inf); + EXPECT_THAT(model->geom_pos[2], inf); + EXPECT_EQ(model->geom_pos[3], inf); + EXPECT_EQ(model->geom_pos[4], -inf); + EXPECT_EQ(model->geom_pos[5], inf); + mj_deleteModel(model); +} + +TEST_F(UserDataTest, CanParseNanAndRaisesWarning) { + static constexpr char xml[] = R"( + + + + + @@ -150,14 +176,15 @@ TEST_F(UserDataTest, RaiseNanWarning) { util::strcpy_arr(warning, msg); }; mjModel* model = LoadModelFromString(xml, error.data(), error.size()); -#if defined(_WIN32) || defined(__CYGWIN__) - ASSERT_THAT(model, IsNull()); - EXPECT_THAT(error.data(), HasSubstr("problem reading attribute 'axisangle'")); -#else - ASSERT_THAT(model, NotNull()); - EXPECT_THAT(warning, HasSubstr("XML contains a 'NaN'")); - mj_deleteModel(model); -#endif + ASSERT_THAT(model, NotNull()); + EXPECT_THAT(warning, HasSubstr("XML contains a 'NaN'")); + EXPECT_THAT(model->geom_pos[0], IsNan()); + EXPECT_THAT(model->geom_pos[1], IsNan()); + EXPECT_THAT(model->geom_pos[2], IsNan()); + EXPECT_EQ(model->geom_pos[3], 1); + EXPECT_EQ(model->geom_pos[4], 0); + EXPECT_THAT(model->geom_pos[5], IsNan()); + mj_deleteModel(model); } TEST_F(UserDataTest, InvalidArrayElement) {