Merge target triple into module triple when constructing module from memory

[xiaobai]

Summary

While debugging an android process remotely from a windows machine, I
noticed that the modules constructed from an object file in memory only had
information about the architecture. Without knowledge of the OS or environment,
expression evaluation sometimes leads to incorrectly generated code or a
debugger crash. While we cannot know for certain what triple a module
constructed from an in-memory object file will have, we can use the triple
from the target to try and fill in the missing details.

[aprantl]

a good guess is the triple that the target executable was compiled for.

What does this do when the executable has multiple slices, such as a Mach-O universal binary?

[xiaobai]

In D58405#1402908, @aprantl wrote:

a good guess is the triple that the target executable was compiled for.

What does this do when the executable has multiple slices, such as a Mach-O universal binary?

Hmm, I hadn't considered that case. I'll test that and see what happens. Thanks for pointing that out.

[clayborg]

source/Target/Process.cpp

2638–2639

Is the MergeFrom in the first part not enough? I am worried about the case where we don't have even an executable, no one has set the architecture on the target, or worse yet, they have set the wrong architecture on the target. We want to correct the architecture on the target if we didn't specify it or the target was wrong. I am worried if we do this here we might hose up things in those cases.

[xiaobai]

source/Target/Process.cpp

2638–2639

MergeFrom is not enough. When debugging an android-aarch64 binary, the triple was just set to aarch64--- for modules constructed from in-memory object files, which is not enough info to do anything meaningful.

However, thinking about this further, MergeFrom might not even be what we want here. Specifically, this is *just* a guess and the information from the in-memory object file is likely more reliable. It would probably better not to merge but to overwrite when information is available.

[clayborg]

source/Target/Process.cpp

2639

So if you have a target that is set to "aarch64-linux-android', and you have "aarch64---" in your object file/module, you would want to merge the missing bits from the target to augment the module's architecture so it grabs the "linux-android" bits. So I do believe MergeFrom should be used to augment the Module's architecture.

[xiaobai]

source/Target/Process.cpp

2639

Ah, yes! Merging from the other direction is definitely what I wanted. Thanks for pointing that out.

[xiaobai]

Updating based on feedback from clayborg

[xiaobai]

Minor change

[aprantl]

Can you think of a way to test this?

[compnerd]

Probably just a replay of GDB protocol packets connected to a testing server would work. But, I don't know if there is infrastructure for that yet.

[aprantl]

Perhaps a unit test?

[labath]

Augmenting the architecture with the target information sounds right to me. If we're reading a module from memory, then we already have a running target, and I consider that information more reliable (since it comes from os/lldb-server) than the object file architecture (which is just random bits in process memory). I might even go so far as to say that if we detect that the module and target ArchSpecs substantially differ (for some meaning of "substantially"), we should reject the attempt to create the module in the first place. Maybe we even do that already at some level?

Unfortunately, I think testing this is going to be hard. The infrastructure for testing in-memory object files is very under-developed. I think the best attempt would be to have a test program which creates some object files in memory (via llvm jit, or perhaps by loading pre-prepared binaries from disk), and then using the gdb jit interface to notify lldb about it. But I don't think we have anything similar to this currently.

In D58405#1402908, @aprantl wrote:

a good guess is the triple that the target executable was compiled for.

What does this do when the executable has multiple slices, such as a Mach-O universal binary?

I don't think this should be an issue since a Module object always points to a specific slice in the fat binary.

[clayborg]

Looks good.

source/Core/Module.cpp

if (process_sp) {
         if (m_objfile_sp) {
           StreamString s;
           s.Printf("0x%16.16" PRIx64, header_addr);
           m_object_name.SetString(s.GetString());
 
           // Once we get the object file, update our module with the object
           // file's architecture since it might differ in vendor/os if some
           // parts were unknown.
-          m_arch = m_objfile_sp->GetArchitecture();
+          m_arch.MergeFrom(m_objfile_sp->GetArchitecture());
         } else {
           error.SetErrorString("unable to find suitable object file plug-in");
         }
       } else {
         error.SetErrorStringWithFormat("unable to read header from memory: %s",
                                        readmem_error.AsCString());
       }
     } else {

source/Target/Process.cpp

 ModuleSP Process::ReadModuleFromMemory(const FileSpec &file_spec,
                                        lldb::addr_t header_addr,
                                        size_t size_to_read) {
   Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
   if (log) {
     log->Printf("Process::ReadModuleFromMemory reading %s binary from memory",
                 file_spec.GetPath().c_str());
   }
-  ModuleSP module_sp(new Module(file_spec, ArchSpec()));
+  ModuleSP module_sp(new Module(
+      file_spec, GetTarget().GetExecutableModule()->GetArchitecture()));
   if (module_sp) {
     Status error;
     ObjectFile *objfile = module_sp->GetMemoryObjectFile(
         shared_from_this(), header_addr, error, size_to_read);
     if (objfile)
       return module_sp;
   }
   return ModuleSP();