Re: [PING #2] [PATCH 0/2] AVR: improve use of EF_AVR_LINKRELAX_PREPARED flag.

[Denis Chertykov <chertykov at gmail dot com>]

Hi Andrew.

I found in binutils mailing list archives that Nick approved your
patches. (I'm not subscribed to this mailing list)
So, feel free to apply them.


Denis.

2014-12-24 0:44 GMT+03:00 Andrew Burgess <andrew.burgess@embecosm.com>:
> * Denis Chertykov <chertykov@gmail.com> [2014-12-23 21:35:29 +0400]:
>
>> What are you trying to achieve with this patch (set of patches) ?
>
> I consider the first patch to be a bug fix, and the second to be a
> natural consequence of the first. Here's why:
>
> In bfd/elf32-avr.c:elf32_avr_relax_section we see that there's code to
> skip relaxation if the EF_AVR_LINKRELAX_PREPARED flag is not set.
> This means that the EF_AVR_LINKRELAX_PREPARED should only be set if an
> ELF is prepared for linker relaxation.
>
> If a section is not prepared for linker relaxation then relaxing it
> will cause the line number table to become out of step with the code,
> this can be seen in gas/dwarf2dbg.c with the use of
> DWARF2_USE_FIXED_ADVANCE_PC.
>
> Originally, the EF_AVR_LINKRELAX_PREPARED was always set on every ELF,
> see the pre-patched
> bfd/elf32-avr.c:bfd_elf_avr_final_write_processing.  Given that the
> flag is only used to prevent linker relaxation kicking in, then this
> unconditional setting of the flag makes it pretty useless.
>
> I have users who are incorrectly apply linker relaxation to an
> unprepared ELF and then wondering why the debug information is
> corrupted.  This led me to look for a mechanism that would either
> prevent, or warn about trying to perform linker relaxation on an
> unprepared ELF.  It was then that I looked and found the
> EF_AVR_LINKRELAX_PREPARED flag, and concluded that it could be
> modified to do what I wanted.  If I've made a mistake about what the
> flag is for then you'll have to help me understand its true purpose as
> I don't see what else it could be for.
>
> So I write the first patch.  But now I have the problem that I need to
> flag to propagate across partial links in such a way that the
> resulting object will not allow a user to shoot themselves in the
> foot (by performing linker relaxation on something that's
> unprepared).  This leads to the second patch.
>
> Hopefully that explains specifically why /this/ patch.
>
> As for why /any/ AVR linker relaxation patches ... that's easy, I was
> asked by a user, why when I perform linker relaxation on these objects
> do I end up with broken debug information?
>
>
> Thanks,
>
> Andrew
>
>
>
>>
>> 2014-12-22 6:33 GMT+03:00 Andrew Burgess <andrew.burgess@embecosm.com>:
>> > Ping.  I'd like to get this merged before I post some follow on
>> > patches I have relating to AVR linker relaxation.
>> >
>> > Thanks,
>> > Andrew
>> >
>> > * Andrew Burgess <andrew.burgess@embecosm.com> [2014-12-15 10:34:20 +0000]:
>> >
>> >> Ping.
>> >>
>> >> * Andrew Burgess <andrew.burgess@embecosm.com> [2014-12-05 22:20:41 +0000]:
>> >>
>> >> > The AVR target has a flag EF_AVR_LINKRELAX_PREPARED that can be set in
>> >> > the ELF header flags to indicate if a file is prepared for linker
>> >> > relaxation or not.
>> >> >
>> >> > The problem is that at th moment the flag is set unconditionally, in
>> >> > every created elf.
>> >> >
>> >> > In the first patch I propose making the flag conditional on whether
>> >> > the assembler was passed the -mlink-relax flag or not.
>> >> >
>> >> > In the second patch I update the linker to propagate the flag from the
>> >> > input files to the output if a partial link is performed.
>> >> >
>> >> > OK to apply?
>> >> >
>> >> > Thanks,
>> >> > Andrew
>> >> >
>> >> > --
>> >> >
>> >> > Andrew Burgess (2):
>> >> >   AVR: Only set link-relax elf flag when appropriate.
>> >> >   AVR/ld: Propagate link-relax elf header flag correctly.
>> >> >
>> >> >  bfd/ChangeLog                                     |  5 +++
>> >> >  bfd/elf32-avr.c                                   |  1 -
>> >> >  gas/ChangeLog                                     | 10 ++++++
>> >> >  gas/config/tc-avr.c                               |  9 +++++-
>> >> >  gas/config/tc-avr.h                               |  3 ++
>> >> >  gas/testsuite/ChangeLog                           |  6 ++++
>> >> >  gas/testsuite/gas/avr/link-relax-elf-flag-clear.d | 10 ++++++
>> >> >  gas/testsuite/gas/avr/link-relax-elf-flag-set.d   |  9 ++++++
>> >> >  gas/testsuite/gas/avr/link-relax-elf-flag.s       |  4 +++
>> >> >  ld/ChangeLog                                      |  6 ++++
>> >> >  ld/emultempl/avrelf.em                            | 37 +++++++++++++++++++++++
>> >> >  ld/testsuite/ChangeLog                            | 13 ++++++++
>> >> >  ld/testsuite/ld-avr/relax-elf-flags-01.d          | 12 ++++++++
>> >> >  ld/testsuite/ld-avr/relax-elf-flags-02.d          | 12 ++++++++
>> >> >  ld/testsuite/ld-avr/relax-elf-flags-03.d          | 12 ++++++++
>> >> >  ld/testsuite/ld-avr/relax-elf-flags-04.d          | 12 ++++++++
>> >> >  ld/testsuite/ld-avr/relax-elf-flags-05.d          | 12 ++++++++
>> >> >  ld/testsuite/ld-avr/relax-elf-flags-06.d          | 12 ++++++++
>> >> >  ld/testsuite/ld-avr/relax-elf-flags-07.d          | 12 ++++++++
>> >> >  ld/testsuite/ld-avr/relax-elf-flags-08.d          | 12 ++++++++
>> >> >  ld/testsuite/ld-avr/relax-elf-flags-a.s           |  4 +++
>> >> >  ld/testsuite/ld-avr/relax-elf-flags-b.s           |  4 +++
>> >> >  22 files changed, 215 insertions(+), 2 deletions(-)
>> >> >  create mode 100644 gas/testsuite/gas/avr/link-relax-elf-flag-clear.d
>> >> >  create mode 100644 gas/testsuite/gas/avr/link-relax-elf-flag-set.d
>> >> >  create mode 100644 gas/testsuite/gas/avr/link-relax-elf-flag.s
>> >> >  create mode 100644 ld/testsuite/ld-avr/relax-elf-flags-01.d
>> >> >  create mode 100644 ld/testsuite/ld-avr/relax-elf-flags-02.d
>> >> >  create mode 100644 ld/testsuite/ld-avr/relax-elf-flags-03.d
>> >> >  create mode 100644 ld/testsuite/ld-avr/relax-elf-flags-04.d
>> >> >  create mode 100644 ld/testsuite/ld-avr/relax-elf-flags-05.d
>> >> >  create mode 100644 ld/testsuite/ld-avr/relax-elf-flags-06.d
>> >> >  create mode 100644 ld/testsuite/ld-avr/relax-elf-flags-07.d
>> >> >  create mode 100644 ld/testsuite/ld-avr/relax-elf-flags-08.d
>> >> >  create mode 100644 ld/testsuite/ld-avr/relax-elf-flags-a.s
>> >> >  create mode 100644 ld/testsuite/ld-avr/relax-elf-flags-b.s
>> >> >
>> >> > --
>> >> > 1.9.3
>> >> >