Re: [PATCH] Fix alignment for the first section frag on xtensa

[Sterling Augustine <augustine dot sterling at gmail dot com>]

On Mon, Apr 21, 2014 at 3:54 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> Linking object files produced by partial linking with link-time
> relaxation enabled sometimes fails with the following error message:
>
> dangerous relocation: call8: misaligned call target: (.text.unlikely+0x63)
>
> This happens because no basic block with an XTENSA_PROP_ALIGN flag in the
> property table is generated for the first basic block, even if the
> .align directive is present.
> It was believed that the first frag alignment could be derived from the
> section alignment, but this was not implemented for the partial linking
> case: after partial linking first frag of a section may become not
> first, but no additional alignment frag is inserted before it.
> Basic block for such frag may be merged with previous basic block into
> extended basic block during relaxation pass losing its alignment
> restrictions.

It seems likely to me that I could construct a case that made for a
failure even after this patch is applied.

The real bug is in the linker relaxation mechanism for not honoring
the section's alignment request. Linking should not change the
alignment of the subsection, without regard to special Xtensa
extensions--that violates Elf semantics.

Nonetheless, as the fix is likely to modify the linker so that the
alignment of the section is propagated to the first basic block,
pre-empting that by having the assembler do it is probably OK. So I
approve this patch.

Now would be a good time to get write-after-approval permission, so
you can check this in yourself.


>
> Fix this by always recording alignment for the first section frag.
>
> 2014-04-15  Max Filippov <jcmvbkbc@gmail.com>
>
> gas/
>     * config/tc-xtensa.c (xtensa_handle_align): record alignment for the
>     first section frag.
>
> gas/testsuite/gas/xtensa/
>     * all.exp: Add test for the first section frag alignment.
>     * trampoline.d: First section frag alignment expected dump.
>     * trampoline.s: First section frag alignment test source.
> ---
>  gas/config/tc-xtensa.c                      | 1 -
>  gas/testsuite/gas/xtensa/all.exp            | 1 +
>  gas/testsuite/gas/xtensa/first_frag_align.d | 9 +++++++++
>  gas/testsuite/gas/xtensa/first_frag_align.s | 5 +++++
>  4 files changed, 15 insertions(+), 1 deletion(-)
>  create mode 100644 gas/testsuite/gas/xtensa/first_frag_align.d
>  create mode 100644 gas/testsuite/gas/xtensa/first_frag_align.s
>
> diff --git a/gas/config/tc-xtensa.c b/gas/config/tc-xtensa.c
> index ea23c96..58ace38 100644
> --- a/gas/config/tc-xtensa.c
> +++ b/gas/config/tc-xtensa.c
> @@ -5609,7 +5609,6 @@ xtensa_handle_align (fragS *fragP)
>        && ! fragP->tc_frag_data.is_literal
>        && (fragP->fr_type == rs_align
>           || fragP->fr_type == rs_align_code)
> -      && fragP->fr_address + fragP->fr_fix > 0
>        && fragP->fr_offset > 0
>        && now_seg != bss_section)
>      {
> diff --git a/gas/testsuite/gas/xtensa/all.exp b/gas/testsuite/gas/xtensa/all.exp
> index 3683b78..b1e6c82 100644
> --- a/gas/testsuite/gas/xtensa/all.exp
> +++ b/gas/testsuite/gas/xtensa/all.exp
> @@ -99,6 +99,7 @@ if [istarget xtensa*-*-*] then {
>      run_dump_test "weak-call"
>      run_dump_test "jlong"
>      run_dump_test "trampoline"
> +    run_dump_test "first_frag_align"
>  }
>
>  if [info exists errorInfo] then {
> diff --git a/gas/testsuite/gas/xtensa/first_frag_align.d b/gas/testsuite/gas/xtensa/first_frag_align.d
> new file mode 100644
> index 0000000..aafcb41
> --- /dev/null
> +++ b/gas/testsuite/gas/xtensa/first_frag_align.d
> @@ -0,0 +1,9 @@
> +#as:
> +#objdump: -s
> +#name: record alignment for the first section frag
> +
> +.*: +file format .*xtensa.*
> +#...
> +Contents of section .xt.prop:
> + 0000 00000000 00000000 00002804 .*
> +#...
> diff --git a/gas/testsuite/gas/xtensa/first_frag_align.s b/gas/testsuite/gas/xtensa/first_frag_align.s
> new file mode 100644
> index 0000000..c120af0
> --- /dev/null
> +++ b/gas/testsuite/gas/xtensa/first_frag_align.s
> @@ -0,0 +1,5 @@
> +       .text
> +       .align  4
> +f1:
> +       entry   a1, 32
> +       retw
> --
> 1.8.1.4
>