From 8095c33951bd97b77f42126e1673226aed6f1508 Mon Sep 17 00:00:00 2001 From: Jan Kratochvil Date: Mon, 27 Oct 2014 15:47:21 +0100 Subject: [PATCH] Backport vDSO regression. - Revert the makeinfo workaround from 7.8-27.fc21. - Further 1.75x improvement of the interactive symbols lookup (Doug Evans). --- gdb-symbols-lookup-accel.patch | 76 ++++++++++++++++++++------ gdb-upstream.patch | 98 ++++++++++++++++++++++++++++++++++ gdb.spec | 14 +++-- 3 files changed, 163 insertions(+), 25 deletions(-) diff --git a/gdb-symbols-lookup-accel.patch b/gdb-symbols-lookup-accel.patch index 4c90f78..1901d07 100644 --- a/gdb-symbols-lookup-accel.patch +++ b/gdb-symbols-lookup-accel.patch @@ -78,49 +78,91 @@ index 055c87e..90bcd6d 100644 --17pEHd4RhPHOinZp-- -http://sourceware.org/ml/gdb-patches/2014-10/msg00525.html -Subject: [patch 2/2] Accelerate lookup_symbol_aux_objfile 8x ---K8nIJk4ghYZn606h +http://sourceware.org/ml/gdb-patches/2014-10/msg00612.html +Subject: [patchv2 2/2] Accelerate lookup_symbol_aux_objfile 14.5x [Re: [patch 0/2] Accelerate symbol lookups 15x] + + +--vtzGhvizbBRQ85DL Content-Type: text/plain; charset=us-ascii Content-Disposition: inline -Hi, +On Wed, 22 Oct 2014 10:55:18 +0200, Doug Evans wrote: +> For example, the count of calls to dict_hash before/after goes from 13.8M to 31. +> I'm guessing one t hing we're doing here is coping with an artifact of +> dwz: -lookup_symbol_aux_objfile() processing is very ineffective. For each primary -symtab it searches it and also all its secondary symtabs. But that means that -secondary symtabs included in many primary symtabs get needlessly searched -many times during one lookup_symbol_aux_objfile() run. +During my simple test on non-DWZ file (./gdb itself) it went 3684->3484. -lookup_symbol_aux_objfile does not care in which primary/secondary symtab the -symbol is found. +The problem is that dict_cash(val) is called for the same val for each block +(== symtab). + +On DWZ the saving is probably much larger as there are many more symtabs due +to DW_TAG_partial_unit ones. +> what was once one global block to represent the entire objfile is +> now N. + +Without DWZ there are X global blocks for X primary symtabs for X CUs of +objfile. With DWZ there are X+Y global blocks for X+Y primary symtabs for +X+Y CUs where Y are 'DW_TAG_partial_unit's. + +For 'DW_TAG_partial_unit's (Ys) their blockvector is usually empty. But not +always, I have found there typedef symbols, there can IMO be optimized-out +static variables etc. + + +> [I'm sure the patches help in the non-dwz case, but I suspect it's less. +> Which isn't to say the patches aren't useful. +> I just need play with a few more examples.] + +I agree. + +[patch 2/2] could needlessly performance-regress non-DWZ cases, therefore +I have put back original ALL_OBJFILE_PRIMARY_SYMTABS (instead of my +ALL_OBJFILE_SYMTABS) as it is perfectly sufficient. For the performance +testcase of mine: + +Benchmark on non-trivial application with 'p ': +Command execution time: 4.215000 (cpu), 4.241466 (wall) --- both fixes with new [patch 2/2] +Command execution time: 7.373000 (cpu), 7.395095 (wall) --- both fixes +Command execution time: 13.572000 (cpu), 13.592689 (wall) --- just lookup_symbol_aux_objfile fix +Command execution time: 113.036000 (cpu), 113.067995 (wall) --- FSF GDB HEAD + +That is additional 1.75x improvement, making the total improvement 26.8x. + + +No regressions on {x86_64,x86_64-m32,i686}-fedora21pre-linux-gnu in standard +and .gdb_index-enabled runs. Neither of the patches should cause any visible +behavior change. + + +Thanks, Jan ---K8nIJk4ghYZn606h +--vtzGhvizbBRQ85DL Content-Type: text/plain; charset=us-ascii -Content-Disposition: inline; filename="idxcache2.patch" +Content-Disposition: inline; filename="idxcache2doug.patch" gdb/ -2014-10-20 Jan Kratochvil +2014-10-23 Jan Kratochvil * symtab.c (lookup_symbol_aux_objfile): Use ALL_OBJFILE_SYMTABS, inline lookup_block_symbol. diff --git a/gdb/symtab.c b/gdb/symtab.c -index c530d50..bc800ef 100644 +index c530d50..da13861 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -1657,15 +1657,25 @@ lookup_symbol_aux_objfile (struct objfile *objfile, int block_index, const struct block *block; struct symtab *s; -- ALL_OBJFILE_PRIMARY_SYMTABS (objfile, s) + gdb_assert (block_index == GLOBAL_BLOCK || block_index == STATIC_BLOCK); + -+ ALL_OBJFILE_SYMTABS (objfile, s) + ALL_OBJFILE_PRIMARY_SYMTABS (objfile, s) { + struct dict_iterator dict_iter; + @@ -145,5 +187,5 @@ index c530d50..bc800ef 100644 } ---K8nIJk4ghYZn606h-- +--vtzGhvizbBRQ85DL-- diff --git a/gdb-upstream.patch b/gdb-upstream.patch index 9d2bac8..2c0cfc9 100644 --- a/gdb-upstream.patch +++ b/gdb-upstream.patch @@ -1770,3 +1770,101 @@ gdb/testsuite/ -- 1.9.3 + + +commit 54fbc750b54271efb75ae11ce49f14c4234a9476 +Author: Jan Kratochvil +Date: Thu Sep 18 08:21:40 2014 +0200 + + Fix regression for Linux vDSO in GDB (PR gdb/17407). + + since + 5979d6b69b20a8355ea94b75fad97415fce4788c + https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=5979d6b69b20a8355ea94b75fad97415fce4788c + vdso handling + https://sourceware.org/ml/binutils/2014-03/msg00082.html + https://sourceware.org/ml/binutils/2014-04/msg00003.html + Message-ID: + I get on + kernel-3.16.2-200.fc20.x86_64 + https://koji.fedoraproject.org/koji/buildinfo?buildID=575860 + attaching its vdso.bin.gz + GDB (FSF HEAD 5e43d46791c4c66fd83947a12d4f716b561a9103) regression: + reproducer: + ./gdb -ex start ./gdb + actual result / FAIL: + Got object file from memory but can't read symbols: File truncated. + expected result / PASS: + + or / PASS: + warning: Could not load shared library symbols for linux-vdso.so.1. + Do you need "set solib-search-path" or "set sysroot"? + + That "warning: Could not load shared library..." is mostly harmless (it is + a bug in GDB), in the FAIL case it is not printed just because + bfd_check_format() fails there. + + It seems logical to me this way when the 'size' parameter has been already + added. + Alan Modra: + I was wrongly thinking that the section headers were + always last when I wrote that code. (They are now! If you relink + that vdso with current binutils master you won't hit this problem, but + that of course doesn't help existing kernels.) + + I do not see a regression for add-symbol-file-from-memory for libncurses.so.5 + from the original thread above. + + Start of section headers: 1080 (bytes into file) + Size of section headers: 64 (bytes) + Number of section headers: 13 + Section header string table index: 8 + Section Headers: + [Nr] Name Type Address Off Size ES Flg Lk Inf Al + [ 8] .fake_shstrtab STRTAB 0000000000000780 000780 000076 00 A 0 0 32 + Program Headers: + Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align + LOAD 0x000000 0x0000000000000000 0x0000000000000000 0x0012fe 0x0012fe R E 0x1000 + + size == 0x2000 + shdr_end == 0x778 == 1080 + 13 * 64 + high_offset == 0x12fe + + else if (size >= shdr_end) + - high_offset = shdr_end; + + high_offset = size; + + But then 0x778 < 0x780 for "Section header string table index" so whole + bfd_check_format() fails because section headers were not cleared here: + /* If the segments visible in memory didn't include the section headers, + then clear them from the file header. */ + if (high_offset < shdr_end) + + bfd/ChangeLog + 2014-09-18 Jan Kratochvil + + PR gdb/17407 + * elfcode.h (bfd_from_remote_memory): Use SIZE for HIGH_OFFSET. + +### a/bfd/ChangeLog +### b/bfd/ChangeLog +## -1,3 +1,8 @@ ++2014-09-18 Jan Kratochvil ++ ++ PR gdb/17407 ++ * elfcode.h (bfd_from_remote_memory): Use SIZE for HIGH_OFFSET. ++ + 2014-07-16 H.J. Lu + + * elf32-i386.c (elf_i386_plt_sym_val): Match PLT entry only for +--- a/bfd/elfcode.h ++++ b/bfd/elfcode.h +@@ -1749,7 +1749,7 @@ NAME(_bfd_elf,bfd_from_remote_memory) + headers. */ + } + else if (size >= shdr_end) +- high_offset = shdr_end; ++ high_offset = size; + else + { + bfd_vma page_size = get_elf_backend_data (templ)->minpagesize; diff --git a/gdb.spec b/gdb.spec index 2ed582b..691bbef 100644 --- a/gdb.spec +++ b/gdb.spec @@ -26,7 +26,7 @@ Version: 7.8 # The release always contains a leading reserved number, start it at 1. # `upstream' is not a part of `name' to stay fully rpm dependencies compatible for the testing. -Release: 28%{?dist} +Release: 29%{?dist} License: GPLv3+ and GPLv3+ with exceptions and GPLv2+ and GPLv2+ with exceptions and GPL+ and LGPLv2+ and BSD and Public Domain and GFDL Group: Development/Debuggers @@ -897,9 +897,6 @@ mv -f readline-doc readline/doc %build rm -rf %{buildroot} -# https://bugzilla.redhat.com/show_bug.cgi?id=1154436 -export PERL5LIB="$PERL5LIB${PERL5LIB:+:}/usr/share/texi2html/lib/Unicode-EastAsianWidth/lib" - # Identify the build directory with the version of gdb as well as the # architecture, to allow for mutliple versions to be installed and # built. @@ -1130,10 +1127,6 @@ echo ====================TESTING END===================== %endif %install - -# https://bugzilla.redhat.com/show_bug.cgi?id=1154436 -export PERL5LIB="$PERL5LIB${PERL5LIB:+:}/usr/share/texi2html/lib/Unicode-EastAsianWidth/lib" - # Initially we're in the %{gdb_src} directory. cd %{gdb_build} rm -rf $RPM_BUILD_ROOT @@ -1350,6 +1343,11 @@ then fi %changelog +* Mon Oct 27 2014 Jan Kratochvil - 7.8-29.fc21 +- Backport vDSO regression. +- Revert the makeinfo workaround from 7.8-27.fc21. +- Further 1.75x improvement of the interactive symbols lookup (Doug Evans). + * Mon Oct 20 2014 Jan Kratochvil - 7.8-28.fc21 - Accelerate interactive symbols lookup 15x.