[patch] Fix dwfl_report_elf BASE alignment [Re: pending patches ping#2]

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Fri, 15 Mar 2013 14:37:53 +0100, Mark Wielaard wrote:
> The only reason I was hesitant about the patch was the testcase. I
> assume this mimics what dwfl_linux_proc_report () would do, and it was
> easier to write it using dwfl_report_elf ()?

dwfl_linux_proc_report uses live PID but if you have a dead core file how to
pass a PID to dwfl_linux_proc_report?

It is also explained in the patch mail that elfutils uses dwfl_report_segment
which does not have such bug but it is elfutils internal function.
Applications cannot use it.  Maybe the unwinder could have a different API
without using dwfl_report_elf but even in such case dwfl_report_elf exists so
its bug should be fixed (by this patch).


> If you could add a comment where the constants for base (0x7f3560c92000)
> and funcaddr (0x7f3560c92585) come from (and/or how to calculate your
> own in case someone wants to extend the testcase) that would be
> appreciated.

Done.


Therefore I expect it can be checked in.


Thanks,
Jan


jankratochvil/basecleanup-basealign


commit f5752a86cb1b191a09a5636dc76addc030440aec
Author: Jan Kratochvil <jan.kratochvil@redhat.com>
Date:   Sat Oct 27 20:50:21 2012 +0200

    libdwfl/
    	* dwfl_report_elf.c (__libdwfl_report_elf): Remove BASE aligning.
    
    tests/
    	* Makefile.am (check_PROGRAMS): Add dwfl-report-elf-align.
    	(TESTS): Add run-dwfl-report-elf-align.sh.
    	(EXTRA_DIST): Add run-dwfl-report-elf-align.sh and
    	testfile-dwfl-report-elf-align-shlib.so.bz2 .
    	(dwfl_report_elf_align_LDADD): New.
    	* dwfl-report-elf-align.c: New file.
    	* run-dwfl-report-elf-align.sh: New file.
    	* testfile-dwfl-report-elf-align-shlib.so.bz2: New file.
    
    Signed-off-by: Jan Kratochvil <jan.kratochvil@redhat.com>

diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 78139ba..f55625c 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,7 @@
+2013-03-15  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* dwfl_report_elf.c (__libdwfl_report_elf): Remove BASE aligning.
+
 2013-02-22  Mark Wielaard  <mjw@redhat.com>
 
 	* open.c (__libdw_gunzip,__libdw_bunzip2,__libdw_unlzma): Define
diff --git a/libdwfl/dwfl_report_elf.c b/libdwfl/dwfl_report_elf.c
index 174ce7c..d706170 100644
--- a/libdwfl/dwfl_report_elf.c
+++ b/libdwfl/dwfl_report_elf.c
@@ -181,8 +181,6 @@ __libdwfl_report_elf (Dwfl *dwfl, const char *name, const char *file_name,
 	    {
 	      vaddr = ph->p_vaddr & -ph->p_align;
 	      address_sync = ph->p_vaddr + ph->p_memsz;
-	      if ((base & (ph->p_align - 1)) != 0)
-		base = (base + ph->p_align - 1) & -ph->p_align;
 	      start = base + vaddr;
 	      break;
 	    }
diff --git a/tests/ChangeLog b/tests/ChangeLog
index d181cd5..4071e6f 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,14 @@
+2013-03-15  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* Makefile.am (check_PROGRAMS): Add dwfl-report-elf-align.
+	(TESTS): Add run-dwfl-report-elf-align.sh.
+	(EXTRA_DIST): Add run-dwfl-report-elf-align.sh and
+	testfile-dwfl-report-elf-align-shlib.so.bz2 .
+	(dwfl_report_elf_align_LDADD): New.
+	* dwfl-report-elf-align.c: New file.
+	* run-dwfl-report-elf-align.sh: New file.
+	* testfile-dwfl-report-elf-align-shlib.so.bz2: New file.
+
 2013-02-22  Mark Wielaard  <mjw@redhat.com>
 
 	* Makefile.am (TESTS): Remove run-readelf-s.sh and run-dwflsyms.sh.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 63184f8..d256733 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -51,7 +51,7 @@ check_PROGRAMS = arextract arsymtest newfile saridx scnnames sectiondump \
 		  dwfl-bug-getmodules dwarf-getmacros addrcfi \
 		  test-flag-nobits dwarf-getstring rerequest_tag \
 		  alldts md5-sha1-test typeiter low_high_pc \
-		  test-elf_cntl_gelf_getshdr dwflsyms
+		  test-elf_cntl_gelf_getshdr dwflsyms dwfl-report-elf-align
 asm_TESTS = asm-tst1 asm-tst2 asm-tst3 asm-tst4 asm-tst5 \
 	    asm-tst6 asm-tst7 asm-tst8 asm-tst9
 
@@ -85,7 +85,7 @@ TESTS = run-arextract.sh run-arsymtest.sh newfile test-nlist \
 	run-readelf-d.sh run-readelf-gdb_index.sh run-unstrip-n.sh \
 	run-low_high_pc.sh run-macro-test.sh run-elf_cntl_gelf_getshdr.sh \
 	run-test-archive64.sh run-readelf-vmcoreinfo.sh \
-	run-readelf-mixed-corenote.sh
+	run-readelf-mixed-corenote.sh run-dwfl-report-elf-align.sh
 
 if !STANDALONE
 check_PROGRAMS += msg_tst md5-sha1-test
@@ -192,7 +192,9 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh \
 	     run-readelf-mixed-corenote.sh testfile63.bz2 testfile64.bz2 \
 	     testfile65.bz2 testfile67.bz2 testfile68.bz2 \
 	     testfile69.core.bz2 testfile69.so.bz2 \
-	     testfile70.core.bz2 testfile70.exec.bz2
+	     testfile70.core.bz2 testfile70.exec.bz2 \
+	     run-dwfl-report-elf-align.sh \
+	     testfile-dwfl-report-elf-align-shlib.so.bz2
 
 if USE_VALGRIND
 valgrind_cmd="valgrind -q --trace-children=yes --error-exitcode=1 --run-libc-freeres=no"
@@ -300,6 +302,7 @@ typeiter_LDADD = $(libdw) $(libelf) $(libmudflap)
 low_high_pc_LDADD = $(libdw) $(libelf) $(libmudflap)
 test_elf_cntl_gelf_getshdr_LDADD = $(libelf) $(libmudflap)
 dwflsyms_LDADD = $(libdw) $(libelf) $(libmudflap)
+dwfl_report_elf_align_LDADD = $(libdw) $(libmudflap)
 
 if GCOV
 check: check-am coverage
diff --git a/tests/dwfl-report-elf-align.c b/tests/dwfl-report-elf-align.c
new file mode 100644
index 0000000..0e8bfe3
--- /dev/null
+++ b/tests/dwfl-report-elf-align.c
@@ -0,0 +1,72 @@
+/* Test program for dwfl_report_elf incorrect BASE alignment.
+   Copyright (C) 2013 Red Hat, Inc.
+   This file is part of elfutils.
+
+   This file is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   elfutils is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <config.h>
+#include <assert.h>
+#include <inttypes.h>
+#include <stdio.h>
+#include <stdio_ext.h>
+#include <error.h>
+#include <locale.h>
+#include <string.h>
+#include <stdlib.h>
+#include ELFUTILS_HEADER(dwfl)
+
+
+static const Dwfl_Callbacks offline_callbacks =
+  {
+    .find_debuginfo = INTUSE(dwfl_standard_find_debuginfo),
+    .section_address = INTUSE(dwfl_offline_section_address),
+  };
+
+
+int
+main (int argc, char **argv)
+{
+  /* We use no threads here which can interfere with handling a stream.  */
+  (void) __fsetlocking (stdout, FSETLOCKING_BYCALLER);
+
+  /* Set locale.  */
+  (void) setlocale (LC_ALL, "");
+
+  if (argc != 5)
+    error (1, 0, "dwfl-report-elf-align shlib.so base funcaddr funcname");
+    
+  Dwfl *dwfl = dwfl_begin (&offline_callbacks);
+  assert (dwfl != NULL);
+
+  char *endptr;
+  uintptr_t base = strtoull (argv[2], &endptr, 0);
+  assert (endptr && !*endptr);
+
+  Dwfl_Module *mod = dwfl_report_elf (dwfl, argv[1], argv[1], -1, base);
+  assert (mod != NULL);
+
+  uintptr_t funcaddr = strtoull (argv[3], &endptr, 0);
+  assert (endptr && !*endptr);
+
+  Dwfl_Module *mod_found = dwfl_addrmodule (dwfl, funcaddr);
+  assert (mod_found == mod);
+
+  const char *symname = dwfl_module_addrname (mod, funcaddr);
+  assert (symname != NULL);
+  assert (strcmp (symname, argv[4]) == 0);
+
+  dwfl_end (dwfl);
+
+  return 0;
+}
diff --git a/tests/run-dwfl-report-elf-align.sh b/tests/run-dwfl-report-elf-align.sh
new file mode 100755
index 0000000..d032aba
--- /dev/null
+++ b/tests/run-dwfl-report-elf-align.sh
@@ -0,0 +1,44 @@
+#! /bin/sh
+# Copyright (C) 2013 Red Hat, Inc.
+# This file is part of elfutils.
+#
+# This file is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# elfutils is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. $srcdir/test-subr.sh
+
+testfiles testfile-dwfl-report-elf-align-shlib.so
+
+# /proc/PID/maps when the process was running:
+# 7f3560c92000-7f3560c93000 r-xp 00000000 fd:02 25037063 testfile-dwfl-report-elf-align-shlib.so
+# 7f3560c93000-7f3560e92000 ---p 00001000 fd:02 25037063 testfile-dwfl-report-elf-align-shlib.so
+# 7f3560e92000-7f3560e93000 rw-p 00000000 fd:02 25037063 testfile-dwfl-report-elf-align-shlib.so
+# testfile-dwfl-report-elf-align-shlib.so:
+# Program Headers:
+#   Type           Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
+#   LOAD           0x000000 0x0000000000000000 0x0000000000000000 0x00065c 0x00065c R E 0x200000
+#   LOAD           0x000660 0x0000000000200660 0x0000000000200660 0x0001f0 0x000200 RW  0x200000
+# Symbol table '.dynsym' contains 12 entries:
+#    Num:    Value          Size Type    Bind   Vis      Ndx Name
+#      8: 000000000000057c    11 FUNC    GLOBAL DEFAULT   11 shlib
+# GDB output showing proper relocation:
+# #1  0x00007f3560c92585 in shlib () from ./testfile-dwfl-report-elf-align-shlib.so
+#
+# 0x7f3560c92000 is VMA address of first byte of testfile-dwfl-report-elf-align-shlib.so.
+# 0x7f3560c92585 = 0x7f3560c92000 + 0x585
+# where 0x585 is any address inside the shlib function: 0x57c .. 0x57c + 11 -1
+
+testrun ./dwfl-report-elf-align ./testfile-dwfl-report-elf-align-shlib.so \
+				0x7f3560c92000 0x7f3560c92585 shlib
+
+exit 0
diff --git a/tests/testfile-dwfl-report-elf-align-shlib.so.bz2 b/tests/testfile-dwfl-report-elf-align-shlib.so.bz2
new file mode 100755
index 0000000000000000000000000000000000000000..1f35df98ffa1dc5883acce9f3b567891f024e43e
GIT binary patch
literal 1546
zcmV+l2KD(uT4*^jL0KkKSx!>Q^#BDofB*mg{r}ege}Dh~eqaCp|NPHtQJ_~;*l2BG
zOTgc3=V{OczV0wl*EV&Ux+Vsvq$!GRDYVLH^%)OH03K67(?N)OnE)Q9ho}GpKma{J
zG-zq+01c!70L1-B^#-ZvAeo?;L8609(E*?ypa1{^Mw$Q`01X2`05oXOG5`S3003%Y
z5t3q))DKNZqyPqiqts{vMnEzE8Vv(a)G0LTo000000G|8X<5E>Z<nKaNangc^jnl!+S
zjG7FFOoJm#jT&ScGyuUGXfX`{44N4*5unkaNRXo`qHPUElW7}MWi-*XHiJosX`s_a
zfYIt17>odeMt}_gpazW|Q$s^cKpHg2H-+9dek8nF7>Hhj5mLIf5qL%9+>6>y987dm
zCZK~BX2dnDlxT!Vv|)gRf)rHrl_+cSr#B6QmN68Q5ERR81(a)S3MbLS;$5km9x0pP$G
zM2H4i#vG?TgiSG1LH7(9!XScO2fmsVhf-lP9&_G4yl6aXCgoWS7A{{z_p4}|6SR;h
z6*mr}ndBsrNDCEX_!6&#%_Zr^;5J~l6PI^+((2q?*zY)Mc0VJ)3gom7|3vJ|J&RBP
zP{LtisOagWU?fw4y0tV;s-(j6=YsBehA7d`?5>KYx@&L(GCXbfMLnj6Wv#hGs(roo
z6r*VdYIpb%IIYG`3|8sO(a)h3fn{Zc|&rjj*GLIAM)rj(a)yAi6+nx#C2^i4ncb40Y<mX
zNhC?6tu!nROdxFlHmnULu<T(Oc(a)TfkABX1f<G9!8C+M-5JzR1FTx6xhmJ$O;)Tnt0
zWf6#3nPU|a24*XHyEjF>Xl0?5Vcg8m8KCajWvw!0?1;G>c<3BbQiT$)H)s0IL+U6=
z1sW#KxtMobSt7>13yc-^v)K2z-Q|vJ#$vR(a)-M0@!m_zF)0m5J`b|Z?=mMhKv8yMtD
zB`HK&RAov8*xH~0LWXiZ8)?~|l^;o_Qmv2s(0~OA+wmPG6r#pmPeX!htFmK+r!tU6
zwTmm2`5K6X;>`&&mwm`)K+EmzCx*P*90W_1F=Rrhz;dAubJk&G&HdhDJZ8a#b?RzP
z_Xi$$d?ZVH4Y~tc0*E>I>a6cgY>-VaxVQ7lYC)NoyIV*kDvUsb8siyqLV~B20eaN3
zmtePOALR=M_uh676G!aHx<D~4!^|w59M@^nmNmy7h~rF*D&;GvLm|NNDxLqB{`<3=
zU1nB#wTp-NnVKValj$~yZH)!gD#=>BWSVZ0hQSdUZpuqryJ+&fcPc5sc40LjNQ%A&
zMPTb)*?4Kn+wa{%)PS}*W(IwSkj3M)Nv!0^H;>D7GnVjN;D>fw(a)OHjVRu)`kxJ(HR
z^kE4ll5GX<dM}<cn9rD+i>@s>USO4$W%soO*><0^q(a)LJ$NdHJu;?1v#OeV)n)nT5y
zGg-)rwfmmhv7|g})b71(a)SfdZC@7rHU*Ww;*Twl$XFq>k#pofB4mL0|{>%B(a)Mc2hZf
zbo~T*t6Z|(;ngECShMRf1xrE+hqM=FzU0$T{6dAJ0#R#Y`cP*VYk@%#wmPA{g4aY@
z$ci)7J?SPCyzrdyF!?Mla8wvSWx`xWvCx(a)A2JNN-RcymRsuU|6jSOnS?!zZENfhe^
zJ?|F}4rEsGs|7l9C{qJ^P9~M3vKb^AGiCut2V4$ia>9`<4WmI+gCaWjpm=hoRBD72
zLjXh$BMY?zAtyYfx|(1Wa6^JAv<OtEVF8zHy6P%5RLceg_&`CQojBrq;AY}&&D6mS
zoh4Co2J65w#CK9;I<2Dd2}EU}$SF$G-59QP8TeHl@&yG}5c)=@woKr964`XXD1rnd
zTOiQfUs^fTYoJ(a)MQJbw<{#zSNjv8$e*v{Eo6D&Sreh^xKSO>zk8^QZ(a)rz|pqAnI(b
z+yTe2-;sHuejQXC-^9z6TwZ8bxsg#+K_aMZ$PhtWBaf+dR~2Z~B)k#}{ayWAj-VT~
w3CkdHfdqj_tI8j#a!WvjqM=LVwVFt(S=+gGkst7LqkqNRkxmpO6O^*OK%{@Z*#H0l

literal 0
HcmV?d00001