[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

clayborg added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145624/new/ https://reviews.llvm.org/D145624 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

This revision was automatically updated to reflect the committed changes. Closed by commit rGf341d7a4091a: [lldb] Make MemoryCache::Read more resilient (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145624/new/

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM if @clayborg is happy Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145624/new/ https://reviews.llvm.org/D145624

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

bulbazord updated this revision to Diff 504873. bulbazord added a comment. Address small nits from Greg Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145624/new/ https://reviews.llvm.org/D145624 Files: lldb/include/lldb/Target/Memory.h

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

bulbazord added inline comments. Comment at: lldb/source/Target/Memory.cpp:245-246 +if (m_invalid_ranges.FindEntryThatContains(cache_line_base_addr)) { + error.SetErrorStringWithFormat("memory read failed for 0x%" PRIx64, +

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

clayborg added inline comments. Comment at: lldb/source/Target/Memory.cpp:133-135 + if (pos != m_L2_cache.end()) { +return pos->second; + } remove braces for single line if statement per llvm coding guidelines Comment at:

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

bulbazord marked 11 inline comments as done. bulbazord added inline comments. Comment at: lldb/source/Target/Memory.cpp:255-256 - // We need to read from the process + if (process_bytes_read < cache_line_byte_size) +

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

bulbazord updated this revision to Diff 503942. bulbazord added a comment. - Addressed reviewer feedback - Simplified the case where we use L2 cache lines Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145624/new/ https://reviews.llvm.org/D145624

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

bulbazord added a comment. In D145624#4182424 , @clayborg wrote: > I would suggest checking the google stadia patch for the L1 > and L2 caches: > >

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

clayborg added a comment. I would suggest checking the google stadia patch for the L1 and L2 caches: https://github.com/googlestadia/vsi-lldb/blob/master/patches/llvm-project/0019-lldb-Fix-incorrect-L1-inferior-memory-cache-flushing.patch Just to see how they did

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

clayborg added a comment. One other optimization we can do is if we read from the process memory and it returns that is read zero bytes, right now we add the range we were trying to read into the m_invalid_ranges member variable. So lets say we were trying to read the range [0x1000-0x2000) on

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

bulbazord added inline comments. Comment at: lldb/source/Target/Memory.cpp:133-141 // Check the L1 cache for a range that contain the entire memory read. If we - // find a range in the L1 cache that does, we use it. Else we fall back to - // reading memory in

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

clayborg added a comment. There was a fix that was never submitted for Google Stadia for the memory cache here: https://github.com/googlestadia/vsi-lldb/tree/master/patches/llvm-project Might be worth checking what they did to ensure we have all of the same abilities.

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

mib added inline comments. Comment at: lldb/source/Target/Memory.cpp:175 - if (dst && bytes_left > 0) { -const uint32_t cache_line_byte_size = m_L2_cache_line_byte_size; -uint8_t *dst_buf = (uint8_t *)dst; -addr_t curr_addr = addr - (addr % cache_line_byte_size);

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

JDevlieghere added inline comments. Comment at: lldb/source/Target/Memory.cpp:131 + size_t bytes_left = dst_len; This isn't used until line 180. I'd move it down, closer to where it is being used. Comment at:

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, clayborg, jasonmolenda, jingham, labath. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. MemoryCache::Read is not