[Lldb-commits] [PATCH] D44738: Add a test for setting the load address of a module with differing physical/virtual addresses

owenpshaw accepted this revision. owenpshaw added a comment. This revision is now accepted and ready to land. Looks good to me. Thanks! https://reviews.llvm.org/D44738 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw added a comment. In https://reviews.llvm.org/D42145#1034499, @labath wrote: > I like this a lot. That's the kind of change I wanted to do as a follow-up > one day. Thank you. Thanks! I don't have commit access to land this. https://reviews.llvm.org/D42145 __

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw updated this revision to Diff 137827. owenpshaw added a comment. Looking at LoadInMemory, it seems like the common code is actually more appropriate in the command handler. It's checking inputs and doing the unrelated task of setting the pc. So here's an attempt at going a little fu

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw added a comment. It can be done that way. As I mentioned, it would require moving the Process.h import from ObjectFile.cpp to ObjectFile.h (to get the declaration of Process::WriteEntry). I'm asking how you guys feel about that. Without a clear sense of project norms, I'll always g

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw updated this revision to Diff 137598. owenpshaw added a comment. - Share some common code in LoadInMemory The DoLoadInMemory implementations call target.CalculateProcess() again, but passing Process or Process::WriteEntries to DoLoadInMemory would require either moving the Process.h i

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw reopened this revision. owenpshaw added a comment. Reopening since the previous land was reverted https://reviews.llvm.org/D42145 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw updated this revision to Diff 137420. owenpshaw added a comment. - Revert changes to SetLoadAddress (always use virtual address there) - Override LoadInMemory in ObjectFileELF to just load segments using physical address instead of using section load list Passes tests, but I don't have

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw added a comment. > Since there is just one caller of this function maybe we don't even need to > that fancy. Could the LoadInMemory function do the shifting itself? > I'm thinking of something like where it takes the load bias as the argument > and then adds that to the physical addre

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw added a comment. > And I'm not even completely clear about your case. I understand you write the > module to the physical address, but what happens when you actually go around > to debugging it? Is it still there or does it get copied/mapped/whatever to > the virtual address? In my

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw added inline comments. Comment at: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:814 value = value - header->p_vaddr; found_offset = true; labath wrote: > Ok so the issue is that here we use the virtual address to co

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw added a comment. Thanks! Just a reminder that I don't have commit access, so someone will need to land this for me. Appreciate all the help. https://reviews.llvm.org/D42145 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http:

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw added a comment. @labath, any other changes you'd like to see on this one? Skipping the test if there's no xml support seemed to be the final todo. https://reviews.llvm.org/D42145 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw added inline comments. Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:3478 +auto header = GetProgramHeaderByIndex(i); +if (header->p_paddr != 0) + return true; clayborg wrote: > Can't p_paddr be zero and still be valid? Would a

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw updated this revision to Diff 135087. owenpshaw added a comment. Herald added a subscriber: arichardson. Update patch to include new @skipIfXmlSupportMissing decorator on test https://reviews.llvm.org/D42145 Files: include/lldb/Host/XML.h include/lldb/Target/MemoryRegionInfo.h i

[Lldb-commits] [PATCH] D42959: Rewrite the flaky test_restart_bug test in a more deterministic way

owenpshaw added a comment. Looks good to me https://reviews.llvm.org/D42959 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw updated this revision to Diff 132641. owenpshaw added a comment. - adjust WriteObjectFile signature to return Status and take an std::vector - match project formatting style I toyed with adding allow_flash to as an argument, but didn't really like it because it seemed to bring things b

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw added a comment. Thanks! Overall it's feeling like we're getting down to mainly differences in personal preferences and judgements. Not sure if you're looking for a discussion, which I'm happy to have, if you're just looking for changes to definitely be made. If it's the latter, I

[Lldb-commits] [PATCH] D42195: [lldb] Generic base for testing gdb-remote behavior

owenpshaw added inline comments. Comment at: lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py:335-337 +# However, if we aren't expecting an ack, it's likely the initial +# ack that lldb client sends, and observ

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw updated this revision to Diff 132260. owenpshaw added a comment. - Remove Begin/End memory batch API - Add Process::WriteObjectFile(llvm::ArrayRef) method. Open to other names since it's not necessarily specific to object files, but wanted to somehow indicate that the method may do an

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw added a comment. > How does the flash memory behave from the POV of the debugged process? Just tested on a couple targets and they both silently failed to change the flash memory. So no exception, but no flash write either. Unless there are objections, I'll work on some changes along

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw added a comment. > So the main question is: do we want WriteMemory to work anywhere and always > try to do the right thing, or return an error an the user would be expected > to know to check the memory region you are writing to and know to call > "Process::WriteFlash(...)". I vote to

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw added a comment. This discussion has got me questioning if WriteMemory is even the best place to put the flash commands. It seems like some of the concern here is around the behavior of arbitrary memory writes to flash regions, which isn't really the main objective of this patch (sup

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw added a comment. Thanks. What I'm struggling to reconcile are your statements that users should not have to know how things must happen, but then that we should make ObjectFile::Load smart so it doesn't result in an unnecessary amount of reads/writes. If writing to flash memory effe

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw added a comment. Hi Greg, I got distracted from this one for a bit. Maybe I missed it, but do you have any thoughts on my previous comment/question about the batch API vs other options? Thanks, Owen https://reviews.llvm.org/D42145 ___

[Lldb-commits] [PATCH] D42195: [lldb] Generic base for testing gdb-remote behavior

owenpshaw added a comment. Thanks for the help! Someone with commit access will need to land it. https://reviews.llvm.org/D42195 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D42195: [lldb] Generic base for testing gdb-remote behavior

owenpshaw updated this revision to Diff 131310. owenpshaw added a comment. - Updated condition for yaml2obj dependency, now based on LLDB_BUILT_STANDALONE - Find yaml2obj in the same directory as clang - Move yaml2obj code into lldbtest.py so it's available to all tests Is the findBuiltClang func

[Lldb-commits] [PATCH] D42195: [lldb] Generic base for testing gdb-remote behavior

owenpshaw updated this revision to Diff 130920. owenpshaw added a comment. Added back socket close() It looks like the yaml2obj target hasn't been defined yet when the check-lldb target is defined, so if(TARGET yaml2obj) always returns false. Is there another way to do the conditional dependen

[Lldb-commits] [PATCH] D42195: [lldb] Generic base for testing gdb-remote behavior

owenpshaw updated this revision to Diff 130643. owenpshaw added a comment. Herald added a subscriber: mgorny. - Updated to use TestBase.process() instead of a new member - Added yaml2obj dependency. It it okay to be an unconditional dependency? - I can put the socket close() back in, but had remo

[Lldb-commits] [PATCH] D42195: [lldb] Generic base for testing gdb-remote behavior

owenpshaw updated this revision to Diff 130472. owenpshaw added a comment. Updated with suggestions - Using process.Kill() to close client connection - Refactored server._receive - renamed elf-specific functions to be generic https://reviews.llvm.org/D42195 Files: packages/Python/lldbsuite/

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw added a comment. I'm not envisioning that anything else needs to change to use begin/end or care it's there. I guess the way I look at it, having ObjectFile::LoadInMemory do begin/end is basically the same as what you're saying about having it be more process aware. If Process is go

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw updated this revision to Diff 130258. owenpshaw added a comment. - Split testing base into separate review https://reviews.llvm.org/D42195 - Use StreamGDBRemote instead of duplicate new function - Move qXfer:memory-map xml parsing into GDBRemoteCommunicationClient - Include qXfer:memory-

[Lldb-commits] [PATCH] D42195: [lldb] Generic base for testing gdb-remote behavior

owenpshaw created this revision. owenpshaw added reviewers: clayborg, labath. Adds new utilities that make it easier to write test cases for lldb acting as a client over a gdb-remote connection. - A GDBRemoteTestBase class that starts a mock GDB server and provides an easy way to check client p

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw added a comment. Thanks for the feedback, I'm working on splitting out the testing base into another patch and making the requested changes. My main unresolved question is about the write batching, with details below. Comment at: include/lldb/Target/Process.h:1916

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

owenpshaw created this revision. owenpshaw added reviewers: clayborg, labath. Herald added subscribers: mgorny, emaste. When writing an object file over gdb-remote, use the vFlashErase, vFlashWrite, and vFlashDone commands if the write address is in a flash memory region. A bare metal target ma

[Lldb-commits] [PATCH] D41745: Handle O reply packets during qRcmd

owenpshaw added a comment. Thanks! I don't have commit access, so someone needs to commit this for me, right? https://reviews.llvm.org/D41745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/

[Lldb-commits] [PATCH] D41745: Handle O reply packets during qRcmd

owenpshaw updated this revision to Diff 128991. owenpshaw added a comment. Updated patch with new function name suggested by @clayborg. Added unit test and changed to llvm::function_ref as suggested by @labath. Based on Greg's comments in the other thread, I've kept the new function separate,

[Lldb-commits] [PATCH] D41745: Handle O reply packets during qRcmd

owenpshaw created this revision. owenpshaw added a reviewer: clayborg. Gdb servers like openocd may send many $O reply packets for the client to output during a qRcmd command sequence. Currently, lldb interprets the first O packet as an unexpected response. Besides generating no output, this c