[PATCH] libelf: Always update e_version and e_shentsize in elf_update.

[Mark Wielaard <mjw at redhat dot com>]

When e_version is EV_NONE we should set it to EV_CURRENT like we do for
the EI_VERSION and like we set EI_DATA to the correct byte order when set
to ELFDATANONE.  Likewise we should always set e_shentsize like we do for
e_phentsize, not just when ELF_F_LAYOUT isn't set.

Add a new elfshphehdr testcase to check the above.

Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 libelf/ChangeLog          |   5 ++
 libelf/elf32_updatenull.c |  13 ++--
 tests/ChangeLog           |   7 ++
 tests/Makefile.am         |   7 +-
 tests/elfshphehdr.c       | 182 ++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 207 insertions(+), 7 deletions(-)
 create mode 100644 tests/elfshphehdr.c

diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index 1916877..0609b37 100644
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,3 +1,8 @@
+2015-09-29  Mark Wielaard  <mjw@redhat.com>
+
+	* elf32_updatenull.c (default_ehdr): Set e_version when EV_NONE.
+	(updatenull_wrlock): Always set e_shentsize.
+
 2015-09-23  Mark Wielaard  <mjw@redhat.com>
 
 	* elf32_getehdr.c (getehdr_wrlock): Mark as internal_function.
diff --git a/libelf/elf32_updatenull.c b/libelf/elf32_updatenull.c
index c59ffcb..d3754d3 100644
--- a/libelf/elf32_updatenull.c
+++ b/libelf/elf32_updatenull.c
@@ -84,8 +84,12 @@ ELFW(default_ehdr,LIBELFBITS) (Elf *elf, ElfW2(LIBELFBITS,Ehdr) *ehdr,
   update_if_changed (ehdr->e_ident[EI_VERSION], EV_CURRENT,
 		     elf->state.ELFW(elf,LIBELFBITS).ehdr_flags);
 
-  if (unlikely (ehdr->e_version == EV_NONE)
-      || unlikely (ehdr->e_version >= EV_NUM))
+  if (unlikely (ehdr->e_version == EV_NONE))
+    {
+      ehdr->e_version = EV_CURRENT;
+      elf->state.ELFW(elf,LIBELFBITS).ehdr_flags |= ELF_F_DIRTY;
+    }
+  else if (unlikely (ehdr->e_version >= EV_NUM))
     {
       __libelf_seterrno (ELF_E_UNKNOWN_VERSION);
       return 1;
@@ -394,6 +398,8 @@ __elfw2(LIBELFBITS,updatenull_wrlock) (Elf *elf, int *change_bop, size_t shnum)
       while ((list = list->next) != NULL);
 
       /* Store section information.  */
+      update_if_changed (ehdr->e_shentsize,
+			 elf_typesize (LIBELFBITS, ELF_T_SHDR, 1), ehdr_flags);
       if (elf->flags & ELF_F_LAYOUT)
 	{
 	  /* The user is supposed to fill out e_shoff.  Use it and
@@ -414,9 +420,6 @@ __elfw2(LIBELFBITS,updatenull_wrlock) (Elf *elf, int *change_bop, size_t shnum)
 	  size = (size + SHDR_ALIGN - 1) & ~(SHDR_ALIGN - 1);
 
 	  update_if_changed (ehdr->e_shoff, (GElf_Word) size, elf->flags);
-	  update_if_changed (ehdr->e_shentsize,
-			     elf_typesize (LIBELFBITS, ELF_T_SHDR, 1),
-			     ehdr_flags);
 
 	  /* Account for the section header size.  */
 	  size += elf_typesize (LIBELFBITS, ELF_T_SHDR, shnum);
diff --git a/tests/ChangeLog b/tests/ChangeLog
index b8ac61d..0f8925e 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,10 @@
+2015-09-29  Mark Wielaard  <mjw@redhat.com>
+
+	* elfshphehdr.c: New test.
+	* Makefile.am (check_PROGRAMS): Add elfshphehdr.
+	(TESTS): Likewise.
+	(elfshphehdr_LDADD): New variable.
+
 2015-09-08  Mark Wielaard  <mjw@redhat.com>
 
 	* dwfl-proc-attach.c: New test.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index ca2c2d7..eaa904c 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -51,7 +51,8 @@ check_PROGRAMS = arextract arsymtest newfile saridx scnnames sectiondump \
 		  dwfl-report-elf-align varlocs backtrace backtrace-child \
 		  backtrace-data backtrace-dwarf debuglink debugaltlink \
 		  buildid deleted deleted-lib.so aggregate_size vdsosyms \
-		  getsrc_die strptr newdata elfstrtab dwfl-proc-attach
+		  getsrc_die strptr newdata elfstrtab dwfl-proc-attach \
+		  elfshphehdr
 
 asm_TESTS = asm-tst1 asm-tst2 asm-tst3 asm-tst4 asm-tst5 \
 	    asm-tst6 asm-tst7 asm-tst8 asm-tst9
@@ -118,7 +119,8 @@ TESTS = run-arextract.sh run-arsymtest.sh newfile test-nlist \
 	run-stack-demangled-test.sh \
 	run-readelf-dwz-multi.sh run-allfcts-multi.sh run-deleted.sh \
 	run-linkmap-cut.sh run-aggregate-size.sh vdsosyms run-readelf-A.sh \
-	run-getsrc-die.sh run-strptr.sh newdata elfstrtab dwfl-proc-attach
+	run-getsrc-die.sh run-strptr.sh newdata elfstrtab dwfl-proc-attach \
+	elfshphehdr
 
 if !BIARCH
 export ELFUTILS_DISABLE_BIARCH = 1
@@ -448,6 +450,7 @@ newdata_LDADD = $(libelf)
 elfstrtab_LDADD = $(libelf)
 dwfl_proc_attach_LDADD = $(libdw)
 dwfl_proc_attach_LDFLAGS = -pthread
+elfshphehdr_LDADD =$(libelf)
 
 if GCOV
 check: check-am coverage
diff --git a/tests/elfshphehdr.c b/tests/elfshphehdr.c
new file mode 100644
index 0000000..5f6b96c
--- /dev/null
+++ b/tests/elfshphehdr.c
@@ -0,0 +1,182 @@
+/* Test program for adding section and program headers and ehdr updates.
+   Copyright (C) 2015 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 ELFUTILS_HEADER(elf)
+#include <gelf.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+#include <stdbool.h>
+
+void
+check (const char *msg, bool check)
+{
+  if (! check)
+    {
+      fprintf (stderr, "%s FAILED\n", msg);
+      exit (-1);
+    }
+  else
+    fprintf (stderr, "%s OK\n", msg);
+}
+
+void
+check_elf (const char *msg, bool check)
+{
+  if (! check)
+    {
+      fprintf (stderr, "%s: %s\n", msg, elf_errmsg (-1));
+      exit (-1);
+    }
+  else
+    fprintf (stderr, "%s OK\n", msg);
+}
+
+void
+test (Elf *elf, int class, bool layout)
+{
+  fprintf (stderr, "testing ELF class: %d, layout: %d\n", class, layout);
+
+  check_elf ("gelf_newehdr", gelf_newehdr (elf, class) != 0);
+  check_elf ("gelf_getclass", gelf_getclass (elf) == class);
+
+  check_elf ("elf_flagelf", elf_flagelf (elf, layout ? ELF_C_SET : ELF_C_CLR,
+					 ELF_F_LAYOUT) != 0);
+
+  GElf_Ehdr ehdr;
+  check_elf ("gelf_getehdr", gelf_getehdr (elf, &ehdr) != NULL);
+  check ("e_shnum == 0", ehdr.e_shnum == 0);
+  check ("e_phnum == 0", ehdr.e_phnum == 0);
+  check ("e_shoff == 0", ehdr.e_shoff == 0);
+  check ("e_phoff == 0", ehdr.e_phoff == 0);
+
+  size_t shnum;
+  check_elf ("elf_getshdrnum", elf_getshdrnum (elf, &shnum) == 0);
+  check ("shnum == 0", shnum == 0);
+
+  size_t phnum;
+  check_elf ("elf_getphdrnum", elf_getphdrnum (elf, &phnum) == 0);
+  check ("phnum == 0", phnum == 0);
+
+  /* Lets fill in some info we are always responsible for.  */
+  ehdr.e_ident[EI_DATA] = ELFDATANONE; /* Ask for native encoding.  */
+  ehdr.e_type = ET_EXEC;
+  ehdr.e_machine = EM_386;
+  ehdr.e_version = EV_NONE; /* Ask for current version. */
+  check_elf ("gelf_update_ehdr", gelf_update_ehdr (elf, &ehdr) != 0);
+
+  check_elf ("elf_update", elf_update (elf, ELF_C_NULL) > 0);
+
+  check_elf ("gelf_getehdr", gelf_getehdr (elf, &ehdr) != NULL);
+  check ("EI_DATA", ehdr.e_ident[EI_DATA] != ELFDATANONE);
+  check ("e_version", ehdr.e_version == EV_CURRENT);
+
+  /* The sh/ph values shouldn't have changed.  */
+  check ("e_shnum == 0", ehdr.e_shnum == 0);
+  check ("e_phnum == 0", ehdr.e_phnum == 0);
+  check ("e_shoff == 0", ehdr.e_shoff == 0);
+  check ("e_phoff == 0", ehdr.e_phoff == 0);
+
+  check_elf ("elf_getshdrnum", elf_getshdrnum (elf, &shnum) == 0);
+  check ("shnum == 0", shnum == 0);
+
+  check_elf ("elf_getphdrnum", elf_getphdrnum (elf, &phnum) == 0);
+  check ("phnum == 0", phnum == 0);
+
+  /* Lets add a header.  */
+  check_elf ("elf_newscn", elf_newscn (elf) != NULL);
+  check_elf ("gelf_newphdr", gelf_newphdr (elf, 1) != 0);
+
+  /* If we are responsible for the layout ourselves we should also
+     tell where to put them.  */
+  if (layout)
+    {
+      check_elf ("gelf_getehdr", gelf_getehdr (elf, &ehdr) != NULL);
+      /* phdrs go right after the ehdr.  */
+      ehdr.e_phoff = ehdr.e_ehsize;
+      /* shdrs go right after the phdrs.  */
+      ehdr.e_shoff = ehdr.e_phoff + ehdr.e_phnum * ehdr.e_phentsize;
+      check_elf ("gelf_update_ehdr", gelf_update_ehdr (elf, &ehdr) != 0);
+    }
+
+  check_elf ("elf_update", elf_update (elf, ELF_C_NULL) > 0);
+
+  check_elf ("elf_getshdrnum", elf_getshdrnum (elf, &shnum) == 0);
+  check ("shnum == 1", shnum == 2); /* section zero is also created.  */
+
+  check_elf ("elf_getphdrnum", elf_getphdrnum (elf, &phnum) != 0);
+  check ("phnum == 1", phnum == 1);
+
+  check_elf ("gelf_getehdr", gelf_getehdr (elf, &ehdr) != NULL);
+
+  check ("EI_DATA", ehdr.e_ident[EI_DATA] != ELFDATANONE);
+  check ("e_version", ehdr.e_version == EV_CURRENT);
+
+  check ("e_shnum == 2", ehdr.e_shnum == 2);
+  check ("e_phnum == 1", ehdr.e_phnum == 1);
+  check ("e_shoff != 0", ehdr.e_shoff != 0);
+  check ("e_phoff != 0", ehdr.e_phoff != 0);
+
+  size_t shentsize = (class == ELFCLASS32
+		      ? sizeof (Elf32_Shdr) : sizeof (Elf64_Shdr));
+  check ("e_shentsize", ehdr.e_shentsize == shentsize);
+  size_t phentsize = (class == ELFCLASS32
+		      ? sizeof (Elf32_Phdr) : sizeof (Elf64_Phdr));
+  check ("e_phentsize", ehdr.e_phentsize == phentsize);
+}
+
+int
+main (int argc __attribute__ ((unused)), char **argv __attribute ((unused)))
+{
+  elf_version (EV_CURRENT);
+
+  int fd = fd = open("/dev/zero", O_WRONLY);
+  check ("open", fd >= 0);
+
+  Elf *elf;
+
+  elf = elf_begin (fd, ELF_C_WRITE, NULL);
+  check_elf ("elf_begin", elf != NULL);
+  test (elf, ELFCLASS32, false);
+  elf_end (elf);
+
+  elf = elf_begin (fd, ELF_C_WRITE, NULL);
+  check_elf ("elf_begin", elf != NULL);
+  test (elf, ELFCLASS32, true);
+  elf_end (elf);
+
+  elf = elf_begin (fd, ELF_C_WRITE, NULL);
+  check_elf ("elf_begin", elf != NULL);
+  test (elf, ELFCLASS64, false);
+  elf_end (elf);
+
+  elf = elf_begin (fd, ELF_C_WRITE, NULL);
+  check_elf ("elf_begin", elf != NULL);
+  test (elf, ELFCLASS64, true);
+  elf_end (elf);
+
+  close (fd);
+  return 0;
+}
-- 
2.4.3