* [PATCH] Fix section symbols in objcopy when removing sections with relocations from Elf objects
@ 2006-05-24 16:20 Julian Brown
2006-05-24 17:23 ` Julian Brown
2006-05-25 13:28 ` Alan Modra
0 siblings, 2 replies; 6+ messages in thread
From: Julian Brown @ 2006-05-24 16:20 UTC (permalink / raw)
To: binutils; +Cc: Richard Earnshaw, Julian Brown, Paul Brook
[-- Attachment #1: Type: text/plain, Size: 4089 bytes --]
Hi,
This patch fixes a problem where, if sections are removed during objcopy
with Elf object files, the section symbols corresponding to the
relocation sections for the removed sections may be left in the target
object file.
In my test case, the file being manipulated is a "crt1.o", and the
command-line to do the stripping is:
arm-none-linux-gnueabi-objcopy -R .debug_info [-R more debug sections]
/path/to/crt1.o
I.e. "-R" is being used to remove various debugging sections, which in
the source object file contain sections such as ".debug_info" along with
a corresponding ".rel.debug_info" containing relocations. The -R switch
removes both the section itself and its relocations.
However, the resulting file contains invalid section symbols like so:
Symbol table '.symtab' contains 34 entries:
Num: Value Size Type Bind Vis Ndx Name
...
11: 00000000 0 SECTION LOCAL DEFAULT 14
12: 00000000 0 SECTION LOCAL DEFAULT 16
13: 00000000 0 SECTION LOCAL DEFAULT 19
14: 00000000 0 SECTION LOCAL DEFAULT 10
15: 00000000 0 SECTION LOCAL DEFAULT 22
16: 00000000 0 SECTION LOCAL DEFAULT 23
17: 00000000 0 SECTION LOCAL DEFAULT 24
Even though the sections in the file are now only numbered up to 13.
These symbols correspond to the relocation sections for the removed
debug sections. Internally, bfd knows little about these sections --
they are folded into the bfd section containing information on the
relocatee during Elf parsing.
Fixing this cleanly isn't particularly easy, because there isn't any one
point at which all the right information seems to be readily available.
The obvious point to remove the extraneous symbols is in
objcopy.c:filter_symbols(), though that function is supposed to be
target-independent, so this involves an Elf-specific hack.
Other choices might have been to remove the bad symbols whilst parsing
in or writing out the Elf file, though since the right information isn't
really available in either of those places either, that would probably
involve similar or worse hacks (elf.c:bfd_section_from_shdr knows
nothing about symbols, and elf.c:swap_out_symbols knows nothing about
the input bfd).
Once the bad symbol sections are removed, another problem becomes
apparent, which is that higher-numbered section symbols (for .shstrtab,
.symtab, .strtab) are not rewritten to point to their now lower-numbered
sections correctly. I think there's existing logic which is supposed to
handle this, e.g. elf.c:swap_out_syms has a comment:
/* This symbol is in a real ELF section which we did
not create as a BFD section. Undo the mapping done
by copy_private_symbol_data. */
With code following to handle several specific sections, including the
ones which are now numbered wrongly in this case. Unfortunately, during
an objcopy invocation, copy_private_symbol_data isn't actually ever
called to set up the mapping before this reverse mapping takes place, so
I've added a call to it in filter_symbols. I then discovered that the
necessary private fields to make this to work aren't set when reading in
the source file, so I've added those (to bfd_section_from_shdr).
I've tested the patch against the proprietary linker which originally
barfed on the bad crt1.o file, and against hand-written tests (based on
tweaked compiler output with -ffunction-sections turned on), and I
believe it DTRT, at least in those cases. It's also tested with "make
check" for --enable-targets=all.
The patch is currently against the CSL branch. OK to apply there? On
mainline? Does this need a new testcase?
Cheers,
Julian
ChangeLog:
bfd/
* elf.c (bfd_section_from_shdr): Set symtab_shndx_section,
shstrtab_section, strtab_section in ELF private data.
binutils/
* objcopy.c (filter_symbols): Filter out section symbols for
sections which are being stripped and their associated relocation
sections. Call bfd_copy_private_symbol_data for each symbol which
passes the filter.
[-- Attachment #2: section-stripping-6 --]
[-- Type: text/plain, Size: 3888 bytes --]
Index: bfd/elf.c
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.329
diff -c -p -r1.329 elf.c
*** bfd/elf.c 16 Mar 2006 12:20:15 -0000 1.329
--- bfd/elf.c 24 May 2006 12:52:58 -0000
*************** bfd_section_from_shdr (bfd *abfd, unsign
*** 1935,1940 ****
--- 1935,1941 ----
BFD_ASSERT (elf_symtab_shndx (abfd) == 0);
elf_symtab_shndx (abfd) = shindex;
elf_tdata (abfd)->symtab_shndx_hdr = *hdr;
+ elf_tdata (abfd)->symtab_shndx_section = shindex;
elf_elfsections (abfd)[shindex] = &elf_tdata (abfd)->symtab_shndx_hdr;
return TRUE;
*************** bfd_section_from_shdr (bfd *abfd, unsign
*** 1944,1949 ****
--- 1945,1951 ----
if (ehdr->e_shstrndx == shindex)
{
elf_tdata (abfd)->shstrtab_hdr = *hdr;
+ elf_tdata (abfd)->shstrtab_section = shindex;
elf_elfsections (abfd)[shindex] = &elf_tdata (abfd)->shstrtab_hdr;
return TRUE;
}
*************** bfd_section_from_shdr (bfd *abfd, unsign
*** 1951,1956 ****
--- 1953,1959 ----
{
symtab_strtab:
elf_tdata (abfd)->strtab_hdr = *hdr;
+ elf_tdata (abfd)->strtab_section = shindex;
elf_elfsections (abfd)[shindex] = &elf_tdata (abfd)->strtab_hdr;
return TRUE;
}
Index: binutils/objcopy.c
===================================================================
RCS file: /cvs/src/src/binutils/objcopy.c,v
retrieving revision 1.95
diff -c -p -r1.95 objcopy.c
*** binutils/objcopy.c 28 Feb 2006 16:09:01 -0000 1.95
--- binutils/objcopy.c 24 May 2006 12:52:58 -0000
*************** filter_symbols (bfd *abfd, bfd *obfd, as
*** 915,920 ****
--- 915,959 ----
|| undefined
|| bfd_is_com_section (bfd_get_section (sym)))
keep = strip_symbols != STRIP_UNNEEDED;
+ else if ((flags & BSF_SECTION_SYM) != 0) /* Section symbol. */
+ {
+ struct bfd_section *section = sym->section;
+
+ keep = 1;
+
+ if (!bfd_is_abs_section (section)
+ && is_strip_section (abfd, section))
+ keep = 0;
+ else if (bfd_get_flavour (abfd) == bfd_target_elf_flavour)
+ {
+ Elf_Internal_Shdr **elfsections = elf_elfsections (abfd);
+ unsigned int sec;
+
+ for (sec = 0; sec < elf_numsections (abfd); sec++)
+ {
+ Elf_Internal_Shdr *elfhdr = elfsections[sec];
+ if ((elfhdr->sh_type == SHT_REL
+ || elfhdr->sh_type == SHT_RELA))
+ {
+ char *secname =
+ bfd_elf_string_from_elf_section (abfd,
+ elf_elfheader (abfd)->e_shstrndx,
+ elfhdr->sh_name);
+
+ if (strcmp (secname, name) == 0)
+ {
+ struct bfd_section *targsec =
+ bfd_section_from_elf_index (abfd, elfhdr->sh_info);
+ if (targsec && is_strip_section (abfd, targsec))
+ {
+ keep = 0;
+ break;
+ }
+ }
+ }
+ }
+ }
+ }
else if ((flags & BSF_DEBUGGING) != 0) /* Debugging symbol. */
keep = (strip_symbols != STRIP_DEBUG
&& strip_symbols != STRIP_UNNEEDED
*************** filter_symbols (bfd *abfd, bfd *obfd, as
*** 969,975 ****
sym->flags |= BSF_GLOBAL;
}
! to[dst_count++] = sym;
}
}
--- 1008,1016 ----
sym->flags |= BSF_GLOBAL;
}
! to[dst_count] = sym;
! bfd_copy_private_symbol_data (abfd, sym, obfd, to[dst_count]);
! dst_count++;
}
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix section symbols in objcopy when removing sections with relocations from Elf objects
2006-05-24 16:20 [PATCH] Fix section symbols in objcopy when removing sections with relocations from Elf objects Julian Brown
@ 2006-05-24 17:23 ` Julian Brown
2006-05-25 0:58 ` H. J. Lu
2006-05-25 13:28 ` Alan Modra
1 sibling, 1 reply; 6+ messages in thread
From: Julian Brown @ 2006-05-24 17:23 UTC (permalink / raw)
To: Julian Brown; +Cc: binutils, Richard Earnshaw, Paul Brook
[-- Attachment #1: Type: text/plain, Size: 471 bytes --]
Julian Brown wrote:
> Hi,
>
> This patch fixes a problem where, if sections are removed during objcopy
> with Elf object files, the section symbols corresponding to the
> relocation sections for the removed sections may be left in the target
> object file.
This is a slightly-smarter version (thanks to offlist discussion with
Dan) which knows about recoving Elf symbols from BFD symbols, hence
avoids some unnecessary work.
Changelog as before.
Cheers,
Julian
[-- Attachment #2: section-stripping-7 --]
[-- Type: text/plain, Size: 3364 bytes --]
Index: bfd/elf.c
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.329
diff -c -p -r1.329 elf.c
*** bfd/elf.c 16 Mar 2006 12:20:15 -0000 1.329
--- bfd/elf.c 24 May 2006 14:21:21 -0000
*************** bfd_section_from_shdr (bfd *abfd, unsign
*** 1935,1940 ****
--- 1935,1941 ----
BFD_ASSERT (elf_symtab_shndx (abfd) == 0);
elf_symtab_shndx (abfd) = shindex;
elf_tdata (abfd)->symtab_shndx_hdr = *hdr;
+ elf_tdata (abfd)->symtab_shndx_section = shindex;
elf_elfsections (abfd)[shindex] = &elf_tdata (abfd)->symtab_shndx_hdr;
return TRUE;
*************** bfd_section_from_shdr (bfd *abfd, unsign
*** 1944,1949 ****
--- 1945,1951 ----
if (ehdr->e_shstrndx == shindex)
{
elf_tdata (abfd)->shstrtab_hdr = *hdr;
+ elf_tdata (abfd)->shstrtab_section = shindex;
elf_elfsections (abfd)[shindex] = &elf_tdata (abfd)->shstrtab_hdr;
return TRUE;
}
*************** bfd_section_from_shdr (bfd *abfd, unsign
*** 1951,1956 ****
--- 1953,1959 ----
{
symtab_strtab:
elf_tdata (abfd)->strtab_hdr = *hdr;
+ elf_tdata (abfd)->strtab_section = shindex;
elf_elfsections (abfd)[shindex] = &elf_tdata (abfd)->strtab_hdr;
return TRUE;
}
Index: binutils/objcopy.c
===================================================================
RCS file: /cvs/src/src/binutils/objcopy.c,v
retrieving revision 1.95
diff -c -p -r1.95 objcopy.c
*** binutils/objcopy.c 28 Feb 2006 16:09:01 -0000 1.95
--- binutils/objcopy.c 24 May 2006 14:21:21 -0000
*************** filter_symbols (bfd *abfd, bfd *obfd, as
*** 915,920 ****
--- 915,945 ----
|| undefined
|| bfd_is_com_section (bfd_get_section (sym)))
keep = strip_symbols != STRIP_UNNEEDED;
+ else if ((flags & BSF_SECTION_SYM) != 0) /* Section symbol. */
+ {
+ struct bfd_section *section = sym->section;
+
+ keep = 1;
+
+ if (!bfd_is_abs_section (section)
+ && is_strip_section (abfd, section))
+ keep = 0;
+ else if (bfd_get_flavour (abfd) == bfd_target_elf_flavour)
+ {
+ Elf_Internal_Shdr **elfsections = elf_elfsections (abfd);
+ elf_symbol_type *elfsym = elf_symbol_from (abfd, sym);
+ unsigned int sec = elfsym->internal_elf_sym.st_shndx;
+ Elf_Internal_Shdr *elfhdr = elfsections[sec];
+
+ if (elfhdr->sh_type == SHT_REL || elfhdr->sh_type == SHT_RELA)
+ {
+ struct bfd_section *targsec =
+ bfd_section_from_elf_index (abfd, elfhdr->sh_info);
+ if (targsec && is_strip_section (abfd, targsec))
+ keep = 0;
+ }
+ }
+ }
else if ((flags & BSF_DEBUGGING) != 0) /* Debugging symbol. */
keep = (strip_symbols != STRIP_DEBUG
&& strip_symbols != STRIP_UNNEEDED
*************** filter_symbols (bfd *abfd, bfd *obfd, as
*** 969,975 ****
sym->flags |= BSF_GLOBAL;
}
! to[dst_count++] = sym;
}
}
--- 994,1002 ----
sym->flags |= BSF_GLOBAL;
}
! to[dst_count] = sym;
! bfd_copy_private_symbol_data (abfd, sym, obfd, to[dst_count]);
! dst_count++;
}
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix section symbols in objcopy when removing sections with relocations from Elf objects
2006-05-24 17:23 ` Julian Brown
@ 2006-05-25 0:58 ` H. J. Lu
0 siblings, 0 replies; 6+ messages in thread
From: H. J. Lu @ 2006-05-25 0:58 UTC (permalink / raw)
To: Julian Brown; +Cc: binutils, Richard Earnshaw, Paul Brook
On Wed, May 24, 2006 at 03:29:59PM +0100, Julian Brown wrote:
> Julian Brown wrote:
> >Hi,
> >
> >This patch fixes a problem where, if sections are removed during objcopy
> >with Elf object files, the section symbols corresponding to the
> >relocation sections for the removed sections may be left in the target
> >object file.
>
> This is a slightly-smarter version (thanks to offlist discussion with
> Dan) which knows about recoving Elf symbols from BFD symbols, hence
> avoids some unnecessary work.
>
Can you add a testcase so that it won't be broken by accident?
H.J.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix section symbols in objcopy when removing sections with relocations from Elf objects
2006-05-24 16:20 [PATCH] Fix section symbols in objcopy when removing sections with relocations from Elf objects Julian Brown
2006-05-24 17:23 ` Julian Brown
@ 2006-05-25 13:28 ` Alan Modra
2006-05-25 21:44 ` Alan Modra
1 sibling, 1 reply; 6+ messages in thread
From: Alan Modra @ 2006-05-25 13:28 UTC (permalink / raw)
To: Julian Brown; +Cc: binutils, Richard Earnshaw, Paul Brook
On Wed, May 24, 2006 at 02:35:14PM +0100, Julian Brown wrote:
> * objcopy.c (filter_symbols): Filter out section symbols for
> sections which are being stripped and their associated relocation
> sections. Call bfd_copy_private_symbol_data for each symbol which
> passes the filter.
I think this should be fixed in elf.c:elf_map_symbols by throwing away
any bad section syms. That way we keep (more) elf specific code out of
objcopy.c. I'll take a look at it.
--
Alan Modra
IBM OzLabs - Linux Technology Centre
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix section symbols in objcopy when removing sections with relocations from Elf objects
2006-05-25 13:28 ` Alan Modra
@ 2006-05-25 21:44 ` Alan Modra
2006-05-25 23:10 ` Julian Brown
0 siblings, 1 reply; 6+ messages in thread
From: Alan Modra @ 2006-05-25 21:44 UTC (permalink / raw)
To: Julian Brown, binutils, Richard Earnshaw, Paul Brook
On Thu, May 25, 2006 at 02:31:07PM +0930, Alan Modra wrote:
> On Wed, May 24, 2006 at 02:35:14PM +0100, Julian Brown wrote:
> > * objcopy.c (filter_symbols): Filter out section symbols for
> > sections which are being stripped and their associated relocation
> > sections. Call bfd_copy_private_symbol_data for each symbol which
> > passes the filter.
>
> I think this should be fixed in elf.c:elf_map_symbols by throwing away
> any bad section syms. That way we keep (more) elf specific code out of
> objcopy.c. I'll take a look at it.
Like this. Tested with "make all check" on practically all elf
targets. I wouldn't rush to put a change like this one on the 2.17
branch, not that I think there's anything wrong with it, but we seem to
have code in elf.c that goes to great lengths to support objcopy of
symbols in unlikely sections like .strtab. This was no doubt because
the assembler at one stage did output section syms for all these special
sections, and you could argue that objcopy ought to make minimal changes
for "objcopy foo.o new.o". Nowadays the assembler doesn't output these
rather silly symbols, so new object files should pass thru objcopy
relatively unchanged. However, "objcopy old_object.o new.o" will strip
out some section symbols with this patch. I can't see this being a
problem, but it's probably best not to make this change on a release
branch.
* elf.c (sym_is_global): Return a bfd_boolean.
(ignore_section_sym): New function.
(elf_map_symbols): Use ignore_section_sym to discard some syms.
(_bfd_elf_symbol_from_bfd_symbol): Ensure section belongs to
bfd before using elf_section_syms.
Index: bfd/elf.c
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.338
diff -u -p -r1.338 elf.c
--- bfd/elf.c 19 May 2006 00:51:28 -0000 1.338
+++ bfd/elf.c 25 May 2006 12:51:45 -0000
@@ -3280,7 +3280,7 @@ assign_section_numbers (bfd *abfd, struc
/* Map symbol from it's internal number to the external number, moving
all local symbols to be at the head of the list. */
-static int
+static bfd_boolean
sym_is_global (bfd *abfd, asymbol *sym)
{
/* If the backend has a special mapping, use it. */
@@ -3293,6 +3293,20 @@ sym_is_global (bfd *abfd, asymbol *sym)
|| bfd_is_com_section (bfd_get_section (sym)));
}
+/* Don't output section symbols for sections that are not going to be
+ output. Also, don't output section symbols for reloc and other
+ special sections. */
+
+static bfd_boolean
+ignore_section_sym (bfd *abfd, asymbol *sym)
+{
+ return ((sym->flags & BSF_SECTION_SYM) != 0
+ && (sym->value != 0
+ || (sym->section->owner != abfd
+ && (sym->section->output_section->owner != abfd
+ || sym->section->output_offset != 0))));
+}
+
static bfd_boolean
elf_map_symbols (bfd *abfd)
{
@@ -3333,51 +3347,29 @@ elf_map_symbols (bfd *abfd)
asymbol *sym = syms[idx];
if ((sym->flags & BSF_SECTION_SYM) != 0
- && sym->value == 0)
+ && !ignore_section_sym (abfd, sym))
{
- asection *sec;
-
- sec = sym->section;
+ asection *sec = sym->section;
- if (sec->owner != NULL)
- {
- if (sec->owner != abfd)
- {
- if (sec->output_offset != 0)
- continue;
-
- sec = sec->output_section;
+ if (sec->owner != abfd)
+ sec = sec->output_section;
- /* Empty sections in the input files may have had a
- section symbol created for them. (See the comment
- near the end of _bfd_generic_link_output_symbols in
- linker.c). If the linker script discards such
- sections then we will reach this point. Since we know
- that we cannot avoid this case, we detect it and skip
- the abort and the assignment to the sect_syms array.
- To reproduce this particular case try running the
- linker testsuite test ld-scripts/weak.exp for an ELF
- port that uses the generic linker. */
- if (sec->owner == NULL)
- continue;
-
- BFD_ASSERT (sec->owner == abfd);
- }
- sect_syms[sec->index] = syms[idx];
- }
+ sect_syms[sec->index] = syms[idx];
}
}
/* Classify all of the symbols. */
for (idx = 0; idx < symcount; idx++)
{
+ if (ignore_section_sym (abfd, syms[idx]))
+ continue;
if (!sym_is_global (abfd, syms[idx]))
num_locals++;
else
num_globals++;
}
- /* We will be adding a section symbol for each BFD section. Most normal
+ /* We will be adding a section symbol for each normal BFD section. Most
sections will already have a section symbol in outsymbols, but
eg. SHT_GROUP sections will not, and we need the section symbol mapped
at least in that case. */
@@ -3403,6 +3395,8 @@ elf_map_symbols (bfd *abfd)
asymbol *sym = syms[idx];
unsigned int i;
+ if (ignore_section_sym (abfd, sym))
+ continue;
if (!sym_is_global (abfd, sym))
i = num_locals2++;
else
@@ -5130,13 +5124,14 @@ _bfd_elf_symbol_from_bfd_symbol (bfd *ab
&& (flags & BSF_SECTION_SYM)
&& asym_ptr->section)
{
+ asection *sec;
int indx;
- if (asym_ptr->section->output_section != NULL)
- indx = asym_ptr->section->output_section->index;
- else
- indx = asym_ptr->section->index;
- if (indx < elf_num_section_syms (abfd)
+ sec = asym_ptr->section;
+ if (sec->owner != abfd && sec->output_section != NULL)
+ sec = sec->output_section;
+ if (sec->owner == abfd
+ && (indx = sec->index) < elf_num_section_syms (abfd)
&& elf_section_syms (abfd)[indx] != NULL)
asym_ptr->udata.i = elf_section_syms (abfd)[indx]->udata.i;
}
--
Alan Modra
IBM OzLabs - Linux Technology Centre
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix section symbols in objcopy when removing sections with relocations from Elf objects
2006-05-25 21:44 ` Alan Modra
@ 2006-05-25 23:10 ` Julian Brown
0 siblings, 0 replies; 6+ messages in thread
From: Julian Brown @ 2006-05-25 23:10 UTC (permalink / raw)
To: Alan Modra; +Cc: binutils, Richard Earnshaw, Paul Brook
Hi,
Alan Modra wrote:
> * elf.c (sym_is_global): Return a bfd_boolean.
> (ignore_section_sym): New function.
> (elf_map_symbols): Use ignore_section_sym to discard some syms.
> (_bfd_elf_symbol_from_bfd_symbol): Ensure section belongs to
> bfd before using elf_section_syms.
Thanks very much for looking into this! It's far preferable that the fix
is in Elf-specific code rather than the generic code, I agree. FYI, this
patch fixes the original problem just fine, and I've committed it on our
binutils-csl-2_17-branch.
Cheers,
Julian
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-05-25 16:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-05-24 16:20 [PATCH] Fix section symbols in objcopy when removing sections with relocations from Elf objects Julian Brown
2006-05-24 17:23 ` Julian Brown
2006-05-25 0:58 ` H. J. Lu
2006-05-25 13:28 ` Alan Modra
2006-05-25 21:44 ` Alan Modra
2006-05-25 23:10 ` Julian Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).