Re: [PATCH 1/2] LoongArch: bfd: Remove elf_seg_map condition in loongarch_elf_relax_section

[mengqinggang <mengqinggang@loongson.cn>]

Hi,

I will apply this two patches to see if there are any errors in specc2006.

在 2023/7/12 下午4:08, Jinyang He 写道:
> Hi,
>
>
> I'm not fimiliar with the process of output binary. I just noted that 
> it calls lang_relax_sections() before calling 
> _bfd_elf_map_sections_to_segments() in ldelf_map_segments(). Thus I 
> think the condition, elf_seg_map (info->output_bfd) == NULL, always be 
> true when first time it calls loongarch_elf_relax_section(). Actually 
> we can get right symbol value when relaxation, but ...
>
> You reminder me that the symbol value will be reduced due to 
> relaxation, which like the instruction pc is reduced. Then we cannot 
> get right symbol value if the symbol in relaxable section. And I write 
> a test,
>
> $ cat c.s
>     .section ".another.text", "x"
>     nop
>     la.local $a0, sym1
> sym2:
>     nop
>
>     .text
> sym1:
>     nop
>     nop
>     nop
>     la.local $a0, sym2
>
> $ ./gas/as-new -mabi=lp64d -o c.o c.s
> $ ./ld/ld-new c.o -o c -e 0 --section-start=.another.text=0x120000000 
> -Ttext 0x120200000
>
> Disassembly of section .another.text:
>
> 0000000120000000 <sym2-0x8>:
>    120000000:    03400000     andi            $zero, $zero, 0x0
>    120000004:    18ffffe4     pcaddi          $a0, 524287(0x7ffff)
>
> 0000000120000008 <sym2>:
>    120000008:    03400000     andi            $zero, $zero, 0x0
>
> Disassembly of section .text:
>
> 0000000120200000 <sym1>:
>    120200000:    03400000     andi            $zero, $zero, 0x0
>    120200004:    03400000     andi            $zero, $zero, 0x0
>    120200008:    03400000     andi            $zero, $zero, 0x0
>    12020000c:    18ffffe4     pcaddi          $a0, 524287(0x7ffff)
>
> Due to the bug repaired by Patch2 and the bug reported by this test. 
> And I also did not see that condition in other archs. I'm not sure 
> that the relaxation spec2006 error is caused by that condition or 
> those bugs.
>
>
> On 2023-07-12 14:35, mengqinggang wrote:
>> Hi,
>>
>> Without this condition, the value of 'symbol - pc' may be differences 
>> between
>> loongarch_elf_relax_section and  loongarch_elf_relocate_section.
>> And it will cause some spec2006 test errors.
>>
>> 在 2023/7/11 下午4:49, Jinyang He 写道:
>>> When I tried "./ld-new sth." the relax does not work because the
>>> elf_seg_map (info->output_bfd) was NULL in loongarch_elf_relax_section.
>>> There is no need for the segment info, we can get the address through
>>> the section info. Thus, remove this condition.
>>>
>>> bfd/ChangeLog:
>>>
>>>     * elfnn-loongarch.c (loongarch_elf_relax_section): Remove
>>>     elf_seg_map condition.
>>>
>>> ld/ChangeLog:
>>>
>>>     * testsuite/ld-loongarch-elf/relax.exp: Update trigger method.
>>>     * testsuite/ld-loongarch-elf/relax.dd: New test.
>>> ---
>>>   bfd/elfnn-loongarch.c                   |  1 -
>>>   ld/testsuite/ld-loongarch-elf/relax.dd  |  6 +++++
>>>   ld/testsuite/ld-loongarch-elf/relax.exp | 35 
>>> +++++++------------------
>>>   3 files changed, 16 insertions(+), 26 deletions(-)
>>>   create mode 100644 ld/testsuite/ld-loongarch-elf/relax.dd
>>>
>>> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
>>> index d3d8419d8..01f349a24 100644
>>> --- a/bfd/elfnn-loongarch.c
>>> +++ b/bfd/elfnn-loongarch.c
>>> @@ -3845,7 +3845,6 @@ loongarch_elf_relax_section (bfd *abfd, 
>>> asection *sec,
>>>         || sec->sec_flg0
>>>         || (sec->flags & SEC_RELOC) == 0
>>>         || sec->reloc_count == 0
>>> -      || elf_seg_map (info->output_bfd) == NULL
>>>         || (info->disable_target_specific_optimizations
>>>         && info->relax_pass == 0)
>>>         /* The exp_seg_relro_adjust is enum phase_enum (0x4),
>>> diff --git a/ld/testsuite/ld-loongarch-elf/relax.dd 
>>> b/ld/testsuite/ld-loongarch-elf/relax.dd
>>> new file mode 100644
>>> index 000000000..e266beb8f
>>> --- /dev/null
>>> +++ b/ld/testsuite/ld-loongarch-elf/relax.dd
>>> @@ -0,0 +1,6 @@
>>> +#...
>>> +.*pcaddi.*
>>> +.*ld.w.*
>>> +.*pcaddi.*
>>> +.*ld.w.*
>>> +#pass
>>> diff --git a/ld/testsuite/ld-loongarch-elf/relax.exp 
>>> b/ld/testsuite/ld-loongarch-elf/relax.exp
>>> index 7ff876d79..d2bb967c6 100644
>>> --- a/ld/testsuite/ld-loongarch-elf/relax.exp
>>> +++ b/ld/testsuite/ld-loongarch-elf/relax.exp
>>> @@ -21,31 +21,6 @@
>>>     if [istarget loongarch64-*-*] {
>>>   -  if [isbuild loongarch64-*-*] {
>>> -    set testname "loongarch relax build"
>>> -    set pre_builds [list \
>>> -      [list \
>>> -    "$testname" \
>>> -    "" \
>>> -    "" \
>>> -    {relax.s} \
>>> -    {} \
>>> -    "relax" \
>>> -      ] \
>>> -    ]
>>> -
>>> -    run_cc_link_tests $pre_builds
>>> -
>>> -    if [file exist "tmpdir/relax"] {
>>> -      set objdump_output [run_host_cmd "objdump" "-d tmpdir/relax"]
>>> -      if { [ regexp ".*pcaddi.*pcaddi.*" $objdump_output] } {
>>> -    pass "loongarch relax"
>>> -      } {
>>> -    fail "loongarch relax"
>>> -      }
>>> -    }
>>> -  }
>>> -
>>>     run_ld_link_tests \
>>>         [list \
>>>         [list \
>>> @@ -58,6 +33,16 @@ if [istarget loongarch64-*-*] {
>>>             ] \
>>>             "relax-align" \
>>>         ] \
>>> +      [list \
>>> +          "relax" \
>>> +          "-e 0 -Ttext 0x10000 -Tdata 0x20000" "" \
>>> +          "" \
>>> +          {relax.s} \
>>> +          [list \
>>> +          [list objdump -d relax.dd] \
>>> +          ] \
>>> +          "relax" \
>>> +      ] \
>>>         ]
>>>       set objdump_flags "-s -j .data"