[PATCH 1/2] LoongArch: bfd: Remove elf_seg_map condition in loongarch_elf_relax_section
Jinyang He
hejinyang@loongson.cn
Thu Jul 13 08:18:50 GMT 2023
On 2023-07-13 15:19, Jinyang He wrote:
> Hi,
>
>
> We should pay attention to them, the bug in relaxation. Relaxation may
> not work correctly because of the bug1 reported by the second patch
> and the bug2 reported in the my previous email, although it works well
> in most of the time. Those 2 bugs are just what we've found, but there
> may be hidden problems.
> I also tested the bug2 on RISCV and although it couldn't be resolved,
> it reported an error.
>
> Test is,
> $ cat c.s
> .section ".another.text", "x"
> .fill 1, 4, 0
> call fn2
> call fn2
> fn1:
> nop
>
> .text
> fn2:
> .fill 4, 4, 0
> call fn1
>
> $ ./gas/as-new -march=rv32ic_zicsr -mno-arch-attr -o c.o c.s
> $ ./ld/ld-new c.o -o c -e 0 --section-start=.another.text=0x1000000
> -Ttext 0x1100000
> c.o: in function `fn2':
> (.text+0x10): relocation truncated to fit: R_RISCV_JAL against `fn1'
>
> So, I'll find the reason why reloc_bits_pcrel20_s2() not reports the
> overfollow error, and add error report which I think it is more
> important than these two patches so that we can get feedback when we
> link.
>
Now, I find that the sign-overflow-check has bug, include
reloc_bits_pcrel20_s2(), reloc_bits_b26() and (others, I did not test
them). And I think the "mask" should be
bfd_signed_vma mask = ((bfd_signed_vma)0x1 << (howto->bitsize -
(sign ? 1 : 0)) - 1;
The test about reloc_bits_b26() is,
$ cat d.s
.section ".another.text", "x"
b sym1
.text
nop
sym1:
nop
$ ./gas/as-new -mabi=lp64d -o d.o d.s
$ ./ld/ld-new d.o -o d -e 0 --section-start=.another.text=0x130000000
-Ttext 0x120000000
I'll do fix and send v2 after you test spec2006. Or other responses?
Thanks!
>
> On 2023-07-13 11:13, mengqinggang wrote:
>> 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"
More information about the Binutils
mailing list