[Lldb-commits] [lldb] r182065 - Implement ObjectFileELF::GetModuleSpecifications(), and add PlatformLinux code to deal with unknown arch properties.

[Malea, Daniel]

Hi Mike, thanks for root-causing this issue. I studied the code in
question a little and my comments inline below:

On 2013-05-17 12:58 AM, "Mike Sartain" <mikesart at valvesoftware.com> wrote:

>The below patch will fix up the vendor & os in
>Target::SetExecutableModule() if either are unknown. I've got a couple of
>questions / concerns though:
>- The code below will run on all platforms. That scares me.

I don't think this is a problem because nothing the patch does in common
code is OS-specific. If Target just fills in the blanks/unknowns, after
the executable has been fully parsed, I don't think it will cause any
problems. I agree that it's not pretty though :)

The only alternative I see is to beef up ObjectFileELF to read more than
just the ELF header. Some quick inspection of some binaries with "readelf
-a" seems to show the OS is encoded somewhere in the 'notes' section of
the binary. I'm not sure what the overhead of reading those sections are,
or if we want to go that far when we're just doing a process list. There
might be another way to determine the Vendor/OS (the 'file' command seems
to do it, so maybe there's a better way?) but I'm not sure it would be
worth the trouble IMHO.

>- This is the 2nd spot where we have to fix this arch stuff up for Linux.
>This scares me a bit as well.
>How big of a can of worms are we opening by returning
>x86_64-unknown-unknown for elf files (since that's all the info we have
>in ObjectFileELF), and then expect everyone to fix up the vendor/os later
>on?
>

I'd like to think this is the only place we might have to do something
like this. I believe (though not sure, maybe Apple guys can confirm?) that
once the Target is initialized, everything that needs to query the
architecture/vendor/OS should just use the target. As such I think your
fix in Target.cpp is OK.


Cheers,
Dan