This is the mail archive of the binutils@sourceware.org mailing list for the binutils 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]

Re: PR23611, objcopy is not removing executable relocatable sections


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.

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.

>  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!

> 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".

> However, wouldn't this mean that your comment should be reversed?
> 
> The original comment said that this function deals with requests to
> remove a section, except that relocation sections must be handled
> differently.
> 
> You've changed this to say that, .... relocation sections must be
> handled differently in a relocatable object.
> 
> And if we apply the assumption that 'in a relocatable object' means
> 'for dynamic relocations', then this says that dynamic relocation
> sections must be handled differently.  Except this isn't true, dynamic
> relocation sections must be handled just like any other section, it's
> non-dynamic relocation sections that must be handled
> differently.  Right?
> 
> One further clarification, the client that I originally did the work
> for uses --emit-relocs extensively, so when I wrote this code, and
> when I've been thinking about it, I'm generally imagining an
> object/executable with all the relocations preserved.  This I guess,
> is why I'm so keen to distinguish between different relocation types
> requiring different handling, instead of different object types.
> 
> Thanks for your time,
> 
> Andrew

-- 
Alan Modra
Australia Development Lab, IBM
commit 0e9a66b344d90c44b45e0205e29cba804f2979de
Author: kamlesh kumar <kamleshbhalui@gmail.com>
Date:   Fri Sep 7 17:45:43 2018 +0530

    PR23611, objcopy is not removing executable relocatable sections
    
    BFD handles ELF relocation sections in an executable differently to
    relocation sections in a relocatable object.  For a relocatable
    object, BFD carries the relocations as data associated with the
    section to which they apply; The relocation section doesn't appear as
    a separate section.  For an executable, dynamic relocation sections do
    appear as separate sections.  This means that objcopy needs to use
    different strategies when dealing with relocations.
    
            PR binutils/23611
            * objcopy.c (is_strip_section_1): Consider .rela and .rel sections
            for stripping.
            * testsuite/binutils-all/objcopy.exp (objcopy_remove_rela_plt),
            (objcopy_remove_rela_dyn),
            (objcopy_remove_relocations_from_executable): New tests.

diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index c58d70e08d..603639615d 100644
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,3 +1,13 @@
+2018-09-10  Kamlesh Kumar  <kamleshbhalui@gmail.com>
+	    Alan Modra  <amodra@gmail.com>
+
+	PR binutils/23611
+	* objcopy.c (is_strip_section_1): Consider .rela and .rel sections
+	for stripping.
+	* testsuite/binutils-all/objcopy.exp (objcopy_remove_rela_plt),
+	(objcopy_remove_rela_dyn),
+	(objcopy_remove_relocations_from_executable): New tests.
+
 2018-09-03  Nick Clifton  <nickc@redhat.com>
 
 	* po/ja.po: Updated Japanese translation.
diff --git a/binutils/objcopy.c b/binutils/objcopy.c
index 6bd933993b..b0d2030ee3 100644
--- a/binutils/objcopy.c
+++ b/binutils/objcopy.c
@@ -1284,11 +1284,18 @@ is_strip_section_1 (bfd *abfd ATTRIBUTE_UNUSED, asection *sec)
     {
       struct section_list *p;
       struct section_list *q;
+      const char *secname = bfd_get_section_name (abfd, sec);
+
+      if (strncmp (secname, ".rela.", 6) == 0)
+	p = find_section_list (secname + 5, FALSE,
+			       SECTION_CONTEXT_REMOVE_RELOCS);
+      else if (strncmp (secname, ".rel.", 5) == 0)
+	p = find_section_list (secname + 4, FALSE,
+			       SECTION_CONTEXT_REMOVE_RELOCS);
+      else
+	p = find_section_list (secname, FALSE, SECTION_CONTEXT_REMOVE);
 
-      p = find_section_list (bfd_get_section_name (abfd, sec), FALSE,
-			     SECTION_CONTEXT_REMOVE);
-      q = find_section_list (bfd_get_section_name (abfd, sec), FALSE,
-			     SECTION_CONTEXT_COPY);
+      q = find_section_list (secname, FALSE, SECTION_CONTEXT_COPY);
 
       if (p && q)
 	fatal (_("error: section %s matches both remove and copy options"),
@@ -3926,6 +3933,7 @@ static void
 handle_remove_relocations_option (const char *section_pattern)
 {
   find_section_list (section_pattern, TRUE, SECTION_CONTEXT_REMOVE_RELOCS);
+  sections_removed = TRUE;
 }
 
 /* Return TRUE if ISECTION from IBFD should have its relocations removed,
diff --git a/binutils/testsuite/binutils-all/objcopy.exp b/binutils/testsuite/binutils-all/objcopy.exp
index d979648758..328ef0ceac 100644
--- a/binutils/testsuite/binutils-all/objcopy.exp
+++ b/binutils/testsuite/binutils-all/objcopy.exp
@@ -1218,6 +1218,80 @@ proc objcopy_test_without_global_symbol { } {
     pass $test
 }
 
+# objcopy remove relocation from executable test
+
+proc objcopy_remove_relocations_from_executable { } {
+    global OBJCOPY
+    global srcdir
+    global subdir
+    global READELF
+
+    set test "remove-section relocation sections"
+
+    if { [target_compile $srcdir/$subdir/testprog.c tmpdir/pr23611 executable debug] != "" } {
+	untested $test
+	return
+    }
+
+    if [is_remote host] {
+	set objfile [remote_download host tmpdir/pr23611]
+    } else {
+	set objfile tmpdir/pr23611
+    }
+    set out tmpdir/pr23611.out1
+
+    set exec_output1 [binutils_run $OBJCOPY "-R .rela.plt -R .rela.dyn -R .rel.plt -R .rel.dyn $objfile $out"]
+    set exec_output2 [binutils_run $READELF "-S $out"]
+    if { [string match "*.rel.plt*" $exec_output2] || [string match "*.rela.plt*" $exec_output2] || [string match "*.rel.dyn*" $exec_output2] || [string match "*.rela.dyn*" $exec_output2] } {
+	fail $test
+	return
+    }
+    pass $test
+}
+
+proc objcopy_remove_rela_plt { } {
+    global OBJCOPY
+    global READELF
+    set test "remove-relocations=.plt"
+    set out tmpdir/pr23611.out2
+    set objfile tmpdir/pr23611
+    if { ![file exists $objfile] } {
+	untested $test
+	return
+    }
+    set exec_output1 [binutils_run $OBJCOPY "--remove-relocations=.plt $objfile $out"]
+    set exec_output2 [binutils_run $READELF "-S $out"]
+    if { [string match "*.rela.plt*" $exec_output2] || [string match "*.rel.plt*" $exec_output2] } {
+	fail $test
+	return
+    }
+    pass $test
+}
+
+proc objcopy_remove_rela_dyn { } {
+    global OBJCOPY
+    global READELF
+    set test "remove-relocations=.dyn"
+    set out tmpdir/pr23611.out3
+    set objfile tmpdir/pr23611
+    if { ![file exists $objfile] } {
+	untested $test
+	return
+    }
+    set exec_output1 [binutils_run $OBJCOPY "--remove-relocations=.dyn $objfile $out"]
+    set exec_output2 [binutils_run $READELF "-S $out"]
+    if { [string match "*.rela.dyn*" $exec_output2] || [string match "*.rel.dyn*" $exec_output2] } {
+	fail $test
+	return
+    }
+    pass $test
+}
+
+objcopy_remove_relocations_from_executable
+objcopy_remove_rela_dyn
+objcopy_remove_rela_plt
+
+
 # The AArch64 and ARM targets preserve mapping symbols
 # in object files, so they will fail this test.
 setup_xfail aarch64*-*-* arm*-*-*

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