[Lldb-commits] [PATCH] D80150: [lldb/DataFormatter] Check for overflow when finding NSDate epoch

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Wed May 20 02:07:17 PDT 2020

labath added a comment.

In D80150#2045364 <https://reviews.llvm.org/D80150#2045364>, @vsk wrote:

> @labath Agreed on all points, I've addressed the feedback in 82dbf4aca84 <https://reviews.llvm.org/rG82dbf4aca84ec889d0dc390674ff44e30441bcfd> by moving "DataFormatters/Mock.h" to "Plugins/Language/ObjC/Utilities.h", and adding a separate LanguageObjCTests unit test.

Cool. Thanks.

Comment at: lldb/unittests/DataFormatter/MockTests.cpp:30
+  // Can't convert the date_value to a time_t.
+  EXPECT_EQ(formatDateValue(std::numeric_limits<time_t>::max() + 1),
+            llvm::None);
vsk wrote:
> labath wrote:
> > Isn't this actually `std::numeric_limits<time_t>::min()` (and UB due to singed wraparound) ? Did you want to convert to double before doing the `+1` ?
> Yes, thank you! It looks like Eric caught this before I did.
Actually, thinking about that further, (for 64-bit `time_t`s), `double(numeric_limits<time_t>::max())` is [[ https://godbolt.org/z/t3iSd7 | exactly the same value ]] as `double(numeric_limits<time_t>::max())+1.0` because `double` doesn't have enough bits to represent the value precisely. So, I have a feeling these checks are still not testing the exact thing you want to test (though I'm not sure what that is exactly).

  rG LLVM Github Monorepo



More information about the lldb-commits mailing list