[Lldb-commits] [PATCH] D28804: Provide a substitute to load command of gdb

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

We need to agree on options names and move the object file loading code into ObjectFile.cpp. See inlined comments.



================
Comment at: source/Commands/CommandObjectTarget.cpp:2571
                       "Fullpath or basename for module to load.", ""),
+        m_write_option(LLDB_OPT_SET_1, false, "write-to-mem", 'w',
+                      "Write file contents to the memory and set PC to its"
----------------
How about "--load" witg -l as a short option? "--write-to-mem" seems a bit long. 


================
Comment at: source/Commands/CommandObjectTarget.cpp:2572-2573
+        m_write_option(LLDB_OPT_SET_1, false, "write-to-mem", 'w',
+                      "Write file contents to the memory and set PC to its"
+                      "entry address.",
+                      false, true),
----------------
I am guessing we will have other clients that will want to load more than one image to memory so I am not sure bundling setting the PC should always happen. This might be better as another option: "--set-pc-to-entry" or -p? Then you could load the main image and any other images needed. I know EFI does something similar to this.


================
Comment at: source/Commands/CommandObjectTarget.cpp:2592-2632
+  bool WriteToMemory(CommandReturnObject &result, Module *module) {
+    ObjectFile *objfile = module->GetObjectFile();
+    Target *target = m_interpreter.GetDebugger().GetSelectedTarget().get();
+    Process *process = m_exe_ctx.GetProcessPtr();
+    if (objfile && target && process) {
+      SectionList *section_list = module->GetSectionList();
+      size_t section_count = section_list->GetNumSections(0);
----------------
We should have a Module and ObjectFile method to do this. Move this code there in a function like:
```
lldb_private::Error lldb_private::Module::LoadInMemory(Target &target);
```

This will call through to the ObjectFile method:

```
lldb_private::Error lldb_private::ObjectFile::LoadInMemory(Target &target);
```

We should have each object file subclass override this method as well. When loading and ELF file you load only the program headers into memory as directed by the contents of the file. With mach-o you would load the entire file into memory with the exception of some of the __LINKEDIT section, but again this depends on the contents of the file. So each object file subclass should do this as they see fit. We could have a default implementation do this in ObjectFile.cpp that does what you did above: load the section contents only depending on the section load lists for that module/object file. Then we would let ObjectFileMachO override this method if we ever need to.


https://reviews.llvm.org/D28804