Re: [PATCH] PR30437 aarch64: make RELA relocs idempotent

["Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com>]

On 24/05/2023 17:03, Michael Matz via Binutils wrote:
> Hello,
> 
> On Wed, 24 May 2023, Richard Earnshaw (lists) wrote:
> 
>>> Tested for aarch64-elf and aarch64-linux (and the other ~150 targets,
>>> though those can't be affected).  Also tested natively on
>>> aarch64-suse-linux-gnu.  Okay for master?
>>>
>>
>> Isn't this what partial_inplace is supposed to describe?  Since that's
>> false, I'd expect any value stored in the section contents to be
>> ignored.
> 
> One might think so, yes.  But in practice that doesn't seem to be the case
> (anymore?).  I can't quite wrap my head around the logic in reloc.c and
> interaction between partial_inplace and src_mask.  The documentation says
> that partial_inplace _and_ src_mask should be zero for RELA targets (or
> rather, that other values are suspicious), and obviously having the
> former, but not the latter does make a difference.  (FWIW, x86_64 recently
> was fixed as well to have zero src_mask, and most other RELA targets also
> have it zero)
> 
> It's possible that partial_inplace only affects partial links (ld -r), at
> least the docu for that field specifically talks about only that, whereas
> here (the go bug) we have the problem of a final link where the section
> contents are non-zero.  I don't know if partial_inplace should or should
> not also affect final links, and if it's a bug that src_mask matters even
> with !partial_replace.  But one of the two needs to yield: either the
> logic regarding partial_inplace in reloc.c (so that src_mask isn't
> relevant anymore), or src_mask needs to be zero.  The advantage of the
> latter change is that it's aarch64-specific, whereas a change to
> partial_inplace handling affects everything and needs more bravery than I
> have.
> 
> Either way, even _if_ partial_inplace would be made so that src_mask
> doesn't matter anymore, then the change to zero src_mask would then still
> be at least harmless.
> 
> (skimming some more uses of src_mask relative to uses of partial_inplace
> in the aarch64 target also shows some more possibly dubious consideration
> of original section contents in _bfd_aarch64_elf_put_addend.  It always
> looks at the original content and uses src_mask as selector of being for
> insns or data, but then goes on to mask the read contents only against
> dst_mask, and not against src_mask.  I don't quite know what to make of
> that)
> 
> 
> Ciao,
> Michael.

Thanks for the detailed explanation.  That makes sense, overall.  However...

I didn't think we had separate relocation rules for REL/RELA format 
relocs.  I know that we recommend RELA for aarch64, but how would an 
object file with REL format relocs be handled if src_mask is always zero?

R.