jasonmolenda added a comment.

I'm not super convinced on the setting.  It seems like something you'd add some 
people could work around a mistake we've made here.

I think we have two cases:

Someone calls Target::ReadMemory prefer_file_cache=true on a writable section.  
This is an easy mistake for people to make, they assume prefer_file_cache will 
only prefer the file cache on read-only Sections.

Someone reads Target::ReadMemory with prefer_file_cache=false on a read-only 
section.  This is *often* a mistake, but there are times where we genuinely 
want to see raw memory.

Let's think about what the default behavior of Target::ReadMemory should be. 
The majority use case is that we prefer the file cache if it is a read-only 
section.  It's a performance bug in almost every case to behave differently, I 
maintain.  There are times when we want to show actual raw memory to people.  
Some naughty programs that manage to modify themselves, we need to see that 
real naughtiness, not show what the file had and pretend that is what is in 
memory.

I think Target::ReadMemory should have a force_live_memory argument, not a 
prefer_file_cache.  It could be the final argument with a default value of 
force_live_memory==false.  Almost everyone should let Target::ReadMemory check 
if this address is in a RO-section, and let it get the data from the file cache 
if so.  If it's not in a RO section, or any section, read from live memory.  
And have the force_live_memory arg to override this.

This is a larger set of changes though, and I don't mean to sign anyone up for 
more than they intended.  But, just thinking of this from a clean slate, I 
think that's the better design.  Adrian, what do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100338

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to