Re: [PATCH] LoongArch: Adapt R_LARCH_{PCALA,GOT,TLS_IE,TLS_DESC}64_* handling per psABI v2.30

[mengqinggang <mengqinggang@loongson.cn>]

在 2024/1/16 下午2:48, Xi Ruoyao 写道:
> On Tue, 2024-01-16 at 14:38 +0800, mengqinggang wrote:
>> 在 2024/1/15 下午8:44, Xi Ruoyao 写道:
>>> In LoongArch psABI v2.30, an offset (-8 for LO20 and -12 for HI12)
>>> should be applied on PC for these reloc types to avoid wrong relocation
>>> when the instruction sequence crosses a page boundary.
>>>
>>> The lld linker has already adapted the change.  Make it for the bfd
>>> linker too.
>>>
>>> Link: https://github.com/loongson/la-abi-specs/releases/v2.30
>>> Link: https://github.com/loongson-community/discussions/issues/17
>>> Link: https://github.com/llvm/llvm-project/pull/73387
>>> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
>>> ---
>>>    bfd/elfnn-loongarch.c                         | 29 +++++++++++--------
>>>    .../ld-loongarch-elf/ld-loongarch-elf.exp     |  1 +
>>>    ld/testsuite/ld-loongarch-elf/pcala64.d       | 15 ++++++++++
>>>    ld/testsuite/ld-loongarch-elf/pcala64.s       |  8 +++++
>>>    4 files changed, 41 insertions(+), 12 deletions(-)
>>>    create mode 100644 ld/testsuite/ld-loongarch-elf/pcala64.d
>>>    create mode 100644 ld/testsuite/ld-loongarch-elf/pcala64.s
>>>
>>> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
>>> index 3858a3179fd..e4f4da7a0dc 100644
>>> --- a/bfd/elfnn-loongarch.c
>>> +++ b/bfd/elfnn-loongarch.c
>>> @@ -3544,14 +3544,16 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
>>>    	    }
>>>    	  break;
>>>    
>>> -	case R_LARCH_PCALA64_LO20:
>>>    	case R_LARCH_PCALA64_HI12:
>>> +	  pc -= 4;
>>> +	  /* Fall through.  */
>>> +	case R_LARCH_PCALA64_LO20:
>>>    	  if (h && h->plt.offset != MINUS_ONE)
>>>    	    relocation = sec_addr (plt) + h->plt.offset;
>>>    	  else
>>>    	    relocation += rel->r_addend;
>>>    
>>> -	  RELOCATE_CALC_PC64_HI32 (relocation, pc);
>>> +	  RELOCATE_CALC_PC64_HI32 (relocation, pc - 8);
>>
>> The 'pc - 8' triggered an error because the pc parameter is not enclosed
>> in parentheses in macro definition.
>>
>> /home/mengqinggang/toolchains/src/binutils-gdb/bfd/elfnn-loongarch.c:3546:51:
>> error: suggest parentheses around '-' in operand of '&'
>> [-Werror=parentheses]
>>    3546 |           RELOCATE_CALC_PC64_HI32 (relocation, pc - 8);
>>         |                                                ~~~^~~
>> /home/mengqinggang/toolchains/src/binutils-gdb/bfd/elfnn-loongarch.c:2532:22:
>> note: in definition of macro 'RELOCATE_CALC_PC64_HI32'
>>    2532 |                   - (pc & ~(bfd_vma)0xfff);             \
>>         |                      ^~
> Interesting, why didn't I see this error...  And in this case it happens
> to work because - has a higher precedence.  But I'll add parentheses in
> V2 anyway.
>
> /* snip */


Maybe you configured with --disable-werror?


>>> diff --git a/ld/testsuite/ld-loongarch-elf/pcala64.d b/ld/testsuite/ld-loongarch-elf/pcala64.d
>>> new file mode 100644
>>> index 00000000000..6b1411577db
>>> --- /dev/null
>>> +++ b/ld/testsuite/ld-loongarch-elf/pcala64.d
>>> @@ -0,0 +1,15 @@
>>> +#ld: -Ttext=0x180000ff8 -Tdata=0x1000000000 -shared
>>
>> The test need to be disabled on loongarch*-elf target because -shared
>> option is not supported on loongarch*-elf target.
>>
>> It can be disabled by  adding a line "#skip: loongarch*-elf" after
>> "#objdump -d".
> I'll just remove the -shared option.  I changed the function name to
> "_start" so it should work without -shared, but I forgot to actually
> remove -shared.
>
>