Page MenuHomePhabricator

Display the pointer value in the libstdc++ unique_ptr summary
ClosedPublic

Authored by labath on Nov 8 2016, 9:07 AM.

Details

Summary

r284830 added a summary provider for unique_ptr in libstdc++, whose value printed
the value of the pointee. This is a bit unintuitive as it becomes unobvious that
the value actually is a pointer, and we lose the way to actually obtain the
pointer value.

Change that to print the pointer value instead. The pointee value can still be
obtained through the synthetic children.

Diff Detail

Repository
rL LLVM

Event Timeline

labath updated this revision to Diff 77202.Nov 8 2016, 9:07 AM
labath retitled this revision from to Display the pointer value in the libstdc++ unique_ptr summary.
labath updated this object.
labath added reviewers: tberghammer, granata.enrico.
labath added a subscriber: lldb-commits.
granata.enrico accepted this revision.Nov 8 2016, 10:36 AM
granata.enrico edited edge metadata.

Make what you will of my suggestion above, but in general the reasoning looks good

source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
129 ↗(On Diff #77202)

This is very nitpick-y but why not

lldb::addr_t ptr_value = m_ptr_obj->GetValueAsUnsigned(LLDB_INVALID_ADDRESS)
switch (ptr_value) {

case 0: stream.Printf("nullptr"); break;
case LLDB_INVALID_ADDRESS: return false;
default: stream.Printf("0x%" PRIx64, ptr_value); break;

}

modulo clang-formatting the above, of course.

This revision is now accepted and ready to land.Nov 8 2016, 10:36 AM
labath added inline comments.Nov 9 2016, 2:51 AM
source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
129 ↗(On Diff #77202)

I've changed this to:

uint64_t ptr_value = m_ptr_obj->GetValueAsUnsigned(0, &success);
if (!success)
  return false;
if (ptr_value == 0)
  stream.Printf("nullptr");
else
  stream.Printf("0x%" PRIx64, ptr_value);

This should address the issue of not printing a bogus value if we fail to read the value, but avoid the case where the pointer value *is* 0xffff.. (which it can legitimately be, e.g. with custom deleters, or if the inferior corrupts it's memory).
At that point, the switch is not necessary.

This revision was automatically updated to reflect the committed changes.

Makes sense. Feel free to commit.