[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
time_t?

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).
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D80150/new/
>
> https://reviews.llvm.org/D80150
>
>
>
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200520/031f3dff/attachment.html>


More information about the lldb-commits mailing list