Re: PR23611, objcopy is not removing executable relocatable sections

[Andrew Burgess <andrew dot burgess at embecosm dot com>]

* Alan Modra <amodra@gmail.com> [2018-09-10 14:02:27 +0930]:

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

One thing I tried but couldn't find a good answer for, is, is there a
flag that appears on the input section that identifies it as a
relocation section, even though it's actually being handled as a
"normal" section?  If such a thing existed then that would make
special casing `is_strip_section_1` slightly less invasive I think.

If we can't identifty relocation section and/or don't want special
cases in the `is_strip_section_1` code, could we not add more logic
into the `handle_remove_relocations_option` code?  Maybe we could
generate two new section names, one prefixed with `rel.` and one with
`rela.` and add both of these to the strip section list?  Or maybe add
a pattern prefix, `rel*.`?

> 
> 	PR binutils/23611
> 	* objcopy.c (handle_remove_section_option): Consider .rela and
> 	.rel sections for stripping directly as well as attached to the
> 	associated section they relocate.
> 	* doc/binutils.texi (remove-relocations): Specify that this
> 	option removes non-dynamic relocation sections.
> 	* testsuite/binutils-all/objcopy.exp
> 	(objcopy_remove_relocations_from_executable): New test.
> 
> diff --git a/binutils/doc/binutils.texi b/binutils/doc/binutils.texi
> index e40ccb073d..76cbed05af 100644
> --- a/binutils/doc/binutils.texi
> +++ b/binutils/doc/binutils.texi
> @@ -1321,17 +1321,20 @@ will remove all sections matching the pattern '.text.*', but will not
>  remove the section '.text.foo'.
>  
>  @item --remove-relocations=@var{sectionpattern}
> -Remove relocations from the output file for any section matching
> -@var{sectionpattern}.  This option may be given more than once.  Note
> -that using this option inappropriately may make the output file
> -unusable.  Wildcard characters are accepted in @var{sectionpattern}.
> +Remove non-dynamic relocations from the output file for any section
> +matching @var{sectionpattern}.  This option may be given more than
> +once.  Note that using this option inappropriately may make the output
> +file unusable, and attempting to remove a dynamic relocation section
> +such as @samp{.rela.plt} from an executable or shared library with
> +@option{--remove-relocations=.plt} will not work.  Wildcard characters
> +are accepted in @var{sectionpattern}.
>  For example:
>  
>  @smallexample
>    --remove-relocations=.text.*
>  @end smallexample
>  
> -will remove the relocations for all sections matching the patter
> +will remove the relocations for all sections matching the pattern
>  '.text.*'.
>  
>  If the first character of @var{sectionpattern} is the exclamation
> diff --git a/binutils/objcopy.c b/binutils/objcopy.c
> index 6bd933993b..f712ffe591 100644
> --- a/binutils/objcopy.c
> +++ b/binutils/objcopy.c
> @@ -3943,7 +3943,7 @@ discard_relocations (bfd *ibfd ATTRIBUTE_UNUSED, asection *isection)
>  /* 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:

     .... then this removal must be done using a different technique,
     except for dynamic relocations (for example .rela.plt and
     rela.dyn) which are handled like standard sections.  */

As far as I can tell through experimenting things like .rela.text are
always held as data off of the .text section no matter what the kind
of object/executable.  Or is there a specific type of build where all
relocation sections are treated like normal sections?

Thanks,
Andrew


>  
>  static void
>  handle_remove_section_option (const char *section_pattern)
> @@ -3952,11 +3952,9 @@ handle_remove_section_option (const char *section_pattern)
>      handle_remove_relocations_option (section_pattern + 5);
>    else if (strncmp (section_pattern, ".rel.", 5) == 0)
>      handle_remove_relocations_option (section_pattern + 4);
> -  else
> -    {
> -      find_section_list (section_pattern, TRUE, SECTION_CONTEXT_REMOVE);
> -      sections_removed = TRUE;
> -    }
> +
> +  find_section_list (section_pattern, TRUE, SECTION_CONTEXT_REMOVE);
> +  sections_removed = TRUE;
>  }
>  
>  /* Copy relocations in input section ISECTION of IBFD to an output
> diff --git a/binutils/testsuite/binutils-all/objcopy.exp b/binutils/testsuite/binutils-all/objcopy.exp
> index d979648758..61793d98f5 100644
> --- a/binutils/testsuite/binutils-all/objcopy.exp
> +++ b/binutils/testsuite/binutils-all/objcopy.exp
> @@ -1223,3 +1223,36 @@ proc objcopy_test_without_global_symbol { } {
>  setup_xfail aarch64*-*-* arm*-*-*
>  
>  objcopy_test_without_global_symbol
> +
> +# 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.out
> +
> +    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
> +}
> +
> +objcopy_remove_relocations_from_executable
> 
> -- 
> Alan Modra
> Australia Development Lab, IBM