[lldb-dev] File offset usage on ObjectFileELF

Filipe Cabecinhas filcab at gmail.com
Thu May 16 11:59:55 PDT 2013


Hi all,

Why is ObjectFileELF using the file offset to parse the ELF headers?

For the changes I'm suggesting, skip to the end of the message (3 last
paragraphs).

>From what I can tell, ObjectFileELF's constructor will get a
FileSpec+file_offset and a DataBuffer+data_offset. CreateInstance made
sure the DataBuffer had all the data we needed.

Then ObjectFileELF's constructor calls ObjectFile's constructor with
those arguments too. ObjectFile's constructor sets its private m_data
field to point to the argument DataBuffer's contents (+ offset), and
simply stores the FileSpec and file_offset.

When ObjectFileELF::ParseHeader() is called, it gets the file_offset
and then parses the header from m_data with that offset. This makes no
sense, for me, since the offset is only for the file, not the
DataBuffer.

I couldn't find any other reference to ObjectFile::GetFileOffset on
either ObjectFile, ObjectFileELF, or ObjectFileMachO. There is one
reference to m_file_offset in ObjectFileMachO, though. Which seems
legit, but is weird since CreateInstance makes sure that the
DataBuffer has all the file, before calling the constructor.




I'm proposing changing ObjectFileELF::ParseHeader() to just create a
variable that is 0 as the offset and use that as the offset for the
ELFHeader::Parse() method.

Either ObjectFileMachO calls the GetFileOffset() or it's not doing
anything and we can delete it. Otherwise bug can appear out of nowhere
if anything is changed in one of those.

Is it possible that ObjectFileMachO actually needs to read the file
again? When can this happen? (ObjectFileMachO's constructor is not
private, but I think it's only called from CreateInstance)

Should I prepare a patch, or did I miss something?

Thank you,

  Filipe

P.S: I found this while trying to make a reader for a format based on
ELF which has an extra header. I just bumped the offsets by that
header's size, on CreateInstance, and created an ObjectFileELF, that's
why I bumped into this bug.



More information about the lldb-dev mailing list