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 <dccitali...@gmail.com> wrote: > On Mon, Feb 5, 2018 at 9:25 AM, Pavel Labath via lldb-commits > <lldb-commits@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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits