This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: PR23611, objcopy is not removing executable relocatable sections
* Alan Modra <amodra@gmail.com> [2018-09-11 09:51:38 +0930]:
> On Mon, Sep 10, 2018 at 04:21:32PM +0100, Andrew Burgess wrote:
> > Hi Alan,
> >
> > Thanks for taking the time to respond, I really appreciate your time.
> >
> > I didn't quite follow all of the detail in your explanation, I'm
> > hoping you might be able to guide me through a little more as I really
> > do want to expand my understanding in this area.
> >
> > * Alan Modra <amodra@gmail.com> [2018-09-10 22:54:24 +0930]:
> >
> > > On Mon, Sep 10, 2018 at 10:54:50AM +0100, Andrew Burgess wrote:
> > > > * Alan Modra <amodra@gmail.com> [2018-09-10 14:02:27 +0930]:
> > > > > When --remove-relocations was added to objcopy with commit
> > > > > d3e5f6c8f1e, objcopy lost the ability to remove dynamic relocation
> > > > > sections such as .rela.plt from executables using the option
> > > > > "--remove-section=.rela.plt". This patch reinstates that
> > > > > functionality.
> > > > >
> > > > > I thought it best to keep --remove-relocations as is, rather than
> > > > > extending to handle dynamic relocations as per the patch in the PR,
> > > > > because executables linked with --emit-relocs may have both dynamic
> > > > > and non-dynamic relocations. In that case --remove-relocataions=* is
> > > > > useful to remove all the non-dynamic relocations.
> > > >
> > > > I think having an option that removes _almost_ all relocation sections
> > > > is going to cause user confusion. Yes, we can document it, but if we
> > > > can make it work for all relocation sections I think we probably
> > > > should.
> > >
> > > Removing dynamic relocation sections from an executable is such a
> > > weird thing to do that I'm not overly concerned. Also, they aren't
> > > really removed. What actually happens is that the section disappears
> > > from the section headers and the contents are overwritten with
> > > zeros.
> >
> > OK. That seems weird, but I feel like this is a deeper level of
> > knowledge, and I'm missing some of the simpler stuff, so, lets come
> > back to this later...
> >
> > > Contrast that with what happens to relocation sections in a
> > > relocatable object, where the section is removed and other sections
> > > take up its file space.
> >
> > So, here you make a distinction between a 'relocatable object' and (I
> > assume) a 'non-relocatable object'.
>
> By "relocatable object" I mean an object file that hasn't been through
> a final linking stage, typically a .o file, eg. the output of
> "gcc -c hello.c".
>
> > So, I tried an experiment (this is all on x86-64 GNU/Linux):
> >
> > $ cat test.c
> > #include <stdio.h>
> >
> > volatile const char *message = "Hello World\n";
> >
> > int
> > main ()
> > {
> > printf ("%s", message);
> > return 0;
> > }
> > $ gcc -o test.o -c test.c -g3 -O0
>
> So test.o here is a relocatable object file.
>
> > $ gcc --static -Wl,-emit-relocs -o test.x test.o -g3 -O0
>
> test.x on the other hand is a final linked static executable, but with
> relocations emitted corresponding to those in each input object
> including those extracted from libraries. --emit-relocs may transform
> the input file relocations if and when the linker edits code
> sequences, and may emit relocations to describe linker added stub code
> too.
>
> > $ readelf -WS test.x | grep rela.
> > [ 3] .rela.plt RELA 00000000004001d8 0001d8 0001f8 18 AI 0 38 8
> > [ 5] .rela.init RELA 0000000000000000 0b5d68 000018 18 I 57 4 8
> > [ 8] .rela.text RELA 0000000000000000 0b5d80 02c298 18 I 57 7 8
> > [10] .rela__libc_freeres_fn RELA 0000000000000000 0e2018 000f90 18 I 57 9 8
> > [12] .rela__libc_thread_freeres_fn RELA 0000000000000000 0e2fa8 000210 18 I 57 11 8
> > [15] .rela.rodata RELA 0000000000000000 0e31b8 011238 18 I 57 14 8
> > [17] .rela__libc_subfreeres RELA 0000000000000000 0f43f0 0000d8 18 I 57 16 8
> > [19] .rela__libc_IO_vtables RELA 0000000000000000 0f44c8 000fa8 18 I 57 18 8
> > [21] .rela__libc_atexit RELA 0000000000000000 0f5470 000018 18 I 57 20 8
> > [24] .rela__libc_thread_subfreeres RELA 0000000000000000 0f5488 000030 18 I 57 23 8
> > [26] .rela.eh_frame RELA 0000000000000000 0f54b8 005100 18 I 57 25 8
> > [29] .rela.tdata RELA 0000000000000000 0fa5b8 000060 18 I 57 28 8
> > [32] .rela.init_array RELA 0000000000000000 0fa618 000030 18 I 57 31 8
> > [34] .rela.fini_array RELA 0000000000000000 0fa648 000030 18 I 57 33 8
> > [36] .rela.data.rel.ro RELA 0000000000000000 0fa678 000270 18 I 57 35 8
> > [40] .rela.data RELA 0000000000000000 0fa8e8 000b40 18 I 57 39 8
> > [46] .rela.note.stapsdt RELA 0000000000000000 0fb428 0008d0 18 I 57 45 8
> > [48] .rela.debug_aranges RELA 0000000000000000 0fbcf8 000030 18 I 57 47 8
> > [50] .rela.debug_info RELA 0000000000000000 0fbd28 000630 18 I 57 49 8
> > [53] .rela.debug_line RELA 0000000000000000 0fc358 000018 18 I 57 52 8
> > [56] .rela.debug_macro RELA 0000000000000000 0fc370 0043e0 18 I 57 55 8
> > $ readelf -WS test2.x | grep rela.
> > [ 3] .rela.plt RELA 00000000004001d8 0001d8 0001f8 18 AI 0 37 8
> > [ 5] .rela.init RELA 0000000000000000 0b5d68 000018 18 I 56 4 8
> > [ 9] .rela__libc_freeres_fn RELA 0000000000000000 0b5d80 000f90 18 I 56 8 8
> > [11] .rela__libc_thread_freeres_fn RELA 0000000000000000 0b6d10 000210 18 I 56 10 8
> > [14] .rela.rodata RELA 0000000000000000 0b6f20 011238 18 I 56 13 8
> > [16] .rela__libc_subfreeres RELA 0000000000000000 0c8158 0000d8 18 I 56 15 8
> > [18] .rela__libc_IO_vtables RELA 0000000000000000 0c8230 000fa8 18 I 56 17 8
> > [20] .rela__libc_atexit RELA 0000000000000000 0c91d8 000018 18 I 56 19 8
> > [23] .rela__libc_thread_subfreeres RELA 0000000000000000 0c91f0 000030 18 I 56 22 8
> > [25] .rela.eh_frame RELA 0000000000000000 0c9220 005100 18 I 56 24 8
> > [28] .rela.tdata RELA 0000000000000000 0ce320 000060 18 I 56 27 8
> > [31] .rela.init_array RELA 0000000000000000 0ce380 000030 18 I 56 30 8
> > [33] .rela.fini_array RELA 0000000000000000 0ce3b0 000030 18 I 56 32 8
> > [35] .rela.data.rel.ro RELA 0000000000000000 0ce3e0 000270 18 I 56 34 8
> > [39] .rela.data RELA 0000000000000000 0ce650 000b40 18 I 56 38 8
> > [45] .rela.note.stapsdt RELA 0000000000000000 0cf190 0008d0 18 I 56 44 8
> > [47] .rela.debug_aranges RELA 0000000000000000 0cfa60 000030 18 I 56 46 8
> > [49] .rela.debug_info RELA 0000000000000000 0cfa90 000630 18 I 56 48 8
> > [52] .rela.debug_line RELA 0000000000000000 0d00c0 000018 18 I 56 51 8
> > [55] .rela.debug_macro RELA 0000000000000000 0d00d8 0043e0 18 I 56 54 8
> >
> > This is a non-dynamic object (as far as I understand it) and notice
> > that relocation section .rela__libc_freeres_fn moved to offset 0xb5d80
> > to replace .rela.text once it was removed.
> >
> > Using the same method I also tested building in these configurations:
> >
> > $ gcc -fPIC -shared -Wl,-emit-relocs -o test.x test.c -g3 -O0
> > $ gcc -g3 -O0 -o test.x -Wl,-z,nocombreloc,--emit-relocs test.c
> > $ gcc -Wl,-emit-relocs -o test.x test.c -g3 -O0
> >
> > In all cases, removing the .rela.text section caused another section
> > to move up to fill its place.
> >
> > So, in my ignorance it appears (to me) that the distinction is between
> > dynamic relocs and non-dynamic relocs, not between relocatable and
> > non-relocatable objects.
>
> True, but typically people don't use --emit-relocs unless they are
> running some post-link optimization or analysis. So the usual place
> to find non-dynamic relocations is in relocatable object files.
>
> > >
> > > You also might like to inspect the output of
> > > gcc -o hello -Wl,-z,nocombreloc,--emit-relocs hello.c
> > > before you discount the utility of the current --remove-relocations
> > > behaviour.
> >
> > As the person who added --remove-relocations I certainly have an
> > interest in its behaviour :)
>
> Yes, I understand. :)
>
> > As far as I can tell, --remove-relocations can remove anything except
> > dynamic relocations both before and after your patch.
> >
> > It certainly was never my intention to introduce such an exception,
> > and if I had understood this limitation at the time of the original
> > patch I would have tried to remove the limitation.
> >
> > However, as you can see above I included the 'nocombreloc' option in
> > one of my test cases, but without more of a clue I'm not sure what it
> > is that I'm supposed to be observing. As far as I can tell an object
> > built with that option responds to remove-relocations just like any
> > other.
>
> With -Wl,-z,nocombreloc,--emit-relocs" and *not* "-static" you'll see
> multiple relocation sections with the same name. For example there
> will likely be two .rela.data sections, one for dynamic relocations
> (that utilise the .dynsym symbol table and are loaded into the process
> image), and one for non-dynamic relocations output by --emit-relocs
> (that use .symtab and are non-alloc).
>
> > > I call it a (perhaps accidental) feature rather than a bug
> > > that the --emit-relocs created sections can be removed with
> > > --remove-relocations.
> >
> > I kind of feel like that's a pretty meta can-of-worms to open.
>
> That's true. I was going to apply a cleaned up version (attached) of
> Kamlesh's patch to make --remove-relocations remove all types of reloc
> section, but on reconsidering decided the existing behaviour was worth
> keeping despite needing to document it.
This is what puzzles me about this discussion. You saw value in
--remove-relocations NOT being able to remove some relocation
sections?
What value can that possibly have? All I see is a way to confuse
users. Sure, you documented it, but, it still seems like an
unexpected caveat, and one that's pretty easy to remove.
>
> Incidentally, another wart is that not all relocation sections start
> with ".rela." or ".rel.". A section FOO has .relaFOO or .relFOO for
> relocations. You might like to fix that, patch preapproved.
Thanks! Patch attached at the end of this email. I push it in a few
days if I don't get any follow up.
>
> > If
> > emit-relocs didn't emit reloc sections that could be removed with
> > remove-relocations then I would have implemented remove-relocations
> > differently in order to allow that to be the case...
>
> Yes, but it was implemented in a way that didn't allow removal of
> dynamic relocataions, and I think that is a good thing. Other people
> may have discovered it allows you to reverse the action of
> ld --emit-relocs and made use of the feature. If we remove that
> capability we'll probably get bug reports that --remove-relocations is
> broken!
Why on earth would we remove that feature? The --remove-relocations
option was specifically put in to remove relocations left in by
--emit-relocs.
Now, I messed up, I admit; I didn't realise that dynamic relocations
were handled differently to non-dynamic relocations. If I had I would
have written --remove-relocations differently to remove all types of
relocation.
>
> > The original motivation behind --remove-relocations was that I had a
> > client who wanted to remove a relocation section. Before this option
> > the only relocation sections that could be removed were dynamic
> > relocation sections, something I didn't realise, and so I concluded
> > that NO relocation sections could be removed.
> >
> > Then --remove-relocations was born and all was good. Except that I
> > managed to break the ability to remove dynamic relocation sections.
> >
> > >
> > > > > /* Wrapper for dealing with --remove-section (-R) command line arguments.
> > > > > A special case is detected here, if the user asks to remove a relocation
> > > > > section (one starting with ".rela." or ".rel.") then this removal must
> > > > > - be done using a different technique. */
> > > > > + be done using a different technique in a relocatable object. */
> > > >
> > > > I'm not sure I agree with this comment, how about:
> > >
> > > I agree that it's not 100% accurate, but it's near enough without
> > > going into too much detail.
> >
> > So, I started to write here:
> >
> > ".... I don't see what 'relocatable objects' have to do with this
> > at all, when it is 'dynamic relocations' that are the problem."
> >
> > and then it struck me. I think what you're getting at is that,
> > without emit-relocs, the only relocations you'd expect to see in a
> > relocatable object are dynamic relocations, and so, in your comment,
> > relocatable object implies dynamic relocations.
>
> No, generally "relocatable object" means the output of "gcc -c" or
> "ld -r".
OK, but, given two comments, this what we have now:
> section (one starting with ".rela." or ".rel.") then this removal must
> be done using a different technique in a relocatable object. */
And this alternative:
> section (one starting with ".rela." or ".rel.") then this removal must
> be done using a different technique for non-dynamic relocations. */
The first is true in all cases, except when it's not true. And the
second comment is always true (right?)
Shouldn't comments make the code more understandable, and draw
attention to gotchas?
I understand you rejected my original proposed revision as being two
verbose, but the above is only 4 characters longer, but is completely
correct. Would you be willing to accept such a revision?
Thanks
Andrew
----
Below is a patch to fix the .rela. / .rel. prefix assumption you
pointed out.
---
objcopy: Don't assume section names start with DOT when removing relocs
When calling --remove-section=.rela.text to remove relocations from
the .text section we had an assuption that section names begin with a
'.'. If a section didn't, then the relocation section would be named,
for example .relaFOO (for a section named 'FOO'), in which case we
would fail to remove the relocations.
Fixed in this commit.
binutils/ChangeLog:
* objcopoy.c (handle_remove_section_option): Don't assume that
section names begin with '.'.
* testsuite/binutils-all/remove-relocs-07.d: New file.
* testsuite/binutils-all/remove-relocs-07.s: New file.
* testsuite/binutils-all/remove-relocs-08.d: New file.
---
binutils/ChangeLog | 8 ++++++++
binutils/objcopy.c | 6 ++++--
binutils/testsuite/binutils-all/remove-relocs-07.d | 10 ++++++++++
binutils/testsuite/binutils-all/remove-relocs-07.s | 9 +++++++++
binutils/testsuite/binutils-all/remove-relocs-08.d | 10 ++++++++++
5 files changed, 41 insertions(+), 2 deletions(-)
create mode 100644 binutils/testsuite/binutils-all/remove-relocs-07.d
create mode 100644 binutils/testsuite/binutils-all/remove-relocs-07.s
create mode 100644 binutils/testsuite/binutils-all/remove-relocs-08.d
diff --git a/binutils/objcopy.c b/binutils/objcopy.c
index f712ffe5917..8a50bebb5b1 100644
--- a/binutils/objcopy.c
+++ b/binutils/objcopy.c
@@ -3948,9 +3948,11 @@ discard_relocations (bfd *ibfd ATTRIBUTE_UNUSED, asection *isection)
static void
handle_remove_section_option (const char *section_pattern)
{
- if (strncmp (section_pattern, ".rela.", 6) == 0)
+ if (strncmp (section_pattern, ".rela", 5) == 0
+ && strlen (section_pattern) > 5)
handle_remove_relocations_option (section_pattern + 5);
- else if (strncmp (section_pattern, ".rel.", 5) == 0)
+ else if (strncmp (section_pattern, ".rel", 4) == 0
+ && strlen (section_pattern) > 4)
handle_remove_relocations_option (section_pattern + 4);
find_section_list (section_pattern, TRUE, SECTION_CONTEXT_REMOVE);
diff --git a/binutils/testsuite/binutils-all/remove-relocs-07.d b/binutils/testsuite/binutils-all/remove-relocs-07.d
new file mode 100644
index 00000000000..5b74fe380bf
--- /dev/null
+++ b/binutils/testsuite/binutils-all/remove-relocs-07.d
@@ -0,0 +1,10 @@
+#PROG: objcopy
+#source: remove-relocs-07.s
+#objcopy: --remove-relocations=data.relocs.01
+#readelf: -r
+
+Relocation section '\.rela?data\.relocs\.02' at offset 0x[0-9a-f]+ contains 3 entries:
+.*
+.*
+.*
+.*
diff --git a/binutils/testsuite/binutils-all/remove-relocs-07.s b/binutils/testsuite/binutils-all/remove-relocs-07.s
new file mode 100644
index 00000000000..e791e087504
--- /dev/null
+++ b/binutils/testsuite/binutils-all/remove-relocs-07.s
@@ -0,0 +1,9 @@
+ .section "data.relocs.01", "aw"
+ .dc.a rel_01_01
+ .dc.a rel_01_02
+ .dc.a rel_01_03
+
+ .section "data.relocs.02", "aw"
+ .dc.a rel_02_01
+ .dc.a rel_02_02
+ .dc.a rel_02_03
diff --git a/binutils/testsuite/binutils-all/remove-relocs-08.d b/binutils/testsuite/binutils-all/remove-relocs-08.d
new file mode 100644
index 00000000000..ab0ea5d266d
--- /dev/null
+++ b/binutils/testsuite/binutils-all/remove-relocs-08.d
@@ -0,0 +1,10 @@
+#PROG: objcopy
+#source: remove-relocs-07.s
+#objcopy: --remove-section=.reladata.relocs.01 --remove-section=.reldata.relocs.01
+#readelf: -r
+
+Relocation section '\.rela?data\.relocs\.02' at offset 0x[0-9a-f]+ contains 3 entries:
+.*
+.*
+.*
+.*
--
2.14.4