[lldb-dev] PATCH for REVIEW: Implement Linux Host::FindProcesses()

[Michael Sartain]

On Thu, May 16, 2013 at 1:28 PM, Greg Clayton <gclayton at apple.com> wrote:

> > ModuleList::GetShareModule() fails and reports my exe doesn't contain
> "x86_64". Modifying the code in ArchSpec to return Linux isn't the correct
> solution, but what is? Something local here where if OS is Unknown we match
> any OS?
>
> Almost! There is "unknown unknown" (OS is unknown only because it wasn't
> specified), and "specified unknown" (we use this to mean "no OS")).
>
> My patch used this feature:
>
>                if
> (!process_info.GetArchitecture().TripleVendorWasSpecified())
>
>
> If you ask the ArchSpec::GetTriple().getOS(), it might return
> "llvm::Triple::UnknownOS". If the string was actually specified as
> "unknown", then "ArchSpec::GetTriple().getOSName()" will return a non-empty
> string containing "unknown".
>
> Another way to fix this would be to have a "llvm::Triple::NoOS"
> enumeration. There isn't one currently, so we currently test to see if the
> OS string is empty to tell if the OS was specified.
>
> Does that make sense now?


Getting there. :) I want to be really clear, so I'm going to break this up
as this issue involves the ObjectFileELF::GetModuleSpecifications() changes
- we can assume GetProcessCPUTypeFromExecutable() and friends don't even
exist...

Here is the trace of what is happening right now when it fails.

calls TargetList::CreateTarget(), line 63
  - platform_arch defaults to unknown/unknown/unknown
  calls ObjectFile::GetModuleSpecifications()
    - only one arch, so platform_arch is set to it
(x86_64/unknownOS/UnknownEnv)
    calls TargetList::CreateTarget, line 187
      - specified_arch == platform_arch == (x86_64/unknownOS/UnknownEnv)
      calls Platform::ResolveExecutble()
        - exe_arch == (x86_64/unknownOS/UnknownEnv)
        - module_spec is intialized with exe_arch
        if (exe_arch.IsValid()) is true so:
          calls ModuleList::GetSharedModule(module_spec, ...)

Given what you've said, I believe I need to add some code somewhere in that
call chain which checks for triple vendor not being specified. My guess is
it should be something like the below patch?

Notes:
 - I'm separating this from the Linux/Host.cpp changes. This will only be a
patch to implement ObjectFileELF::GetModuleSpecifications().
 - The TripleVendorWasSpecified() checks whether GetVendorName() is empty
and the string "unknown" isn't, so I'm explicitly checking for the Unknown
enum in the below code.

Thanks Greg.
 -Mike

Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===================================================================
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp    (revision 182038)
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp    (working copy)
@@ -222,6 +222,18 @@
     return NULL;
 }

+bool
+ObjectFileELF::MagicBytesMatch (DataBufferSP& data_sp,
+                                  lldb::addr_t data_offset,
+                                  lldb::addr_t data_length)
+{
+    if (data_sp && data_sp->GetByteSize() > (llvm::ELF::EI_NIDENT +
data_offset))
+    {
+        const uint8_t *magic = data_sp->GetBytes() + data_offset;
+        return ELFHeader::MagicBytesMatch(magic);
+    }
+    return false;
+}

 size_t
 ObjectFileELF::GetModuleSpecifications (const lldb_private::FileSpec& file,
@@ -231,7 +243,33 @@
                                         lldb::offset_t length,
                                         lldb_private::ModuleSpecList
&specs)
 {
-    return 0;
+    const size_t initial_count = specs.GetSize();
+
+    if (ObjectFileELF::MagicBytesMatch(data_sp, 0, data_sp->GetByteSize()))
+    {
+        DataExtractor data;
+        data.SetData(data_sp);
+        elf::ELFHeader header;
+        if (header.Parse(data, &data_offset))
+        {
+            if (data_sp)
+            {
+                ModuleSpec spec;
+                spec.GetFileSpec() = file;
+                spec.GetArchitecture().SetArchitecture(eArchTypeELF,
+                                                       header.e_machine,
+
LLDB_INVALID_CPUTYPE);
+                if (spec.GetArchitecture().IsValid())
+                {
+                    // ObjectFileMachO adds the UUID here also, but that
isn't in the elf header
+                    // so we'd have to read the entire file in and
calculate the md5sum.
+                    // That'd be bad for this routine...
+                    specs.Append(spec);
+                }
+            }
+        }
+    }
+    return specs.GetSize() - initial_count;
 }

 //------------------------------------------------------------------
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.h
===================================================================
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.h    (revision 182038)
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.h    (working copy)
@@ -65,6 +65,12 @@
                              lldb::offset_t file_offset,
                              lldb::offset_t length,
                              lldb_private::ModuleSpecList &specs);
+
+    static bool
+    MagicBytesMatch (lldb::DataBufferSP& data_sp,
+                     lldb::addr_t offset,
+                     lldb::addr_t length);
+
     //------------------------------------------------------------------
     // PluginInterface protocol
     //------------------------------------------------------------------
Index: source/Plugins/Platform/Linux/PlatformLinux.cpp
===================================================================
--- source/Plugins/Platform/Linux/PlatformLinux.cpp    (revision 182038)
+++ source/Plugins/Platform/Linux/PlatformLinux.cpp    (working copy)
@@ -208,6 +208,29 @@
                                                  NULL,
                                                  NULL,
                                                  NULL);
+            if (error.Fail())
+            {
+                // If we failed, it may be because the vendor and os
aren't known. If that is the
+                // case, try setting them to the host architecture and
give it another try.
+                llvm::Triple &module_triple =
module_spec.GetArchitecture().GetTriple();
+                bool is_vendor_specified = (module_triple.getVendor() !=
llvm::Triple::UnknownVendor);
+                bool is_os_specified = (module_triple.getOS() !=
llvm::Triple::UnknownOS);
+                if (!is_vendor_specified || !is_os_specified)
+                {
+                    const llvm::Triple &host_triple =
Host::GetArchitecture (Host::eSystemDefaultArchitecture).GetTriple();
+
+                    if (!is_vendor_specified)
+                        module_triple.setVendorName
(host_triple.getVendorName());
+                    if (!is_os_specified)
+                        module_triple.setOSName (host_triple.getOSName());
+
+                    error = ModuleList::GetSharedModule (module_spec,
+                                                         exe_module_sp,
+                                                         NULL,
+                                                         NULL,
+                                                         NULL);
+                }
+            }

             // TODO find out why exe_module_sp might be NULL
             if (!exe_module_sp || exe_module_sp->GetObjectFile() == NULL)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20130516/2873b9bc/attachment.html>