[Lldb-commits] [PATCH] D31280: [LLDB][MIPS] Fix Core file Architecture and OS information

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Let me know what you think about the MergeFrom comment. I am generally ok with this, but just wanted to check in case the merge made any sense in this patch somewhere.



================
Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:220
+  // information. Hence we need to relied on target architecture for that.
+  if (arch.IsValid() && !arch.IsMIPS())
+    GetTarget().SetArchitecture(arch);
----------------
nitesh.jain wrote:
> labath wrote:
> > Here you copy the architecture from core to target.
> Actually we are skipping it , if the arch is MIPS ( ! arch.IsMIPS()).  
So the target has an incomplete architecture here? If that is true, you should probably get the architecture and merge it. You can merge any missing info with:

```
ArchSpec target_arch = GetTarget().GetArchitecture();
ArchSpec core_arch(m_core_module_sp->GetArchitecture());
target_arch.MergeFrom(core_arch);
GetTarget().SetArchitecture(target_arch);
```

Not sure how that would work here, but we should be using this. If we have a target whose arch is "x86_64-apple-macosx" and you have a core file that is "mips-..." an error should be produced.


https://reviews.llvm.org/D31280