[patchv2] Fix dwfl_report_elf BASE alignment

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

Just a small testcase fix, forgotten "testrun".

On Sat, 27 Oct 2012 23:21:30 +0200, Jan Kratochvil wrote:

Hi Roland,

jankratochvil/basecleanup-basealign

this patch has dependency on:
	[patch] Code cleanup: Simplify __libdwfl_report_elf
	https://lists.fedorahosted.org/pipermail/elfutils-devel/2012-October/002730.html

elfutils code does:
	      if ((base & (ph->p_align - 1)) != 0)
		base = (base + ph->p_align - 1) & -ph->p_align;

which conflicts with the p_align ELF spec definition:
	Loadable process segments must have congruent values for p_vaddr and
	p_offset, modulo the page size.This member gives the value to which
	the segments are aligned in memory and in the file.

(I have always found its name "align" to be confusing.)

One can see in real world runtime this "alignment" 0x200000 really is not
applied - base address 0x...92000 is not 2MB-aligned.

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
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

Also I do not see why an alignment of BASE (to whatever value) is useful.

This bug does not affect elfutils as they use dwfl_report_segment internally
and neither dwfl_report_elf nor __libdwfl_report_elf.  Also this code is never
executed by elfutils testsuite and I do not know hot so easilt execute it:
./libdwfl/link_map.c
          mod = INTUSE(dwfl_report_elf) (dwfl, basename (name),
                                         name, -1, l_addr);

Application cannot use dwfl_report_segment as its real use requires private
elfutils members/functions.


Thanks,
Jan


commit acb297e023b68ec27ef697e19efb74981a736436
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 83bdf46..cdf539e 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,5 +1,9 @@
 2012-10-27  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
+	* dwfl_report_elf.c (__libdwfl_report_elf): Remove BASE aligning.
+
+2012-10-27  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
 	* dwfl_report_elf.c (__libdwfl_report_elf): Simplify START and BIAS
 	calculation.
 
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 edb82b4..896105d 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,5 +1,16 @@
 2012-10-27  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.
+
+2012-10-27  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
 	* Makefile.am (EXTRA_DIST): Add testfile64.bz2, testfile65.bz2,
 	testfile69.core.bz2 and testfile69.so.bz2 .
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index f2d2484..a1ed284 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
+		  test-elf_cntl_gelf_getshdr 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
@@ -180,7 +180,9 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh \
 	     testfile60.bz2 testfile61.bz2 \
 	     run-readelf-vmcoreinfo.sh testfile62.bz2 \
 	     run-readelf-mixed-corenote.sh testfile63.bz2 testfile64.bz2 \
-	     testfile65.bz2 testfile69.core.bz2 testfile69.so.bz2
+	     testfile65.bz2 testfile69.core.bz2 testfile69.so.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"
@@ -287,6 +289,7 @@ md5_sha1_test_LDADD = $(libeu)
 typeiter_LDADD = $(libdw) $(libelf) $(libmudflap)
 low_high_pc_LDADD = $(libdw) $(libelf) $(libmudflap)
 test_elf_cntl_gelf_getshdr_LDADD = $(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..86297fa
--- /dev/null
+++ b/tests/dwfl-report-elf-align.c
@@ -0,0 +1,72 @@
+/* Test program for dwfl_report_elf incorrect BASE alignment.
+   Copyright (C) 2012 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 = strtol (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 = strtol (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..4c4c3e6
--- /dev/null
+++ b/tests/run-dwfl-report-elf-align.sh
@@ -0,0 +1,34 @@
+#! /bin/sh
+# Copyright (C) 2012 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
+
+# 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
+# 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
+# #1  0x00007f3560c92585 in shlib () from ./testfile-dwfl-report-elf-align-shlib.so
+
+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