[lldb-dev] How to handle "O XX..." reply packets from gdb servers like openocd
Looking for guidance. This seems like an lldb issue, but maybe I'm missing something. And while I have a potential solution, I'm not familiar enough with lldb code to know if its a "good" solution. Background -- openocd sends mutliple "O XX..." reply packets (https://sourceware.org/gdb/onlinedocs/gdb/Stop-Reply-Packets.html) in response to certain commands. The XX... hex payload should be decoded and printed for the lldb user. For example, (lldb) process plugin packet monitor help Results in a gdb client/server packet sequence like: -> $qRcmd,68656c70#fc <- $O#4f <- $O616461707465725f6b687a#66 <- $O5b6b687a5d0a#ae ... (many more $O packets) <- $OK#9a and should print in the lldb console something like: adapter [khz] ... (many more lines) The Issues -- 1) lldb assumes the first O packet is the command response, so it thinks the command failed since O is an unknown response 2) lldb only reads the first O packet, causing subsequent commands to read the remaining O reply packets from the first command as their responses 3) lldb doesn't print the O packet payloads as expected Possible Solution - I've coded up a possible solution: 1) Add StringExtractorGDBRemote::IsOutputResponse() that returns true for O packets 2) Have GDBRemoteCommunication::ReadPacket() check IsOutputResponse() as it reads 3) If ReadPacket() sees an O packet, print its payload and read another packet. Keep looping and return the first non-O packet. 4) Add an output stream member to GDBRemoteCommunication so it can print to the lldb console Seems to work. I see the expected command output in the lldb console, the command succeeds, and subsequent commands read the correct responses. If this is anywhere near the right track, I can look into writing test cases and submitting a patch for closer evaluation. Thanks, Owen ___ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
Re: [lldb-dev] How to handle "O XX..." reply packets from gdb servers like openocd
I dug into this a bit more, and these output reply packets seem to be handled already, but only if the program is running. Since the relevant openocd commands are often issued when the program paused, the reply packets aren't processed as expected. The spec does say that reply packets can happen "any time while the program is running", so perhaps openocd is abusing the protocol, but gdb handles the packets just fine when stopped. It's unclear in which other cases these packets would arrive when the program is stopped, so I've put together a narrowed solution: 1. Adding GDBRemoteClientBase::SendPacketHandlingOutput(), with a callback argument to handle any output 2. Calling SendPacketHandlingOutput() from CommandObjectProcessGDBRemotePacketMonitor, providing a callback lamda that uses an already-available output stream It's a simpler and less intrusive solution, but will only apply to "process plugin packet monitor" commands. Perhaps that's sufficient. Any thoughts still appreciated. Thanks, Owen On Fri, Dec 29, 2017 at 11:37 AM, Owen Shaw wrote: > Looking for guidance. This seems like an lldb issue, but maybe I'm > missing something. And while I have a potential solution, I'm not > familiar enough with lldb code to know if its a "good" solution. > > Background > -- > > openocd sends mutliple "O XX..." reply packets > (https://sourceware.org/gdb/onlinedocs/gdb/Stop-Reply-Packets.html) in > response to certain commands. The XX... hex payload should be decoded > and printed for the lldb user. > > For example, > > (lldb) process plugin packet monitor help > > Results in a gdb client/server packet sequence like: > > -> $qRcmd,68656c70#fc > <- $O#4f > <- $O616461707465725f6b687a#66 > <- $O5b6b687a5d0a#ae > ... (many more $O packets) > <- $OK#9a > > and should print in the lldb console something like: > > adapter [khz] > ... (many more lines) > > > The Issues > -- > > 1) lldb assumes the first O packet is the command response, so it > thinks the command failed since O is an unknown response > 2) lldb only reads the first O packet, causing subsequent commands to > read the remaining O reply packets from the first command as their > responses > 3) lldb doesn't print the O packet payloads as expected > > > Possible Solution > - > > I've coded up a possible solution: > > 1) Add StringExtractorGDBRemote::IsOutputResponse() that returns true > for O packets > 2) Have GDBRemoteCommunication::ReadPacket() check IsOutputResponse() > as it reads > 3) If ReadPacket() sees an O packet, print its payload and read > another packet. Keep looping and return the first non-O packet. > 4) Add an output stream member to GDBRemoteCommunication so it can > print to the lldb console > > Seems to work. I see the expected command output in the lldb console, > the command succeeds, and subsequent commands read the correct > responses. > > If this is anywhere near the right track, I can look into writing test > cases and submitting a patch for closer evaluation. > > Thanks, > Owen ___ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
Re: [lldb-dev] How to handle "O XX..." reply packets from gdb servers like openocd
Hi Greg, I've been assuming that we can't force a change to openocd's behavior, or at least that the easier path is to handle its current behavior. I don't know why openocd sends O packets in response to an rCmd, but it does and gdb is okay with it. I'm still learning my way around the lldb code, but it looked like the continue scenario has its own function, SendContinuePacketAndWaitForResponse(), and with continue-specific logic, there didn't seem to be an obvious way to leverage that code or to have the continue commands share some new code. I have a working option coded up, and can easily throw a patch up in Phabricator if it'd be helpful to discuss the details. Let me know how you'd like to proceed. Thanks, Owen On Wed, Jan 3, 2018 at 11:11 AM, Greg Clayton wrote: > >> On Jan 1, 2018, at 6:30 PM, Owen Shaw via lldb-dev >> wrote: >> >> I dug into this a bit more, and these output reply packets seem to be >> handled already, but only if the program is running. > >> Since the relevant openocd commands are often issued when the program >> paused, the reply packets aren't processed as expected. >> >> The spec does say that reply packets can happen "any time while the >> program is running", so perhaps openocd is abusing the protocol, but >> gdb handles the packets just fine when stopped. > > Yes, LLDB is assuming that an "O" packet will only come during a continue > command. "O" is for stdout output and it seems the rCmd assuming it will > work. Why not just send back the text in response to the rCmd? Is some common > code path being hit where it might send this text while running and also in > response to the rCmd? I am confused by "O" packets are needed in response to > the rCmd. > >> >> It's unclear in which other cases these packets would arrive when the >> program is stopped, so I've put together a narrowed solution: >> >> 1. Adding GDBRemoteClientBase::SendPacketHandlingOutput(), with a >> callback argument to handle any output >> 2. Calling SendPacketHandlingOutput() from >> CommandObjectProcessGDBRemotePacketMonitor, providing a callback lamda >> that uses an already-available output stream >> >> It's a simpler and less intrusive solution, but will only apply to >> "process plugin packet monitor" commands. Perhaps that's sufficient. >> >> Any thoughts still appreciated. > > If the rCmd can't be fixed to just return the text without using "O" packets, > I would think a simpler solution might be to change the response packet code > to take a boolean that says "handle_output". Then only the continue (c, s, > vCont, etc) packets would set this to true, and the "rCmd" command could set > this to true. This bool would need to be plumbed through to the > SendPacketAndWaitForResponse(...) functions. We could have a default argument > set to false so most code won't change, just the continue and rCmd packet > senders. > > Greg > >> >> Thanks, >> Owen >> >> On Fri, Dec 29, 2017 at 11:37 AM, Owen Shaw wrote: >>> Looking for guidance. This seems like an lldb issue, but maybe I'm >>> missing something. And while I have a potential solution, I'm not >>> familiar enough with lldb code to know if its a "good" solution. >>> >>> Background >>> -- >>> >>> openocd sends mutliple "O XX..." reply packets >>> (https://sourceware.org/gdb/onlinedocs/gdb/Stop-Reply-Packets.html) in >>> response to certain commands. The XX... hex payload should be decoded >>> and printed for the lldb user. >>> >>> For example, >>> >>> (lldb) process plugin packet monitor help >>> >>> Results in a gdb client/server packet sequence like: >>> >>> -> $qRcmd,68656c70#fc >>> <- $O#4f >>> <- $O616461707465725f6b687a#66 >>> <- $O5b6b687a5d0a#ae >>> ... (many more $O packets) >>> <- $OK#9a >>> >>> and should print in the lldb console something like: >>> >>> adapter [khz] >>> ... (many more lines) >>> >>> >>> The Issues >>> -- >>> >>> 1) lldb assumes the first O packet is the command response, so it >>> thinks the command failed since O is an unknown response >>> 2) lldb only reads the first O packet, causing subsequent commands to >>> read the remaining O reply packets from the first command as their >>> responses >>
[lldb-dev] Loading object file to target flash memory using vFlashWrite
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. 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? Thanks, Owen ___ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
Re: [lldb-dev] Loading object file to target flash memory using vFlashWrite
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 wrote: > > On Jan 9, 2018, at 4:32 PM, Owen Shaw 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 wrote: > > > On Jan 9, 2018, at 3:53 PM, Owen Shaw via lldb-dev > 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
