[Lldb-commits] [PATCH] Simplify indirect rld_map for mips (r192408).

[Greg Clayton]

Very nice. I almost suggested passing the Process in your first patch, but the Target is better as it allows you to do this before your process is running, so this is better.

On Oct 11, 2013, at 8:20 AM, Ed Maste <emaste at freebsd.org> wrote:

> Just pass a Target* into ObjectFileELF::GetImageInfoAddress so that it can do the extra dereference necessary on MIPS, instead of passing a flag back to the caller.
> 
> No functional difference but I think this is cleaner; it avoids having identical code in ProcessPOSIX and ProcessElfCore.
> 
> http://llvm-reviews.chandlerc.com/D1899
> 
> Files:
>  include/lldb/Symbol/ObjectFile.h
>  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
>  source/Plugins/ObjectFile/ELF/ObjectFileELF.h
>  source/Plugins/Process/POSIX/ProcessPOSIX.cpp
>  source/Plugins/Process/elf-core/ProcessElfCore.cpp
> 
> Index: include/lldb/Symbol/ObjectFile.h
> ===================================================================
> --- include/lldb/Symbol/ObjectFile.h
> +++ include/lldb/Symbol/ObjectFile.h
> @@ -506,7 +506,7 @@
>     ///     The address of any auxiliary tables, or an invalid address if this
>     ///     object file format does not support or contain such information.
>     virtual lldb_private::Address
> -    GetImageInfoAddress (bool &indirect) { indirect = false; return Address(); }
> +    GetImageInfoAddress (Target *target) { return Address(); }
> 
>     //------------------------------------------------------------------
>     /// Returns the address of the Entry Point in this object file - if
> Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
> ===================================================================
> --- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
> +++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
> @@ -23,6 +23,7 @@
> #include "lldb/Core/Stream.h"
> #include "lldb/Symbol/DWARFCallFrameInfo.h"
> #include "lldb/Symbol/SymbolContext.h"
> +#include "lldb/Target/Target.h"
> #include "lldb/Host/Host.h"
> 
> #include "llvm/ADT/PointerUnion.h"
> @@ -515,7 +516,7 @@
> }
> 
> Address
> -ObjectFileELF::GetImageInfoAddress(bool &indirect)
> +ObjectFileELF::GetImageInfoAddress(Target *target)
> {
>     if (!ParseDynamicSymbols())
>         return Address();
> @@ -539,14 +540,24 @@
>     {
>         ELFDynamic &symbol = m_dynamic_symbols[i];
> 
> -        if (symbol.d_tag == DT_DEBUG || symbol.d_tag == DT_MIPS_RLD_MAP)
> +        if (symbol.d_tag == DT_DEBUG)
>         {
> -            indirect = (symbol.d_tag == DT_MIPS_RLD_MAP);
>             // Compute the offset as the number of previous entries plus the
>             // size of d_tag.
>             addr_t offset = i * dynsym_hdr->sh_entsize + GetAddressByteSize();
>             return Address(dynsym_section_sp, offset);
>         }
> +        else if (symbol.d_tag == DT_MIPS_RLD_MAP && target)
> +        {
> +            addr_t offset = i * dynsym_hdr->sh_entsize + GetAddressByteSize();
> +            addr_t dyn_base = dynsym_section_sp->GetLoadBaseAddress(target);
> +            if (dyn_base == LLDB_INVALID_ADDRESS)
> +                return Address();
> +            Address addr;
> +            Error error;
> +            if (target->ReadPointerFromMemory(dyn_base + offset, false, error, addr))
> +                return addr;
> +        }
>     }
> 
>     return Address();
> Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.h
> ===================================================================
> --- source/Plugins/ObjectFile/ELF/ObjectFileELF.h
> +++ source/Plugins/ObjectFile/ELF/ObjectFileELF.h
> @@ -127,7 +127,7 @@
>     GetDependentModules(lldb_private::FileSpecList& files);
> 
>     virtual lldb_private::Address
> -    GetImageInfoAddress(bool &indirect);
> +    GetImageInfoAddress(lldb_private::Target *target);
> 
>     virtual lldb_private::Address
>     GetEntryPointAddress ();
> Index: source/Plugins/Process/POSIX/ProcessPOSIX.cpp
> ===================================================================
> --- source/Plugins/Process/POSIX/ProcessPOSIX.cpp
> +++ source/Plugins/Process/POSIX/ProcessPOSIX.cpp
> @@ -289,23 +289,10 @@
> {
>     Target *target = &GetTarget();
>     ObjectFile *obj_file = target->GetExecutableModule()->GetObjectFile();
> -    bool indirect;
> -    Address addr = obj_file->GetImageInfoAddress(indirect);
> +    Address addr = obj_file->GetImageInfoAddress(target);
> 
>     if (addr.IsValid())
> -    {
> -        if (indirect)
> -        {
> -            Address ind_addr;
> -            Error error;
> -            if (target->ReadPointerFromMemory(addr.GetLoadAddress(target), false, error, ind_addr))
> -                return ind_addr.GetLoadAddress(target);
> -        }
> -        else
> -        {
> -            return addr.GetLoadAddress(target);
> -        }
> -    }
> +        return addr.GetLoadAddress(target);
>     return LLDB_INVALID_ADDRESS;
> }
> 
> Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp
> ===================================================================
> --- source/Plugins/Process/elf-core/ProcessElfCore.cpp
> +++ source/Plugins/Process/elf-core/ProcessElfCore.cpp
> @@ -347,23 +347,10 @@
> {
>     Target *target = &GetTarget();
>     ObjectFile *obj_file = target->GetExecutableModule()->GetObjectFile();
> -    bool indirect;
> -    Address addr = obj_file->GetImageInfoAddress(indirect);
> +    Address addr = obj_file->GetImageInfoAddress(target);
> 
>     if (addr.IsValid())
> -    {
> -        if (indirect)
> -        {
> -            Address ind_addr;
> -            Error error;
> -            if (target->ReadPointerFromMemory(addr.GetLoadAddress(target), false, error, ind_addr))
> -                return ind_addr.GetLoadAddress(target);
> -        }
> -        else
> -        {
> -            return addr.GetLoadAddress(target);
> -        }
> -    }
> +        return addr.GetLoadAddress(target);
>     return LLDB_INVALID_ADDRESS;
> }
> <D1899.1.patch>_______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits