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

[Malea, Daniel]

If we only support one ModuleSpec on Linux, maybe add:

assert(num_specs == 1 && "Linux plugin supports only a single
architecture");

To keep things explicit if/when we run across multi-arch binaries.
Otherwise, the changes look great; feel free to commit at your leisure!


Cheers,
Dan


On 2013-05-15 8:07 PM, "Greg Clayton" <gclayton at apple.com> wrote:

>This looks good. The only thing I would change is change:
>
>if (num_specs >= 1)
>
>to be:
>
>if (num_specs == 1)
>
>in the GetELFProcessCPUType() function. If linux ever supports files that
>contain more than one architecture, we can't just select the first
>architecture slice, we would need to select the correct arch by
>inspecting the process a bit closer. On MacOSX, we have universal files
>that contain 32 and 64 bit copies of the executable, and we need to
>inspect the process to figure out which slice each process uses. On linux
>currently, all ELF files should contain only 1 spec, and the "if
>(num_specs == 1)" will ensure we select the one and only correct one.
>
>Greg
>
>
>On May 15, 2013, at 3:59 PM, Michael Sartain <mikesart at valvesoftware.com>
>wrote:
>
>> On Wed, May 15, 2013 at 2:03 PM, Greg Clayton <gclayton at apple.com>
>>wrote:
>> The correct fix for this is to fill in the GetModuleSpecifications() in
>>ObjectFileELF:
>> 
>> How does the below look? It's definitely cleaner. I stepped through all
>>the code and performance seems more than acceptable as well.
>> 
>> Fire any feedback my way when you get a chance and I'll test it some
>>more and hopefully get this in tomorrow assuming it's all ok.
>> 
>> Thanks!
>>  -Mike
>> 
>> in linux/Host.cpp:
>> 
>> static bool
>> GetELFProcessCPUType (const char *exe_path, ProcessInstanceInfo
>>&process_info)
>> {
>>     // Clear the architecture.
>>     process_info.GetArchitecture().Clear();
>> 
>>     ModuleSpecList specs;
>>     FileSpec filespec (exe_path, false);
>>     const size_t num_specs = ObjectFile::GetModuleSpecifications
>>(filespec, 0, specs);
>>     if (num_specs >= 1)
>>     {
>>         ModuleSpec module_spec;
>>         if (specs.GetModuleSpecAtIndex (0, module_spec) &&
>>module_spec.GetArchitecture().IsValid())
>>         {
>>             process_info.GetArchitecture () =
>>module_spec.GetArchitecture();
>>             // SetArchitecture() in ArchSpec.cpp sets vendor and os to
>>unknown. Reset them to PC and Linux.
>>             process_info.GetArchitecture ().GetTriple().setVendor
>>(llvm::Triple::PC);
>>             process_info.GetArchitecture ().GetTriple().setOS
>>(llvm::Triple::Linux);
>>             return true;
>>         }
>>     }
>>     return false;
>> }
>> 
>> in ELF/ObjectFileELF.cpp:
>> 
>> 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,
>>                                         lldb::DataBufferSP& data_sp,
>>                                         lldb::offset_t data_offset,
>>                                         lldb::offset_t file_offset,
>>                                         lldb::offset_t length,
>>                                         lldb_private::ModuleSpecList
>>&specs)
>> {
>>     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;
>> }
>
>_______________________________________________
>lldb-dev mailing list
>lldb-dev at cs.uiuc.edu
>http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev