philnik accepted this revision. philnik added a comment. This revision is now accepted and ready to land.
LGTM assuming you checked that it actually works with my patch applied. Do you want to land your patch first, or should we do it the other way around? ================ Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:658 + if (!using_bitmasks) + capacity *= 2; if (size == LLDB_INVALID_OFFSET || capacity == LLDB_INVALID_OFFSET || ---------------- labath wrote: > philnik wrote: > > This should only be done if the string is in the normal layout and little > > endian or in the alternate layout and big endian. Why do you care about the > > capacity at all? Isn't that just another point of failure? > I've added the layout check and made a note that big-endian is not supported. > The capacity check comes from <https://reviews.llvm.org/D73860>, but I'm not > sure of the overall purpose. I guess it was trying to screen out > corrupt/uninitialized string objects, but I'm not convinced that we're doing > anyone a favour by refusing to format those -- after all, the application > itself might access such objects without checking the capacity, and operate > on the data that we've decided to ignore. Maybe it would make sense to say that the string is uninitialized or corrupted when the `__invariants()` fail? That would probably catch more bugs, but I don't know if you can just run a member function in the pretty printer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124113/new/ https://reviews.llvm.org/D124113 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
