Re: Pending nested function removal
On Thu, Jan 28, 2021, 13:16 Timm Bäder wrote: > On 28/01/2021 06:19, Navin P via Elfutils-devel wrote: > > Hi, > > I tried to compile the nested functions and most of the files are > > complete. I find few others still pending. Below is the list which > > still has a nested function. > > I'm attaching the logfile for line numbers and column no. > > > > grep "function definition is not allowed here" logfile > > elf-from-memory.c:234:2: error: function definition is not allowed here > > elf-from-memory.c:314:2: error: function definition is not allowed here > > elf-from-memory: > https://sourceware.org/pipermail/elfutils-devel/2021q1/003373.html > > > readelf.c:391:3: error: function definition is not allowed here > > readelf.c:3617:7: error: function definition is not allowed here > > readelf.c:6222:3: error: function definition is not allowed here > > readelf.c:8747:7: error: function definition is not allowed here > > readelf.c:12119:3: error: function definition is not allowed here > > readelf: > https://sourceware.org/pipermail/elfutils-devel/2021q1/003381.html > > > strip.c:524:4: error: function definition is not allowed here > > strip.c:1540:8: error: function definition is not allowed here > > strip.c:2131:2: error: function definition is not allowed here > > strip.c:2159:6: error: function definition is not allowed here > > strip: > https://sourceware.org/pipermail/elfutils-devel/2021q1/003368.html > > > > elflint.c:3450:3: error: function definition is not allowed here > > elflint.c:3463:3: error: function definition is not allowed here > > addr2line.c:697:4: error: function definition is not allowed here > > addr2line.c:704:4: error: function definition is not allowed here > > arlib-argp.c:63:3: error: function definition is not allowed here > > ar.c:449:3: error: function definition is not allowed here > > addr2line, arlib-argp, ar, zstrptr: > https://sourceware.org/pipermail/elfutils-devel/2021q1/003376.html > > > unstrip.c:443:5: error: function definition is not allowed here > > unstrip.c:1202:5: error: function definition is not allowed here > > unstrip.c:1453:5: error: function definition is not allowed here > > unstrip.c:2236:3: error: function definition is not allowed here > > unstrip.c:2496:5: error: function definition is not allowed here > > elfcompress.c:280:3: error: function definition is not allowed here > > elfcompress.c:285:3: error: function definition is not allowed here > > elfcompress.c:291:3: error: function definition is not allowed here > > elfcompress.c:299:3: error: function definition is not allowed here > > I have the others locally but wanted feedback on the patches I posted > first. Maybe I should re-post them with the changelog entries > I forgot :) > Yes, please merge them.The already committed ones are good. Please update when you are done >
Re: [PATCH 1/4] strip: Replace nested check_preserved function with loop
Hi Timm, OK, this simply expands the inlined check_preserved function and keeps track of the section indexes with the new shdr_indices array that is populated with the sh_link and possibly the sh_info indices. Added a ChangeLog entry and pushed. Thanks, Mark
Re: [PATCH 2/4] strip: Pull relocate() info file scope
Hi Timm, On Fri, Jan 08, 2021 at 09:04:47AM +0100, Timm Bäder via Elfutils-devel wrote: > Pull relocate() info file scope and get rid of a nested function this > way. Refactor remove_debug_relocations() to minimize the parameters we > need to pass to relocate(). OK. The diff makes it slightly hard to see whether things were moved/updated correctly, but if we missed something I am sure the buildbots will catch it. I added a ChangeLog entry and pushed. Cheers, Mark
Re: [PATCH 3/4] strip: Pull update_section_size() into file scope
Hi Timm, On Fri, Jan 08, 2021 at 09:04:48AM +0100, Timm Bäder via Elfutils-devel wrote: > Get rid of a nested function this way. OK, it is a little messy IMHO that we have to pass 4 extra arguments to the function now, but it seems straightforward. Added a ChangeLog entry and pushed. Thanks, Mark
Re: [PATCH 4/4] strip: Remove no_symtab_updates() function
Hi Timm,
On Fri, Jan 08, 2021 at 09:04:49AM +0100, Timm Bäder via Elfutils-devel wrote:
> The no_symtab_updates() function was being called at the beginning of
> all case labels in this switch, so we can just call it once before the
> switch. Then it only has one call-site, so inline this short function
> there.
Yes, this is actually nicer to read. I added a ChangeLog entry and
made one tiny change so we don't need to add brackets to scope the
case label, preventing extra indentation which made the patch harder
to read.
Pushed as attached,
Mark>From e29965f401896a3652394df8e28cc14979fedc91 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?=
Date: Fri, 8 Jan 2021 09:04:49 +0100
Subject: [PATCH] strip: Remove no_symtab_updates() function
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The no_symtab_updates() function was being called at the beginning of
all case labels in this switch, so we can just call it once before the
switch. Then it only has one call-site, so inline this short function
there.
Signed-off-by: Timm Bäder
---
src/ChangeLog | 6 +
src/strip.c | 74 ---
2 files changed, 35 insertions(+), 45 deletions(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index d1d9a8bf..4fc54ce7 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,9 @@
+2021-01-08 Timm Bäder
+
+ * strip.c (handle_elf): Remove no_symtab_updates function and
+ calls inside the switch statement. Add checks and (possibly)
+ continue, before switch statement is called.
+
2021-01-08 Timm Bäder
* strip.c (handle_elf): Move inlined update_section_size function
diff --git a/src/strip.c b/src/strip.c
index e608dc5e..7a5d4e4c 100644
--- a/src/strip.c
+++ b/src/strip.c
@@ -2175,49 +2175,43 @@ handle_elf (int fd, Elf *elf, const char *prefix, const char *fname,
/* Find all relocation sections which use this symbol table. */
for (cnt = 1; cnt <= shdridx; ++cnt)
{
- if (shdr_info[cnt].idx == 0 && debug_fname == NULL)
+ struct shdr_info *info = &shdr_info[cnt];
+ if (info->idx == 0 && debug_fname == NULL)
/* Ignore sections which are discarded. When we are saving a
relocation section in a separate debug file, we must fix up
the symbol table references. */
continue;
- const Elf32_Word symtabidx = shdr_info[cnt].old_sh_link;
+ const Elf32_Word symtabidx = info->old_sh_link;
elf_assert (symtabidx < shnum + 2);
const Elf32_Word *const newsymidx = shdr_info[symtabidx].newsymidx;
- switch (shdr_info[cnt].shdr.sh_type)
- {
- inline bool no_symtab_updates (void)
- {
- /* If the symbol table hasn't changed, do not do anything. */
- if (shdr_info[symtabidx].newsymidx == NULL)
- return true;
-
- /* If the symbol table is not discarded, but additionally
- duplicated in the separate debug file and this section
- is discarded, don't adjust anything. */
- return (shdr_info[cnt].idx == 0
- && shdr_info[symtabidx].debug_data != NULL);
- }
+ /* If the symbol table hasn't changed, do not do anything. */
+ if (newsymidx == NULL)
+ continue;
+
+ /* If the symbol table is not discarded, but additionally
+ duplicated in the separate debug file and this section
+ is discarded, don't adjust anything. */
+ if (info->idx == 0 && shdr_info[symtabidx].debug_data != NULL)
+ continue;
+
+ switch (info->shdr.sh_type)
+ {
case SHT_REL:
case SHT_RELA:
- if (no_symtab_updates ())
- break;
-
- Elf_Data *d = elf_getdata (shdr_info[cnt].idx == 0
- ? elf_getscn (debugelf, cnt)
- : elf_getscn (newelf,
- shdr_info[cnt].idx),
- NULL);
+ scn = (info->idx == 0
+ ? elf_getscn (debugelf, cnt)
+ : elf_getscn (newelf, info->idx));
+ Elf_Data *d = elf_getdata (scn, NULL);
elf_assert (d != NULL && d->d_buf != NULL
- && shdr_info[cnt].shdr.sh_entsize != 0);
- size_t nrels = (shdr_info[cnt].shdr.sh_size
- / shdr_info[cnt].shdr.sh_entsize);
+ && info->shdr.sh_entsize != 0);
+ size_t nrels = (info->shdr.sh_size / info->shdr.sh_entsize);
size_t symsize = gelf_fsize (elf, ELF_T_SYM, 1, EV_CURRENT);
const Elf32_Word symidxn = (shdr_info[symtabidx].data->d_size
/ symsize);
- if (shdr_info[cnt].shdr.sh_type == SHT_REL)
+ if (info->shdr.sh_type == SHT_REL)
for (size_t relidx = 0; relidx < nrels; ++relidx)
{
GElf_Rel rel_mem;
@@ -2258,15 +2252,12 @@ handle_elf (int fd, Elf *elf, const char *prefix, const char *fname,
break;
case SHT_HASH:
- if (no_symtab_updates ())
- break;
-
/* We have to recompute the hash table. */
- elf_assert (shdr_info[cnt].idx > 0);
+ elf_assert (info->idx > 0);
/* The hash section in the new file. */
- scn = elf_getscn (newelf, shdr_info[cnt].idx);
+ scn = elf_getscn (newelf, info->idx);
/* The symbol table data. */
Elf_Data *
Re: [PATCH 1/2] elf-from-memory: Restructure code to get rid of nested handle_segment()
Hi Timm, On Fri, Jan 08, 2021 at 09:09:55AM +0100, Timm Bäder via Elfutils-devel wrote: > Use one loop for both 32 and 64 bit case. This allows for only one call > site of the old handle_segment(), which we can then inline into the for > loop. It is a bit hard to see because of the reindenting, but the new code does indeed do the same as the old code. Added a ChangeLog entry and pushed. Thanks, Mark
Re: [PATCH 2/2] elf-from-memory: Refactor to get rid of nested function
Hi Timm, On Fri, Jan 08, 2021 at 09:09:56AM +0100, Timm Bäder via Elfutils-devel wrote: > Try to unify the 32/64 bit code paths and get rid of the nested > handle_segment() this way. The new code does seem to do the same thing as the old code. Added a ChangeLog entry and pushed. Thanks, Mark
