clayborg added inline comments.
================
Comment at: include/lldb/Target/Process.h:1916
+ //------------------------------------------------------------------
+ virtual bool BeginWriteMemoryBatch() { return true; }
+
----------------
owenpshaw wrote:
> clayborg wrote:
> > labath wrote:
> > > Instead of this begin/end combo, what would you think about a WriteMemory
> > > overload that takes a list of memory regions?
> > > Something like:
> > > ```
> > > struct WriteEntry { addr_t Dest; llvm::ArrayRef<uint8_t> Contents; };
> > > Status WriteMemory(llvm::ArrayRef<WriteEntry>);
> > > ```
> > > Then the default implementation can just lower this into regular
> > > `WriteMemory` sequence, and the gdb-remote class can add the extra
> > > packets around it when needed.
> > Do we really need this? Can't we just take care of it inside of the
> > standard Process::WriteMemory()? This doesn't seem like something that a
> > user should have to worry about. The process plug-in should just make the
> > write happen IMHO. Let me know.
> Maybe I'm misunderstanding the flash commands, but I couldn't figure a way
> around the need to somehow indicate that several writes are batched together.
> The reason has to do with how vFlashErase must erase an entire block at a
> time.
>
> ObjectFile::LoadInMemory makes one Process::WriteMemory call per section. If
> each WriteMemory call was self-contained, and two sections are in the same
> flash block, it would go something like this:
>
> # WriteMemory (section 1)
> ## DoWriteMemory
> ### vFlashErase (block 1)
> ### vFlashWrite (section 1)
> ### vFlashDone
> # WriteMemory (section 2)
> ## DoWriteMemory
> ### vFlashErase (block 1, again)
> ### vFlashWrite (section 2)
> ### vFlashDone
>
> Wouldn't the second erase undo the first write?
>
> I found the begin/end to be the least intrusive way around, but I'm open to
> other options.
Seems like any block write that isn't writing an entire block should read the
contents that won't be overwritten, create a block's worth of data to write by
using the pre-existing data and adding any new data, then erase the block and
and rewrite the entire block. Then in ObjectFile::Load() it would batch up any
consecutive writes to minimize any block erasing...
================
Comment at: source/Symbol/ObjectFile.cpp:671
+ continue;
+ // We can skip sections like bss
+ if (section_sp->GetFileSize() == 0)
----------------
owenpshaw wrote:
> clayborg wrote:
> > labath wrote:
> > > You're not actually changing this part, but it caught my eye, and you may
> > > care about this:
> > >
> > > Is it true that we can ignore .bss here? Programs generally assume that
> > > .bss is zero-initialized, and if you just ignore it here, it can contain
> > > whatever garbage was in the memory before you loaded (?)
> > Very important to zero out bss
> For the embedded projects I've worked on, ignoring .bss here is fine and
> expected because the code always starts by zeroing out the .bss memory (and
> also copying the .data section contents from ROM to RAM, which is related to
> the physical address change).
>
> For comparison, gdb doesn't try to load the bss section either.
Sounds good, if gdb doesn't do it, then I am fine if we don't do it
https://reviews.llvm.org/D42145
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
