================
@@ -254,13 +254,17 @@ bool
lldb_private::formatters::LibStdcppStringSummaryProvider(
} else
addr_of_string =
valobj.GetAddressOf(scalar_is_load_addr, &addr_type);
- if (addr_of_string != LLDB_INVALID_ADDRESS) {
+
+ // We have to check for host address here
+ // because GetAddressOf returns INVALID for all non load addresses.
+ // But we can still format strings in host memory.
+ if (addr_of_string != LLDB_INVALID_ADDRESS ||
+ addr_type == eAddressTypeHost) {
----------------
clayborg wrote:
> I think we need to be more careful here. GetAddressOf is really meant to do
> "can you find an address in the target for this object". We use it that way
> in a whole bunch of places, e.g.:
>
> ```
> cstr_address = GetAddressOf(true, &cstr_address_type);
> } else {
> // We have a pointer
> cstr_address = GetPointerValue(&cstr_address_type);
> }
>
> if (cstr_address == 0 || cstr_address == LLDB_INVALID_ADDRESS) {
> if (cstr_address_type == eAddressTypeHost && is_array) {
> const char *cstr = GetDataExtractor().PeekCStr(0);
> ```
>
> So in that case we are expecting a host address type to return an invalid
> address from GetAddressOf.
>
> This change worries me, I don't think it will be what other code expects?
> Maybe you can get the value you need like the code above does instead?
The code above works, but I think it doesn't total sense from a readability
standpoint. We have a `GetAddressOf` function that return the address kind and
a value, I am not sure why we wouldn't want to take advantage of that and use
it? The above code is a bit interesting as it makes a bunch of assumptions like
"is this a host address and if this is an array, I know my data is in the data
extractor at offset zero". I mean it works, but does it make sense to duplicate
these kind of assumptions?
Make sense to fix GetAddressOf to take advantage of the API it is implementing.
If the address kind can be host and we can return a valid host address value, I
would say we use it. We will need to look over all uses of this API internally
if we do change it.
https://github.com/llvm/llvm-project/pull/89110
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
