[Lldb-commits] [PATCH] D75200: AddressSanitizer failure in MemoryCache

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 246868.
paolosev added a comment.

In D75200#1894162 <https://reviews.llvm.org/D75200#1894162>, @clayborg wrote:

> Not sure this is the right fix. The calling code should listen to how many bytes were actually read from memory and avoid touching any bytes. So we should fix Module::GetMemoryObjectFile to resize its buffer back to only the size that was read. S


I agree that the previous fix wasn't great, but it had the advantage that it did not change the semantics of `MemoryCache::Read`. I was afraid there could be other places where the caller might assume that MemoryCache::Read fills the whole buffer even when it does not have enough data to copy, even though it was a strange semantics.

(By the way, I am having some difficulties in running //all// the tests with 'ninja check-lldb' to check this fix, both on Windows and on Linux/WSL2, there must be something wrong with my CMake configurations).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75200/new/

https://reviews.llvm.org/D75200

Files:
  lldb/source/Core/Module.cpp
  lldb/source/Target/Memory.cpp


Index: lldb/source/Target/Memory.cpp
===================================================================
--- lldb/source/Target/Memory.cpp
+++ lldb/source/Target/Memory.cpp
@@ -232,8 +232,13 @@
         if (process_bytes_read == 0)
           return dst_len - bytes_left;
 
-        if (process_bytes_read != cache_line_byte_size)
+        if (process_bytes_read != cache_line_byte_size) {
+          if (process_bytes_read < data_buffer_heap_up->GetByteSize()) {
+            dst_len -= data_buffer_heap_up->GetByteSize() - process_bytes_read;
+            bytes_left = process_bytes_read;
+          }
           data_buffer_heap_up->SetByteSize(process_bytes_read);
+        }
         m_L2_cache[curr_addr] = DataBufferSP(data_buffer_heap_up.release());
         // We have read data and put it into the cache, continue through the
         // loop again to get the data out of the cache...
Index: lldb/source/Core/Module.cpp
===================================================================
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -297,7 +297,9 @@
       const size_t bytes_read =
           process_sp->ReadMemory(header_addr, data_up->GetBytes(),
                                  data_up->GetByteSize(), readmem_error);
-      if (bytes_read == size_to_read) {
+      if (bytes_read < size_to_read)
+        data_up->SetByteSize(bytes_read);
+      if (data_up->GetByteSize() > 0) {
         DataBufferSP data_sp(data_up.release());
         m_objfile_sp = ObjectFile::FindPlugin(shared_from_this(), process_sp,
                                               header_addr, data_sp);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D75200.246868.patch
Type: text/x-patch
Size: 1619 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200227/f2ff3285/attachment.bin>