<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, May 16, 2013 at 10:39 AM, Malea, Daniel <span dir="ltr"><<a href="mailto:daniel.malea@intel.com" target="_blank">daniel.malea@intel.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">If we only support one ModuleSpec on Linux, maybe add:<br>
<br>
assert(num_specs == 1 && "Linux plugin supports only a single<br>
architecture");<br></blockquote><div><br></div><div>Yeah, that makes me feel more comfortable with that "== 1" check. It's in.<br></div><div> <br></div></div>I ran into an issue where TargetList::CreateTarget() was failing because it's calling ObjectFile::GetModuleSpecifications() and getting back x86_64 / UnknownVendor / UnknownOS and is expecting to find the Linux for the OS when it calls IsCompatibleMatch().<br>
<br></div><div class="gmail_extra">So I modified ArchSpec::SetArchitecture() to set the Linux OS using an ifdef in ArchSpec.cpp. Is this the best way to do this?<br><br></div><div class="gmail_extra">I also got rid of the llvm::Triple::PC for vendor. This just seemed wrong as Linux can run on all kinds of platforms. I did write some code to get the distributor name from /etc/lsb-release, but the llvm triple doesn't line up with anything there of course. So for now it spits out "x86_64-unknown-linux".<br>
<br>The final patch is down below. Let me know if this is all ok and I'll submit.<br><br>Thanks again for all the help / feedback on everything.<br></div><div class="gmail_extra"> -Mike<br><br>Index: source/Host/linux/Host.cpp<br>
===================================================================<br>--- source/Host/linux/Host.cpp (revision 182033)<br>+++ source/Host/linux/Host.cpp (working copy)<br>@@ -18,6 +18,8 @@<br> // C++ Includes<br> // Other libraries and framework includes<br>
// Project includes<br>+#include "llvm/Support/ELF.h"<br>+<br> #include "lldb/Core/Error.h"<br> #include "lldb/Target/Process.h"<br> <br>@@ -25,6 +27,9 @@<br> #include "lldb/Core/DataBufferHeap.h"<br>
#include "lldb/Core/DataExtractor.h"<br> <br>+#include "lldb/Core/ModuleSpec.h"<br>+#include "lldb/Symbol/ObjectFile.h"<br>+<br> using namespace lldb;<br> using namespace lldb_private;<br> <br>
@@ -292,6 +297,29 @@<br> }<br> <br> static bool<br>+GetELFProcessCPUType (const char *exe_path, ProcessInstanceInfo &process_info)<br>+{<br>+ // Clear the architecture.<br>+ process_info.GetArchitecture().Clear();<br>
+<br>+ ModuleSpecList specs;<br>+ FileSpec filespec (exe_path, false);<br>+ const size_t num_specs = ObjectFile::GetModuleSpecifications (filespec, 0, specs);<br>+ assert(num_specs == 1 && "Linux plugin supports only a single architecture");<br>
+ if (num_specs == 1)<br>+ {<br>+ ModuleSpec module_spec;<br>+ if (specs.GetModuleSpecAtIndex (0, module_spec) && module_spec.GetArchitecture().IsValid())<br>+ {<br>+ process_info.GetArchitecture () = module_spec.GetArchitecture();<br>
+ return true;<br>+ }<br>+ }<br>+ return false;<br>+}<br>+<br>+<br>+static bool<br> GetProcessAndStatInfo (lldb::pid_t pid, ProcessInstanceInfo &process_info, ProcessStatInfo &stat_info, lldb::pid_t &tracerpid)<br>
{<br> tracerpid = 0;<br>@@ -299,9 +327,6 @@<br> ::memset (&stat_info, 0, sizeof(stat_info));<br> stat_info.ppid = LLDB_INVALID_PROCESS_ID;<br> <br>- // Architecture is intentionally omitted because that's better resolved<br>
- // in other places (see ProcessPOSIX::DoAttachWithID().<br>-<br> // Use special code here because proc/[pid]/exe is a symbolic link.<br> char link_path[PATH_MAX];<br> char exe_path[PATH_MAX] = "";<br>
@@ -323,6 +348,10 @@<br> {<br> exe_path[len - deleted_len] = 0;<br> }<br>+ else<br>+ {<br>+ GetELFProcessCPUType (exe_path, process_info);<br>+ }<br> <br> process_info.SetProcessID(pid);<br>
process_info.GetExecutableFile().SetFile(exe_path, false);<br>Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.h<br>===================================================================<br>--- source/Plugins/ObjectFile/ELF/ObjectFileELF.h (revision 182033)<br>
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.h (working copy)<br>@@ -65,6 +65,12 @@<br> lldb::offset_t file_offset,<br> lldb::offset_t length,<br> lldb_private::ModuleSpecList &specs);<br>
+<br>+ static bool<br>+ MagicBytesMatch (lldb::DataBufferSP& data_sp,<br>+ lldb::addr_t offset, <br>+ lldb::addr_t length);<br>+<br> //------------------------------------------------------------------<br>
// PluginInterface protocol<br> //------------------------------------------------------------------<br>Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp<br>===================================================================<br>
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp (revision 182033)<br>+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp (working copy)<br>@@ -222,6 +222,18 @@<br> return NULL;<br> }<br> <br>+bool<br>+ObjectFileELF::MagicBytesMatch (DataBufferSP& data_sp,<br>
+ lldb::addr_t data_offset,<br>+ lldb::addr_t data_length)<br>+{<br>+ if (data_sp && data_sp->GetByteSize() > (llvm::ELF::EI_NIDENT + data_offset))<br>
+ {<br>+ const uint8_t *magic = data_sp->GetBytes() + data_offset;<br>+ return ELFHeader::MagicBytesMatch(magic);<br>+ }<br>+ return false;<br>+}<br> <br> size_t<br> ObjectFileELF::GetModuleSpecifications (const lldb_private::FileSpec& file,<br>
@@ -231,7 +243,33 @@<br> lldb::offset_t length,<br> lldb_private::ModuleSpecList &specs)<br> {<br>- return 0;<br>+ const size_t initial_count = specs.GetSize();<br>
+ <br>+ if (ObjectFileELF::MagicBytesMatch(data_sp, 0, data_sp->GetByteSize()))<br>+ {<br>+ DataExtractor data;<br>+ data.SetData(data_sp);<br>+ elf::ELFHeader header;<br>+ if (header.Parse(data, &data_offset))<br>
+ {<br>+ if (data_sp)<br>+ {<br>+ ModuleSpec spec;<br>+ spec.GetFileSpec() = file;<br>+ spec.GetArchitecture().SetArchitecture(eArchTypeELF,<br>+ header.e_machine,<br>
+ LLDB_INVALID_CPUTYPE);<br>+ if (spec.GetArchitecture().IsValid())<br>+ {<br>+ // ObjectFileMachO adds the UUID here also, but that isn't in the elf header<br>
+ // so we'd have to read the entire file in and calculate the md5sum.<br>+ // That'd be bad for this routine...<br>+ specs.Append(spec);<br>+ }<br>
+ }<br>+ }<br>+ }<br>+ return specs.GetSize() - initial_count;<br> }<br> <br> //------------------------------------------------------------------<br>Index: source/Core/ArchSpec.cpp<br>===================================================================<br>
--- source/Core/ArchSpec.cpp (revision 182033)<br>+++ source/Core/ArchSpec.cpp (working copy)<br>@@ -692,7 +692,11 @@<br> else<br> {<br> m_triple.setVendor (llvm::Triple::UnknownVendor);<br>
+#if defined(__linux__) <br>+ m_triple.setOS (llvm::Triple::Linux);<br>+#else<br> m_triple.setOS (llvm::Triple::UnknownOS);<br>+#endif<br> }<br> // Fall back onto setting the machine type if the arch by name failed...<br>
if (m_triple.getArch () == llvm::Triple::UnknownArch)<br><br></div></div>