[Lldb-commits] [lldb] r324254 - Fix parsing of object files with "early" section headers

[Pavel Labath via lldb-commits]

Committed as r324256. Thanks for keeping me honest.

Going back to this, I noticed that I did not check-in the final tiny
binary that I intended to, which happened because I was rushing to get
the build-id test fixed, so I could get back to what I originally
planned to do for today, and the fact that I had to fix a completely
unrelated bug to do that got me upset.

I'll have to be more careful next time.

On 5 February 2018 at 17:42, Davide Italiano <dccitaliano at gmail.com> wrote:
> On Mon, Feb 5, 2018 at 9:25 AM, Pavel Labath via lldb-commits
> <lldb-commits at lists.llvm.org> wrote:
>> Author: labath
>> Date: Mon Feb  5 09:25:40 2018
>> New Revision: 324254
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=324254&view=rev
>> Log:
>> Fix parsing of object files with "early" section headers
>>
>> ObjectFileELF::GetModuleSpecifications contained a lot of tip-toing code
>> which was trying to avoid loading the full object file into memory. It
>> did this by trying to load data only up to the offset if was accessing.
>> However, in practice this was useless, as 99% of object files we
>> encounter have section headers at the end, so we would load the whole
>> file as soon as we start parsing the section headers.
>>
>> In fact, this would break as soon as we encounter a file which does
>> *not* have section headers at the end (yaml2obj produces these), as the
>> access to .strtab (which we need to get the section names) was not
>> guarded by this offset check.
>>
>> As this strategy was completely ineffective anyway, I do not attempt to
>> proliferate it further by guarding the .strtab accesses. Instead I just
>> lead the full file as soon as we are reasonably sure that we are indeed
>> processing an elf file.
>>
>> If we really care about the load size here, we would need to reimplement
>> this to just load the bits of the object file we need, instead of
>> loading everything from the start of the object file to the given
>> offset. However, given that the OS will do this for us for free when
>> using mmap, I think think this is really necessary.
>>
>> For testing this I check a (tiny) SO file instead of yaml2obj-ing it
>> because the fact that they come out first is an implementation detail of
>> yaml2obj that can change in the future.
>>
>
> I personally don't mind at all checking shared libraries directly if
> it saves us some time, but, FWIW, can you also add a comment to the
> test explaining how to generate the shared object?
> e.g. something like run `yaml2obj` on the following <> (with yaml2obj
> rev123456) [or, if you used a c file, just add the c file to a comment
> & the compiler/linker invocation).
> This worked us very well for `llvm-mc` tests in the past, if for any
> reason whatsoever we have to regenerate them.
>
> Thank you!
>
> --
> Davide