[PATCH] Not append rela for abs_symbol

[ywgrit]

在 2024/8/15 下午2:40, Xi Ruoyao 写道:
> On Thu, 2024-08-15 at 14:12 +0800, Xin Wang wrote:
>> LoongArch: Not append rela for absolute symbol
>>
>> Use la.global to get absolute symbol like la.abs.
>> la.global put address of a global symbol into got and
>> append a rela for it, which will be used to relocate by
>> dynamic linker. Dynamic linker should not relocate for
>> got entry of absolute symbol as it stores symval not
>> symbol's address.
>>
>> Signed-off-by: Xin Wang <wangxin03@loongson.cn>
> You shouldn't use S-o-b if this is done as a Loongson employee (in the
> working time): the attribution should belong to the Loongson company,
> and the company uses FSF copyright assignment instead of DCO.
Thanks.
>
> Some comment follows.
>
> /* snip */
>
>> @@ -5482,7 +5485,18 @@ loongarch_elf_relax_section (bfd *abfd,
>> asection *sec,
>>   	  break;
>>   
>>   	case R_LARCH_GOT_PC_HI20:
>> +	  if (h)
>> +	    is_abs_symbol = bfd_is_abs_section(h-
>>> root.u.def.section);
>> +	  else
>> +	    {
>> +	      Elf_Internal_Sym *sym = (Elf_Internal_Sym *)symtab_hdr-
>>> contents
>> +				    + ELFNN_R_SYM (rel->r_info);
>> +	      is_abs_symbol = sym->st_shndx == SHN_ABS;
>> +	    }
>>   	  if (local_got && 0 == info->relax_pass
>> +	      // We can not change the r_type which will be
>> +	      // needed when relocate for absolute symbol.
> AFAIK BFD code base prefer /* ... */ over //.
>
> Technically we can relax pcalau12i + ld.d to lu12i.w + ori if the
> absolute symbol is in [-2^31, 2^31), and then r_type can be changed to
> R_LARCH_ABS_{HI20,LO12}.  It's OK not to implement this in the patch,
> but the comment should say "not implemented yet" instead of "cannot."

Yeah, you are right. I didn't realize there is a possibility of applying 
relax here for absolute symbol, Do you think I should implement relax 
first before submitting new code?

>> +	      && !is_abs_symbol
>>   	      && (i + 4) <= sec->reloc_count)
>>   	    {
>>   	      if (loongarch_relax_pcala_ld (abfd, sec, rel))
> A test case should be added into ld/testsuite/ld-loongarch-elf/ld-
> loongarch-elf.exp.

Yes, thanks.