[lldb-dev] PATCH for REVIEW: Implement Linux Host::FindProcesses()
Greg Clayton
gclayton at apple.com
Thu May 16 16:05:10 PDT 2013
I agree with what you said, and the patches look good. A few inlined comments below...
On May 16, 2013, at 3:38 PM, Michael Sartain <mikesart at valvesoftware.com> wrote:
> 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?
Yep, the PlatformLinux needs to be able to deal with "unknown unknown" properly which your patch does do.
>
> 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.
The patch looks good.
> 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)
>
More information about the lldb-dev
mailing list