[lldb-dev] More ELF work.

[Greg Clayton]

On Jul 13, 2010, at 1:36 PM, Stephen Wilson wrote:

> Hi Eli,
> 
> Eli Friedman <eli.friedman at gmail.com> writes:
> 
>> On Tue, Jul 13, 2010 at 12:39 PM, Stephen Wilson <wilsons at start.ca> wrote:
>>>   - ObjectFileELF32 and ObjectFile64 are thin wrappers atop
>>>     ObjectFileELF and provide the plugin interface.
>> 
>> If the code is all shared, what's the justification at this point for
>> keeping the ELF32 and ELF64 readers as separate plugins?  It seems
>> like it would be just as easy to have a unified ELF reader.
> 
> There is no technical justification -- just a meta-level "conceptual"
> difference.  But I think you are right, will rework the patch to remove
> ObjectFileELF32/64.

That sounds good.

> 
>>>   - With this patch and LLVM commit r108221 we can remove our local
>>>     elf.h.
>> 
>> Make sure you don't commit this until the llvm.zip is updated with
>> that commit.  Also, the XCode project will need to be updated, but it
>> should be okay to just let someone update that post-commit.
> 
> Right.  Thanks for the reminder.  I can remove reliance an
> llvm/Support/ELF.h for now and use the local elf.h until a new snapshot
> is taken.

No worries, leave this as is and I will update the llvm.zip with a today's llvm sources. Just watch for the llvm.zip file to be updated before you commit your changes (or post your patch if you don't have commit access).

>> +bool
>> +ELFHeader::MagicBytesMatch(const uint8_t *magic)
>> +{
>> +    if (magic != NULL)
>> +        return memcmp(magic, ElfMagic, strlen(ElfMagic)) == 0;
>> +    return false;
>> +}
>> 
>> Is it actually possible for magic to be null here?  

magic currently should never be NULL when MagicBytesMatch is called.

>> Same applies to ELFHeader::AddressSizeInBytes.

Caller (I assume the only caller is the ELF plug-in itself) would need to verify that magic isn't NULL for this.

> 
> I suppose it is possible that the plugin could be passed an empty
> DataBuffer.  That being said, this check belongs in the CreateInstance
> function.  Will fix.
> 
> 
> Thanks for looking this over!
> 
> Steve
> 
> _______________________________________________
> lldb-dev mailing list
> lldb-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev