Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chrono asserts and errors for negative values #1178

Closed
pauldreik opened this issue May 29, 2019 · 1 comment
Closed

chrono asserts and errors for negative values #1178

pauldreik opened this issue May 29, 2019 · 1 comment

Comments

@pauldreik
Copy link
Contributor

While investigating a fuzzer crash, I found that negative values do not seem to be handled correctly. Here are some test cases with comments, which I would appreciate could get added to the unit tests and fixed.
My problem is that the fuzzer trips on these, which stops me from working with the real problem I try to solve :-)

TEST(ChronoTest, Negative_case1) {
  // this prints --12345 instead of -12345
  EXPECT_EQ("-12345", fmt::format("{:%Q}", std::chrono::seconds(-12345)));
}
TEST(ChronoTest, Negative_case2) {
  // this works as intended
  EXPECT_EQ("-03:25:45",
            fmt::format("{:%H:%M:%S}", std::chrono::seconds(-12345)));
  // this prints "-25:25:25" instead of "-25:-25:-25"
  EXPECT_EQ("-25:-25:-25",
            fmt::format("{:%M:%M:%M}", std::chrono::seconds(-12345)));
}
TEST(ChronoTest, Negative_case3) {
  // this prints -s instead of s
  EXPECT_EQ("s", fmt::format("{:%q}", std::chrono::seconds(-12345)));
}
TEST(ChronoTest, Negative_case4) {
  // this works as intended.
  EXPECT_EQ("0.127", fmt::format("{:%S}",
                                std::chrono::duration<char, std::milli>{127}));
  // this triggers an assert in make_unsigned
  EXPECT_EQ("-.127", fmt::format("{:%S}",
                                std::chrono::duration<signed char, std::milli>{-127}));
}
TEST(ChronoTest, Negative_case5) {
  // this works as intended.
  EXPECT_EQ("-03:25", fmt::format("{:%R}", std::chrono::seconds(-12345)));
}
TEST(ChronoTest, Negative_case6) {
  // this works as intended.
  EXPECT_EQ("-03:25:45", fmt::format("{:%T}", std::chrono::seconds(-12345)));
}
TEST(ChronoTest, ShouldntCrash1) {
  try {
    // this triggers an assert in make_unsigned
    fmt::format("{:%R}", std::chrono::duration<char, std::mega>{2});
  } catch (...) {
  }
}
TEST(ChronoTest, ShouldntCrash2) {
  try {
    // this triggers an assert in make_unsigned
    fmt::format("{:%T}", std::chrono::duration<char, std::mega>{2});
  } catch (...) {
  }
}
@vitaut
Copy link
Contributor

vitaut commented May 30, 2019

Negative cases 1, 3 and assertions should be fixed in 30bce6c.

I think case 2 is correct and fmt::format("{:%M:%M:%M}", std::chrono::seconds(-12345)) should produce "-25:25:25" for consistency with other cases (although the whole format is kinda meaningless).

I couldn't repro case 2. It was printing "-00.127" (not "-.127") as expected.

Thanks for catching all of these!

@vitaut vitaut closed this as completed May 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants