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" <[email protected]> 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 <[email protected]> >wrote: > >> On Wed, May 15, 2013 at 2:03 PM, Greg Clayton <[email protected]> >>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 >[email protected] >http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev _______________________________________________ lldb-dev mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
