This is the mail archive of the
elfutils-devel@sourceware.org
mailing list for the elfutils project.
[PATCH] Move 4 libdwfl nested functions.
- From: Chih-Hung Hsieh <chh at google dot com>
- To: elfutils-devel at lists dot fedorahosted dot org
- Date: Fri, 18 Sep 2015 11:25:13 -0700
- Subject: [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