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

[Greg Clayton]

On May 16, 2013, at 1:09 PM, Michael Sartain <mikesart at valvesoftware.com> wrote:

> On Thu, May 16, 2013 at 11:50 AM, Greg Clayton <gclayton at apple.com> wrote:
> 
> On May 16, 2013, at 11:32 AM, Michael Sartain <mikesart at valvesoftware.com> wrote:
> 
> > On Thu, May 16, 2013 at 10:39 AM, Malea, Daniel <daniel.malea at intel.com> wrote:
> > If we only support one ModuleSpec on Linux, maybe add:
> >
> > assert(num_specs == 1 && "Linux plugin supports only a single
> > architecture");
> >
> > Yeah, that makes me feel more comfortable with that "== 1" check. It's in.
> >
> > 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().
> >
> > So I modified ArchSpec::SetArchitecture() to set the Linux OS using an ifdef in ArchSpec.cpp. Is this the best way to do this?
> 
> No, I would do this by modifying all the architectures in ModuleSpec objects that are returned from ObjectFile::GetModuleSpecifications() inside GetELFProcessCPUType().
> 
> Sorry I wasn't clear - this issue has nothing to do with the code in Linux/Host.cpp. It's showing up because we've now implemented ObjectFileELF::GetModuleSpecifications().
> 
> It's returning x86_64 / UnknownVendor / UnknownOS for this target I'm trying to debug where it used to return nothing. Ie, invalid arch.

Again, this arch for the target comes from the main executable in the target. The main executable's arch is filled in from getting the process info right? So if we fix the process info so that the arch info is correct (it should return "x86_64-pc-linux" or whatever the llvm target triple is supposed to be for linux).

> 
> So in PlatformLinux.cpp, we are now running the exe_arch.IsValid() block down below (used to take the else clause).
> 
> m_arch in module_spec is:
>     Arch = llvm::Triple::x86_64,
>     Vendor = llvm::Triple::UnknownVendor,
>     OS = llvm::Triple::Linux,   
>     Environment = llvm::Triple::GNU
> exe_arch is:
>     Arch = llvm::Triple::x86_64,
>     Vendor = llvm::Triple::UnknownVendor,
>     OS = llvm::Triple::UnknownOS,
>     Environment = llvm::Triple::UnknownEnvironment
> 
> 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? So you can use this in your code to with

if (!arch.TripleVendorWasSpecified())
{
    // Match any vendor because it wasn't specified
}

if (!arch.TripleOSWasSpecified())
{
    // Match any OS because it wasn't specified
}

I believe the comparison functions in ArchSpec do this (see ArchSpec::IsEqualTo()).



> Thanks.
> 
>     if (error.Success())
>     {
>         ModuleSpec module_spec (resolved_exe_file, exe_arch);
>         if (exe_arch.IsValid())
>         {
>             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)
>             {
>                 exe_module_sp.reset();
>                 error.SetErrorStringWithFormat ("'%s' doesn't contain the architecture %s",
>                                                 exe_file.GetPath().c_str(),
>                                                 exe_arch.GetArchitectureName());
>             }
>         }
>         else
>         {
>             // No valid architecture was specified, ask the platform for
>             // the architectures that we should be using (in the correct order)
>             // and see if we can find a match that way
>             StreamString arch_names;
>             for (uint32_t idx = 0; GetSupportedArchitectureAtIndex (idx, module_spec.GetArchitecture()); ++idx)
>             {
>                 error = ModuleList::GetSharedModule (module_spec, 
>                                                      exe_module_sp, 
>                                                      NULL, 
>                                                      NULL,
>                                                      NULL);
>                 // Did we find an executable using one of the 
>                 if (error.Success())
>                 {
>                     if (exe_module_sp && exe_module_sp->GetObjectFile())
>                         break;
>                     else
>                         error.SetErrorToGenericError();
>                 }
>                 
>                 if (idx > 0)
>                     arch_names.PutCString (", ");
>                 arch_names.PutCString (module_spec.GetArchitecture().GetArchitectureName());
>             }
>             
>             if (error.Fail() || !exe_module_sp)
>             {
>                 error.SetErrorStringWithFormat ("'%s' doesn't contain any '%s' platform architectures: %s",
>                                                 exe_file.GetPath().c_str(),
>                                                 GetPluginName().GetCString(),
>                                                 arch_names.GetString().c_str());
>             }
>         }
>     }
>