Thanks! ObjectFileELF::SetLoadAddress(...) is where I already have a change that got things working, just wasn't sure if I'd be stepping on anything else that relied on the values set there.
Looks like I dropped the dev list in my last message. Adding it back so this is captured. On Wed, Jan 10, 2018 at 1:30 PM, Greg Clayton <clayb...@gmail.com> wrote: > > On Jan 9, 2018, at 4:32 PM, Owen Shaw <l...@owenpshaw.net> wrote: > > Thanks! No problem doing one patch, totally get the reason...thought > it be be easier to review in pieces, but it's not very much even with > everything in. > > I'll work on figuring out the tests before submitting. Seems like > most lldb tests are python, but I feel like these would fall in the > c++ unit tests instead since there's no python API for them. Is that > correct? > > > There are many lldb-server tests around. I would try to do what those tests > are doing. > > > I've actually been working on an llvm-objcopy issue related to virtual > vs physical addresses, and we've become convinced that when different > from virtual addresses, physical addresses should be used when > creating a binary file from an elf, which is analogous to flashing via > lldb. However, in lldb the addresses used by ObjectFile::LoadInMemory > are all from Target::GetSectionLoadList(). Would it cause problems to > put physical addresses in the target's section load list, or should > ObjectFile::LoadInMemory ignore the section load list and do its own > thing? > > > You will want to modify ObjectFileELF::SetLoadAddress(...). It is what loads > an ELF file at a specific location. If the "value" argument is zero and > value_is_offset is true, then it should be easy to figure out where stuff > gets loaded. Since each object file might have its own notion of how it > should be loaded, we should rely on this function if possible. Hopefully > that is what "target modules load --load" does. Then you can use the paddr > as needed when loading. So do the right thing for ELF and it should all work > out. Good to hear you are actively working on what should be in vaddr vs > paddr. > > Greg > > > -Owen > > On Tue, Jan 9, 2018 at 4:15 PM, Greg Clayton <clayb...@gmail.com> wrote: > > > On Jan 9, 2018, at 3:53 PM, Owen Shaw via lldb-dev <lldb-dev@lists.llvm.org> > wrote: > > Looks like there was an addition about a year ago to load an object > file to bare metal target memory via "target modules load --load". > https://reviews.llvm.org/D28804 > > However, this isn't working in my case with openocd and a target that > uses flash memory. By comparison, gdb loads the object just fine, but > instead of using the M memory write command, it uses vFlashErase, > vFlashWrite, and vFlashDone. > > These flash commands don't seem to be supported in lldb. Any reason > not to add them? > > I've got a basic working implementation, which is probably easier to > break into two reviews: > > 1. Add support for qXfer:memory-map and parse the resulting xml to > determine which memory regions are flash and what their blocksizes are > (required for vFlashErase) > 2. Update ProcessGDBRemote::DoWriteMemory to use the flash commands > when writing to a flash region, while continuing to use M when writing > to non-flash regions. > > If this sounds like the right track, I'll open a Phabricator review > for #1 to start. > > > Those two things sound about right. Though we typically don't like to add > support for something if it isn't used. #1 would add support for > "qXfer:memory-map", but nothing would use it. So it might be ok to submit #1 > and #2 as one patch. I am open to suggestions from others if they believe > differently. A patch should have tests for this and I am not sure how to > test it, but just know that we will want some sort of tests. > > > One question I have relates to the load address for object sections. > The current ObjectFile::LoadInMemory implementation will incorrectly > place my .data section at its virtual address in RAM when it really > should be placed at its physical address in flash memory. I have code > that can place it at the physical address, but it's unclear if that > change is correct for every case. Does anyone have insight/opinions? > > > Right now we just load things at the virtual address found in the ELF file. > Not sure if the physical address should be used for everyone as I am not > sure of exactly who is using this right now. Most of the time when I see ELF > files the virtual address is equal to the physical address, so it might just > be ok. Feel free to let us know what you think should happen and submit a > patch. If no one has any tests that break in response to this, then we can > try it and see how things go. > > Greg > > > Thanks, > Owen > _______________________________________________ > lldb-dev mailing list > lldb-dev@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev > > > _______________________________________________ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
