[Lldb-commits] [lldb] [lldb] Memory cache: no overlap and read across cache entries (PR #222688)

[Yao Qi via lldb-commits]

qiyao wrote:

> The only high level piece of feedback I have left is that I, personally, would not have tried to handle the cases where address + length overflow, as it makes some parts of the code harder to reason about. This is for two reasons:
> 
> 1. if this overflows in the LLDB code, it would likely also overflow in the process itself, so realistically this code should never trigger.
> 2. To provide some safety, I would simply have had an early return at the entry point of the caches: if a addr+length cache request overflows, just don't cache it.

That is a fair point on the insert path, and I will have a followup PR for it. On the flush path I would rather keep the clamp.   The reason is below,

first of all, the overflow handling is not new in this PR. `MemoryCache::Flush` has carried an explicit guard since 2012, de0e9d04ad17 and `MemoryCache::Read` has avoided overflow in cee6c47a62c4 (2019) too.  `main` handles overflow in the L2 walk and gets it wrong in the L1 walk, that is the FIXME on `TestFlushAtTheTopOfTheAddressSpace`, and this PR fixes/removes it.

The early return works for the insert path, but not for the flush path.  `Flush` is invalidation, and its only caller is `Process::WriteMemory`, which flushes before it writes.  Not flushing an overflowing range leaves the pre-write bytes cached and the next read hands them back as valid, and it may cause a wrong read value.

https://github.com/llvm/llvm-project/pull/222688