This is an archive of the discontinued LLVM Phabricator instance.

Change Target::ReadMemory to ensure the amount of memory read from the file-cache is the amount requested.
ClosedPublic

Authored by augusto2112 on Apr 27 2021, 12:31 PM.

Details

Summary

This change ensures that if for whatever reason we read less bytes than expected (for example, when trying to read memory that spans multiple sections), we try reading from the live process as well.

Diff Detail

Event Timeline

augusto2112 requested review of this revision.Apr 27 2021, 12:31 PM
augusto2112 created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptApr 27 2021, 12:31 PM
shafik added a subscriber: shafik.Apr 28 2021, 11:35 AM
shafik added inline comments.
lldb/source/Target/Target.cpp
1780

Is there a reason why we need to use malloc and free?

augusto2112 added inline comments.Apr 28 2021, 11:40 AM
lldb/source/Target/Target.cpp
1780

Since the type is void I thought that was the correct way. What should I use instead?

shafik added inline comments.Apr 28 2021, 5:14 PM
lldb/source/Target/Target.cpp
1780

I looked at the places we are calling Target::ReadMemory and they are using some form of a uint8_t buffer. So a uint8_t array would be fine:

std::unique_ptr<uint8_t[]> file_cache_read_buffer;
//....
p = std::make_unique<uint8_t[]>(file_cache_bytes_read);

Change buffer unique pointer from void to uint8_t, eliminating the need to call malloc/free.

augusto2112 marked 2 inline comments as done.Apr 29 2021, 8:42 AM
augusto2112 added inline comments.
lldb/source/Target/Target.cpp
1780

Thanks Shakif, it does look a bit simpler now.

shafik added inline comments.Apr 29 2021, 9:30 AM
lldb/source/Target/Target.cpp
1780

std::vector for a buffer is also possible some callers do that but I don't see a reason to prefer that over this solution.

JDevlieghere added inline comments.Apr 29 2021, 9:38 AM
lldb/source/Target/Target.cpp
1823–1830

The unique_ptr provides an operator bool, so you don't need to check the underlying pointer yourself.

1830
augusto2112 marked an inline comment as done.

Rely on unique_ptr's bool operator, instead of checking the underlying pointer.

augusto2112 marked 2 inline comments as done.Apr 29 2021, 9:43 AM
augusto2112 added inline comments.
lldb/source/Target/Target.cpp
1823–1830

Thanks Jonas!

This revision is now accepted and ready to land.May 11 2021, 1:28 AM
This revision was landed with ongoing or failed builds.May 11 2021, 9:09 AM
This revision was automatically updated to reflect the committed changes.
augusto2112 marked an inline comment as done.