[Lldb-commits] [PATCH] D80150: [lldb/DataFormatter] Check for overflow when finding NSDate epoch
Eric Christopher via lldb-commits
lldb-commits at lists.llvm.org
Wed May 20 09:23:45 PDT 2020
Agreed. Something is off here. My change was only to silence a few
warnings, but they're definitely highlighting a conversion issue. What's up
with NSDate conversions here. Does the API have a way to convert from
On Wed, May 20, 2020, 2:07 AM Pavel Labath via Phabricator via lldb-commits
<lldb-commits at lists.llvm.org> wrote:
> 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
> CHANGES SINCE LAST ACTION
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the lldb-commits