[lldb-dev] Target::ReadMemory query

[Greg Clayton]

> On Jun 11, 2014, at 1:35 AM, Matthew Gardiner <mg11 at csr.com> wrote:
> 
> Greg Clayton wrote:
>> Your problem seems to be that ELF files all claim to have the host OS and vendor:
> 
> Yes, indeed.
>> bool
>> ObjectFileELF::GetArchitecture (ArchSpec &arch)
>> {
>>     if (!ParseHeader())
>>         return false;
> 
> However, in my working copy (only a week old), GetArchitecture's logic is duplicated by GetModuleSpecifications, and upon invoking "target create" my lldb ends up using the host's specification:
> 
> ObjectFileELF::GetModuleSpecifications (const lldb_private::FileSpec& file,
> ...
> {
> <snip>
> 
>    if (spec.GetArchitecture().IsValid())
>    {
>       // We could parse the ABI tag information ...
> ==>spec.GetArchitecture().GetTriple().setOSName (Host::GetOSString().GetCString());
> 
> But regardless of this, i.e. ObjectFileELF::Get* returning unreliable OS type at "target create", I think the problem is when the target's inferior is first attached, particularly in the case of embedded targets modeled by "ProcessGDBRemote", the target architecture is not correctly "adjusted" when the qHostInfo/qProcessInfo are received from the stub (why have the messages if they are not acted on?).
> 
> In ProcessGDBRemote::DoConnectRemote, we have:
> 
> if (!m_target.GetArchitecture().IsValid())
> {
>    if (m_gdb_comm.GetProcessArchitecture().IsValid())
>    {
> m_target.SetArchitecture(m_gdb_comm.GetProcessArchitecture());
>    }
>    else
>    {
>         m_target.SetArchitecture(m_gdb_comm.GetHostArchitecture());
>    }
> }
> 
> So in my case, since my target's architecture is already "valid" (i.e. m_core is defined, etc.), the DoConnectRemote code doesn't consider the stub's opinion on the target. Surely in the remote/embedded case we must trust the stub's host info if supplied?

Not necessarily. A host can be "x86_64-apple-macosx", yet if we are debugging an iOS simulator app we will get "x86_64-apple-ios" as the process triple. So really we want to really trust the process info over the host info.


> In my opinion, this is the cause of a lot of my problems.
> 
> However, if I comment out "if (!m_target.GetArchitecture().IsValid())" and allow the SetArchitectures to proceed, I still run into problems, since:
> 
> bool
> Target::SetArchitecture (const ArchSpec &arch_spec)
> {
> ...
>           m_arch = arch_spec;
> ....
> ==>    SetExecutableModule (executable_sp, true);
> 
> That is, the arch_spec my gdb-remote passes to SetArchitecture, is firstly assigned to m_arch, but is then overwritten by SetExecutableModule.
> 
> It turns out that SetExecutableModule overwrites the archspec supplied by my stub, since
> 
> void
> Target::SetExecutableModule
> {
>        ....
>        if (!m_arch.IsValid())
>        {
>        ==>m_arch = executable_sp->GetArchitecture();
> 
> !m_arch.IsValid() occurred, seemingly, because my stub was not setting cputype. However, when I setup the "cputype" in qHostInfo, more problems arise:
> 
> bool
> GDBRemoteCommunicationClient::GetHostInfo (bool force)
> {
> ...
>    if (cpu != LLDB_INVALID_CPUTYPE)
>    {
> 
> since the code inside this logic then sets up some very apple/ios/macosx behaviour.

cputype and subtype is currently assumed to be a mach-o thing and it then might default to the wrong vendor and OS.  I would suggest setting the triple only for non-mach-o cases.
> 
> I apologise for the above braindump, but my conclusion is that to get lldb to work properly for non-apple, non-linux, etc. bare-metal embedded architectures, I need to submit several patches, in particular to the gdb-remote handling logic.
> 
> With your blessing, are you happy for me to do this?

We need to watch out for certain pitfalls, but yes, this should work better than it does now and fixes are required. We just need to make sure not to regress on the desktop and remote targets with any changes we make.

> 
>> But it would be better to also look around in the ELF file and look for .note sections or anything else that can help you determine the correct triple for a given ELF file. If "kalimba" architectures are never native you can put an extra check in here. You might be able to also look at the type of the ELF file in the ELF header (e_type) and see if it is:
>> 
>> ET_NONE - probably best not to set the os and vendor to host (is this the kind of file you have?)
>> ET_EXEC, ET_DYN, ET_CORE - do what is being done above with host architectures and maybe add some .note code to see if you can identify anything more about the binary. I am guessing linux ELF files for executables and shared libraries have something that you will be able to use to properly identify them.
>> 
>> So some more intelligent code in the ObjectFileELF can help us to classify the binaries more correctly, it should improve things in LLDB.
> 
> I'm happy to supply some better ObjectFileELF code in lldb. But my opinion as stated above is that the information received from the stub should *strongly* influence the specification of the architecture/OS etc. in the final Target object.

We need both to be correct. The object files often determine the triple before we run and we want to get this right in the object file. After we run, you might have created a target with a completely wrong file and arch and we might need to change things once we attach. So both should be possible and be as correct as they can be.
> 
> Out of interest the ELF for one of our kalimba variants is as follows:
> 
> ELF Header:
>  Magic:   7f 45 4c 46 01 01 01 00 00 00 00 00 00 00 00 00
>  Class:                             ELF32
>  Data:                              2's complement, little endian
>  Version:                           1 (current)
>  OS/ABI:                            UNIX - System V
>  ABI Version:                       0
>  Type:                              EXEC (Executable file)
>  Machine:                           <unknown>: 0x72ec
>  Version:                           0x1
>  Entry point address:               0x80000000
> ....
> 
> (Sooner or later I'd like to submit a patch to upstream lldb with core definitions for this chip. )

You could currently assume if the Elf header contains kalimba (0x72ec) that its triple is always then unknown-unknown.

> 
> thanks
> Matt
> 
> 
> 
> Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
> More information can be found at www.csr.com. Keep up to date with CSR on our technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, www.youtube.com/user/CSRplc, Facebook, www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at www.twitter.com/CSR_plc.
> New for 2014, you can now access the wide range of products powered by aptX at www.aptx.com.