This is the mail archive of the elfutils-devel@sourceware.org mailing list for the elfutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Move 4 libdwfl nested functions.


Now they should compile with clang.

Used local variables are passed to new file scope functions
as constant parameters, or pointers, or embedded in a
'state' structure.

One simple function "report" is changed to a macro.
It triggers a gcc false positive -Werror=maybe-uninitialized,
so the local variables are explicitly initialized.

Signed-off-by: Chih-Hung Hsieh <chh@google.com>
---
 libdwfl/ChangeLog              |  16 ++++
 libdwfl/linux-kernel-modules.c | 120 +++++++++++++++------------
 libdwfl/relocate.c             | 184 ++++++++++++++++++++++-------------------
 3 files changed, 182 insertions(+), 138 deletions(-)

diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 0ab386f..b9f42a1 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,19 @@
+2015-09-18  Chih-Hung Hsieh  <chh@google.com>
+
+	* relocate.c (relocate_section): Move nested function "relocate"
+	to file scope, pass all used local variables as constants.
+	Move nested "check_badreltype" to file scope, pass addresses of
+	updated used local variables.
+	* linux-kernel-modules.c (intuit_kernel_bounds): Move nested function
+	"read_address" to file scope, pass all updated local variables in
+	a state structure.
+	* linux-kernel-modules.c (dwfl_linux_kernel_find_elf): Move nested
+	function "subst_name" to file scope, pass all used local variables
+	as constants.
+	* linux-kernel-modules.c (dwfl_linux_kernel_report_kernel): Replace
+	simple nested function "report" with a macro. Work around gcc static
+	analysis error -Werror=maybe-uninitialized.
+
 2015-09-09  Chih-Hung Hsieh  <chh@google.com>
 	    Mark Wielaard  <mjw@redhat.com>
 
diff --git a/libdwfl/linux-kernel-modules.c b/libdwfl/linux-kernel-modules.c
index 236e2cd..dafe893 100644
--- a/libdwfl/linux-kernel-modules.c
+++ b/libdwfl/linux-kernel-modules.c
@@ -440,46 +440,55 @@ dwfl_linux_kernel_report_offline (Dwfl *dwfl, const char *release,
 INTDEF (dwfl_linux_kernel_report_offline)
 
 
+/* State of read_address used by intuit_kernel_bounds. */
+struct read_address_state {
+  FILE *f;
+  char *line;
+  size_t linesz;
+  size_t n;
+  char *p;
+  const char *type;
+};
+
+static inline bool
+read_address (struct read_address_state *state, Dwarf_Addr *addr)
+{
+  if ((state->n = getline (&state->line, &state->linesz, state->f)) < 1 ||
+      state->line[state->n - 2] == ']')
+    return false;
+  *addr = strtoull (state->line, &state->p, 16);
+  state->p += strspn (state->p, " \t");
+  state->type = strsep (&state->p, " \t\n");
+  if (state->type == NULL)
+    return false;
+  return state->p != NULL && state->p != state->line;
+}
+
+
 /* Grovel around to guess the bounds of the runtime kernel image.  */
 static int
 intuit_kernel_bounds (Dwarf_Addr *start, Dwarf_Addr *end, Dwarf_Addr *notes)
 {
-  FILE *f = fopen (KSYMSFILE, "r");
-  if (f == NULL)
+  struct read_address_state state = { NULL, NULL, 0, 0, NULL, NULL };
+
+  state.f = fopen (KSYMSFILE, "r");
+  if (state.f == NULL)
     return errno;
 
-  (void) __fsetlocking (f, FSETLOCKING_BYCALLER);
+  (void) __fsetlocking (state.f, FSETLOCKING_BYCALLER);
 
   *notes = 0;
 
-  char *line = NULL;
-  size_t linesz = 0;
-  size_t n;
-  char *p = NULL;
-  const char *type;
-
-  inline bool read_address (Dwarf_Addr *addr)
-  {
-    if ((n = getline (&line, &linesz, f)) < 1 || line[n - 2] == ']')
-      return false;
-    *addr = strtoull (line, &p, 16);
-    p += strspn (p, " \t");
-    type = strsep (&p, " \t\n");
-    if (type == NULL)
-      return false;
-    return p != NULL && p != line;
-  }
-
   int result;
   do
-    result = read_address (start) ? 0 : -1;
-  while (result == 0 && strchr ("TtRr", *type) == NULL);
+    result = read_address (&state, start) ? 0 : -1;
+  while (result == 0 && strchr ("TtRr", *state.type) == NULL);
 
   if (result == 0)
     {
       *end = *start;
-      while (read_address (end))
-	if (*notes == 0 && !strcmp (p, "__start_notes\n"))
+      while (read_address (&state, end))
+	if (*notes == 0 && !strcmp (state.p, "__start_notes\n"))
 	  *notes = *end;
 
       Dwarf_Addr round_kernel = sysconf (_SC_PAGE_SIZE);
@@ -489,12 +498,12 @@ intuit_kernel_bounds (Dwarf_Addr *start, Dwarf_Addr *end, Dwarf_Addr *notes)
       if (*start >= *end || *end - *start < round_kernel)
 	result = -1;
     }
-  free (line);
+  free (state.line);
 
   if (result == -1)
-    result = ferror_unlocked (f) ? errno : ENOEXEC;
+    result = ferror_unlocked (state.f) ? errno : ENOEXEC;
 
-  fclose (f);
+  fclose (state.f);
 
   return result;
 }
@@ -619,12 +628,11 @@ check_module_notes (Dwfl_Module *mod)
 int
 dwfl_linux_kernel_report_kernel (Dwfl *dwfl)
 {
-  Dwarf_Addr start;
-  Dwarf_Addr end;
-  inline Dwfl_Module *report (void)
-    {
-      return INTUSE(dwfl_report_module) (dwfl, KERNEL_MODNAME, start, end);
-    }
+  Dwarf_Addr start = 0;
+  Dwarf_Addr end = 0;
+
+  #define report() \
+    (INTUSE(dwfl_report_module) (dwfl, KERNEL_MODNAME, start, end))
 
   /* This is a bit of a kludge.  If we already reported the kernel,
      don't bother figuring it out again--it never changes.  */
@@ -657,6 +665,29 @@ dwfl_linux_kernel_report_kernel (Dwfl *dwfl)
 INTDEF (dwfl_linux_kernel_report_kernel)
 
 
+static inline bool
+subst_name (char from, char to,
+            const char * const module_name,
+            char * const alternate_name,
+            const size_t namelen)
+{
+  const char *n = memchr (module_name, from, namelen);
+  if (n == NULL)
+    return false;
+  char *a = mempcpy (alternate_name, module_name, n - module_name);
+  *a++ = to;
+  ++n;
+  const char *p;
+  while ((p = memchr (n, from, namelen - (n - module_name))) != NULL)
+    {
+      a = mempcpy (a, n, p - n);
+      *a++ = to;
+      n = p + 1;
+    }
+  memcpy (a, n, namelen - (n - module_name) + 1);
+  return true;
+}
+
 /* Dwfl_Callbacks.find_elf for the running Linux kernel and its modules.  */
 
 int
@@ -713,25 +744,8 @@ dwfl_linux_kernel_find_elf (Dwfl_Module *mod,
       free (modulesdir[0]);
       return ENOMEM;
     }
-  inline bool subst_name (char from, char to)
-    {
-      const char *n = memchr (module_name, from, namelen);
-      if (n == NULL)
-	return false;
-      char *a = mempcpy (alternate_name, module_name, n - module_name);
-      *a++ = to;
-      ++n;
-      const char *p;
-      while ((p = memchr (n, from, namelen - (n - module_name))) != NULL)
-	{
-	  a = mempcpy (a, n, p - n);
-	  *a++ = to;
-	  n = p + 1;
-	}
-      memcpy (a, n, namelen - (n - module_name) + 1);
-      return true;
-    }
-  if (!subst_name ('-', '_') && !subst_name ('_', '-'))
+  if (!subst_name ('-', '_', module_name, alternate_name, namelen) &&
+      !subst_name ('_', '-', module_name, alternate_name, namelen))
     alternate_name[0] = '\0';
 
   FTSENT *f;
diff --git a/libdwfl/relocate.c b/libdwfl/relocate.c
index e102e1e..2dc6737 100644
--- a/libdwfl/relocate.c
+++ b/libdwfl/relocate.c
@@ -277,77 +277,18 @@ resolve_symbol (Dwfl_Module *referer, struct reloc_symtab_cache *symtab,
   return DWFL_E_RELUNDEF;
 }
 
+/* Apply one relocation.  Returns true for any invalid data.  */
 static Dwfl_Error
-relocate_section (Dwfl_Module *mod, Elf *relocated, const GElf_Ehdr *ehdr,
-		  size_t shstrndx, struct reloc_symtab_cache *reloc_symtab,
-		  Elf_Scn *scn, GElf_Shdr *shdr,
-		  Elf_Scn *tscn, bool debugscn, bool partial)
+relocate (Dwfl_Module * const mod,
+          Elf * const relocated,
+          struct reloc_symtab_cache * const reloc_symtab,
+          Elf_Data * const tdata,
+          const GElf_Ehdr * const ehdr,
+          GElf_Addr offset,
+          const GElf_Sxword *addend,
+          int rtype,
+          int symndx)
 {
-  /* First, fetch the name of the section these relocations apply to.  */
-  GElf_Shdr tshdr_mem;
-  GElf_Shdr *tshdr = gelf_getshdr (tscn, &tshdr_mem);
-  const char *tname = elf_strptr (relocated, shstrndx, tshdr->sh_name);
-  if (tname == NULL)
-    return DWFL_E_LIBELF;
-
-  if (unlikely (tshdr->sh_type == SHT_NOBITS) || unlikely (tshdr->sh_size == 0))
-    /* No contents to relocate.  */
-    return DWFL_E_NOERROR;
-
-  if (debugscn && ! ebl_debugscn_p (mod->ebl, tname))
-    /* This relocation section is not for a debugging section.
-       Nothing to do here.  */
-    return DWFL_E_NOERROR;
-
-  /* Fetch the section data that needs the relocations applied.  */
-  Elf_Data *tdata = elf_rawdata (tscn, NULL);
-  if (tdata == NULL)
-    return DWFL_E_LIBELF;
-
-  /* If either the section that needs the relocation applied, or the
-     section that the relocations come from overlap one of the ehdrs,
-     shdrs or phdrs data then we refuse to do the relocations.  It
-     isn't illegal for ELF section data to overlap the header data,
-     but updating the (relocation) data might corrupt the in-memory
-     libelf headers causing strange corruptions or errors.  */
-  size_t ehsize = gelf_fsize (relocated, ELF_T_EHDR, 1, EV_CURRENT);
-  if (unlikely (shdr->sh_offset < ehsize
-		|| tshdr->sh_offset < ehsize))
-    return DWFL_E_BADELF;
-
-  GElf_Off shdrs_start = ehdr->e_shoff;
-  size_t shnums;
-  if (elf_getshdrnum (relocated, &shnums) < 0)
-    return DWFL_E_LIBELF;
-  /* Overflows will have been checked by elf_getshdrnum/get|rawdata.  */
-  size_t shentsize = gelf_fsize (relocated, ELF_T_SHDR, 1, EV_CURRENT);
-  GElf_Off shdrs_end = shdrs_start + shnums * shentsize;
-  if (unlikely ((shdrs_start < shdr->sh_offset + shdr->sh_size
-		 && shdr->sh_offset < shdrs_end)
-		|| (shdrs_start < tshdr->sh_offset + tshdr->sh_size
-		    && tshdr->sh_offset < shdrs_end)))
-    return DWFL_E_BADELF;
-
-  GElf_Off phdrs_start = ehdr->e_phoff;
-  size_t phnums;
-  if (elf_getphdrnum (relocated, &phnums) < 0)
-    return DWFL_E_LIBELF;
-  if (phdrs_start != 0 && phnums != 0)
-    {
-      /* Overflows will have been checked by elf_getphdrnum/get|rawdata.  */
-      size_t phentsize = gelf_fsize (relocated, ELF_T_PHDR, 1, EV_CURRENT);
-      GElf_Off phdrs_end = phdrs_start + phnums * phentsize;
-      if (unlikely ((phdrs_start < shdr->sh_offset + shdr->sh_size
-		     && shdr->sh_offset < phdrs_end)
-		    || (phdrs_start < tshdr->sh_offset + tshdr->sh_size
-			&& tshdr->sh_offset < phdrs_end)))
-	return DWFL_E_BADELF;
-    }
-
-  /* Apply one relocation.  Returns true for any invalid data.  */
-  Dwfl_Error relocate (GElf_Addr offset, const GElf_Sxword *addend,
-		       int rtype, int symndx)
-  {
     /* First see if this is a reloc we can handle.
        If we are skipping it, don't bother resolving the symbol.  */
 
@@ -482,7 +423,89 @@ relocate_section (Dwfl_Module *mod, Elf *relocated, const GElf_Ehdr *ehdr,
 
     /* We have applied this relocation!  */
     return DWFL_E_NOERROR;
-  }
+}
+
+static inline void
+check_badreltype (bool *first_badreltype,
+                  Dwfl_Module *mod,
+                  Dwfl_Error *result)
+{
+  if (*first_badreltype)
+    {
+       *first_badreltype = false;
+       if (ebl_get_elfmachine (mod->ebl) == EM_NONE)
+          /* This might be because ebl_openbackend failed to find
+             any libebl_CPU.so library.  Diagnose that clearly.  */
+          *result = DWFL_E_UNKNOWN_MACHINE;
+     }
+}
+
+static Dwfl_Error
+relocate_section (Dwfl_Module *mod, Elf *relocated, const GElf_Ehdr *ehdr,
+		  size_t shstrndx, struct reloc_symtab_cache *reloc_symtab,
+		  Elf_Scn *scn, GElf_Shdr *shdr,
+		  Elf_Scn *tscn, bool debugscn, bool partial)
+{
+  /* First, fetch the name of the section these relocations apply to.  */
+  GElf_Shdr tshdr_mem;
+  GElf_Shdr *tshdr = gelf_getshdr (tscn, &tshdr_mem);
+  const char *tname = elf_strptr (relocated, shstrndx, tshdr->sh_name);
+  if (tname == NULL)
+    return DWFL_E_LIBELF;
+
+  if (unlikely (tshdr->sh_type == SHT_NOBITS) || unlikely (tshdr->sh_size == 0))
+    /* No contents to relocate.  */
+    return DWFL_E_NOERROR;
+
+  if (debugscn && ! ebl_debugscn_p (mod->ebl, tname))
+    /* This relocation section is not for a debugging section.
+       Nothing to do here.  */
+    return DWFL_E_NOERROR;
+
+  /* Fetch the section data that needs the relocations applied.  */
+  Elf_Data *tdata = elf_rawdata (tscn, NULL);
+  if (tdata == NULL)
+    return DWFL_E_LIBELF;
+
+  /* If either the section that needs the relocation applied, or the
+     section that the relocations come from overlap one of the ehdrs,
+     shdrs or phdrs data then we refuse to do the relocations.  It
+     isn't illegal for ELF section data to overlap the header data,
+     but updating the (relocation) data might corrupt the in-memory
+     libelf headers causing strange corruptions or errors.  */
+  size_t ehsize = gelf_fsize (relocated, ELF_T_EHDR, 1, EV_CURRENT);
+  if (unlikely (shdr->sh_offset < ehsize
+		|| tshdr->sh_offset < ehsize))
+    return DWFL_E_BADELF;
+
+  GElf_Off shdrs_start = ehdr->e_shoff;
+  size_t shnums;
+  if (elf_getshdrnum (relocated, &shnums) < 0)
+    return DWFL_E_LIBELF;
+  /* Overflows will have been checked by elf_getshdrnum/get|rawdata.  */
+  size_t shentsize = gelf_fsize (relocated, ELF_T_SHDR, 1, EV_CURRENT);
+  GElf_Off shdrs_end = shdrs_start + shnums * shentsize;
+  if (unlikely ((shdrs_start < shdr->sh_offset + shdr->sh_size
+		 && shdr->sh_offset < shdrs_end)
+		|| (shdrs_start < tshdr->sh_offset + tshdr->sh_size
+		    && tshdr->sh_offset < shdrs_end)))
+    return DWFL_E_BADELF;
+
+  GElf_Off phdrs_start = ehdr->e_phoff;
+  size_t phnums;
+  if (elf_getphdrnum (relocated, &phnums) < 0)
+    return DWFL_E_LIBELF;
+  if (phdrs_start != 0 && phnums != 0)
+    {
+      /* Overflows will have been checked by elf_getphdrnum/get|rawdata.  */
+      size_t phentsize = gelf_fsize (relocated, ELF_T_PHDR, 1, EV_CURRENT);
+      GElf_Off phdrs_end = phdrs_start + phnums * phentsize;
+      if (unlikely ((phdrs_start < shdr->sh_offset + shdr->sh_size
+		     && shdr->sh_offset < phdrs_end)
+		    || (phdrs_start < tshdr->sh_offset + tshdr->sh_size
+			&& tshdr->sh_offset < phdrs_end)))
+	return DWFL_E_BADELF;
+    }
 
   /* Fetch the relocation section and apply each reloc in it.  */
   Elf_Data *reldata = elf_getdata (scn, NULL);
@@ -491,17 +514,6 @@ relocate_section (Dwfl_Module *mod, Elf *relocated, const GElf_Ehdr *ehdr,
 
   Dwfl_Error result = DWFL_E_NOERROR;
   bool first_badreltype = true;
-  inline void check_badreltype (void)
-  {
-    if (first_badreltype)
-      {
-	first_badreltype = false;
-	if (ebl_get_elfmachine (mod->ebl) == EM_NONE)
-	  /* This might be because ebl_openbackend failed to find
-	     any libebl_CPU.so library.  Diagnose that clearly.  */
-	  result = DWFL_E_UNKNOWN_MACHINE;
-      }
-  }
 
   size_t sh_entsize
     = gelf_fsize (relocated, shdr->sh_type == SHT_REL ? ELF_T_REL : ELF_T_RELA,
@@ -514,10 +526,11 @@ relocate_section (Dwfl_Module *mod, Elf *relocated, const GElf_Ehdr *ehdr,
 	GElf_Rel rel_mem, *r = gelf_getrel (reldata, relidx, &rel_mem);
 	if (r == NULL)
 	  return DWFL_E_LIBELF;
-	result = relocate (r->r_offset, NULL,
+	result = relocate (mod, relocated, reloc_symtab, tdata, ehdr,
+			   r->r_offset, NULL,
 			   GELF_R_TYPE (r->r_info),
 			   GELF_R_SYM (r->r_info));
-	check_badreltype ();
+	check_badreltype (&first_badreltype, mod, &result);
 	if (partial)
 	  switch (result)
 	    {
@@ -543,10 +556,11 @@ relocate_section (Dwfl_Module *mod, Elf *relocated, const GElf_Ehdr *ehdr,
 					       &rela_mem);
 	if (r == NULL)
 	  return DWFL_E_LIBELF;
-	result = relocate (r->r_offset, &r->r_addend,
+	result = relocate (mod, relocated, reloc_symtab, tdata, ehdr,
+			   r->r_offset, &r->r_addend,
 			   GELF_R_TYPE (r->r_info),
 			   GELF_R_SYM (r->r_info));
-	check_badreltype ();
+	check_badreltype (&first_badreltype, mod, &result);
 	if (partial)
 	  switch (result)
 	    {
-- 
2.6.0.rc0.131.gf624c3d


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]