vsk added inline comments.
================
Comment at: lldb/source/DataFormatters/StringPrinter.cpp:144
+ return retval;
+ if (!llvm::checkedAdd(reinterpret_cast<intptr_t>(buffer),
+ static_cast<intptr_t>(utf8_encoded_len)))
----------------
shafik wrote:
> Wouldn't we want `checkedAddUnsigned`? This would also mean casting to
> `uintptr_t`.
Not sure about that, as a change in the MSB seems significant to me.
Stepping back a bit though, it seems like this overflow check isn't useful. The
data formatter must have called `GetPointeeData` to transfer the string payload
into a host buffer, and we validate the number of bytes read against the size
of the payload. So, if pointer overflow occurs, the `bytes_read` validation
would catch it.
I'll just delete this.
================
Comment at: lldb/source/DataFormatters/StringPrinter.cpp:474
uint8_t *next_data = nullptr;
auto printable = escaping_callback(data, data_end, next_data);
auto printable_bytes = printable.GetBytes();
----------------
shafik wrote:
> Not your code but these `auto` seem unnecessary. Especially `printable` is
> that just a `bool`? Same comment in similar code above.
I'll leave this as a follow-up. `printable` is a
`StringPrinter::StringPrinterBufferPointer`.
================
Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:530
+
+ // When the small-string optimization takes place, the data must fit in the
+ // inline string buffer (23 bytes on x86_64/Darwin). If it doesn't, it's
----------------
shafik wrote:
> So `short_mode` means SSO?
Yes.
================
Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:555
+ size = size_vo->GetValueAsUnsigned(LLDB_INVALID_OFFSET);
+ const uint64_t cap = cap_vo->GetValueAsUnsigned(LLDB_INVALID_OFFSET);
+ if (size == LLDB_INVALID_OFFSET || cap == LLDB_INVALID_OFFSET || cap <
size)
----------------
shafik wrote:
> What does `cap` represent? It is not obvious in this context.
"capacity". I'm using the same jargon as the comment above
`LibcxxStringLayoutMode` does, but it doesn't hurt to write this out.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73860/new/
https://reviews.llvm.org/D73860
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
