[PATCH 3/6] bfd: refine handling of relocations between debugging sections

[Jan Beulich]

On 09.03.2021 03:24, Alan Modra wrote:
> On Mon, Mar 08, 2021 at 03:17:54PM +0100, Jan Beulich wrote:
>> On 05.03.2021 13:00, Alan Modra wrote:
>>> Yes, it's true all the extra conditions don't inspire confidence.  If
>>> it's OK to do at all then it should be OK with just
>>>
>>>   if (output_bfd == NULL
>>>       && bfd_get_flavour (abfd) == bfd_target_elf_flavour
>>>       && (input_section->flags & SEC_DEBUGGING) != 0
>>>       && (symbol->section->flags & SEC_DEBUGGING) != 0)
>>>     output_base = 0;
>>>
>>> with a comment saying why, of course.
>>
>> While, as said, I agree the extra conditions don't look reassuring,
>> dropping them adds to what we imply towards relocations between
>> debugging sections. I understand you're okay with that (as far as
>> "okay" can go with a hack like this)?
>>
>> With the code snippet above I observe a few more modifications
>> than just to drop the extra conditions. You use symbol->section
>> when I used reloc_target_output_section. Is that for a reason?
>>
>> You've also split my
>>
>>      && (reloc_target_output_section->flags
>> 	 & input_section->flags & SEC_DEBUGGING))
>>
>> which was probably just a style change (including the addition of
>> () != 0 around them). While I don't really see the reason for
>> this transformation, if that's what binutils style requires I can
>> surely change.
> 
> Yes, I deliberatey changed the test of what is effectively
>   symbol->section->output_section->flags
> to
>   symbol->section->flags
> 
> You care about what is in the input section, not the output section.
> The output section may even be .data, for example.

Oh, yes, good point.

> (As far as the style issue goes, I wasn't unhappy with the way you
> wrote the expression.  The change was mostly to avoid a line-wrap in a
> place you wouldn't normally wrap if allowed very long lines.)

I see. With the change to use symbol->section the line wouldn't
need wrapping anymore, though.

>> However, meanwhile I've realized that
>> input_section's flags shouldn't really matter: If there indeed
>> was a relocation in a non-debugging section targeting a debugging
>> one, I would suppose output_base should be similarly ignored.
>> Thoughts? (Such relocations don't seem to make a lot of sense, so
>> perhaps this aspect is largely academical anyway.)
> 
> Yes, it likely is academic, but you put forward the idea of loaded
> DWARF data accessed by an application.  In that situation you want
> relocations from non-DWARF to DWARF to behave normally: The DWARF is
> just a chunk of data.

I guess this can really be viewed both ways: When relocations
between debugging sections are specified to be section relative,
relocations targeting a debugging sections may also be assumed to
be so - consuming code needs to add in section base addresses
anyway. But yes, perhaps checking both sides produces the more
"natural" result.

> Does this work for you?

I'll give this a try, perhaps later today. However, ...

> @@ -1323,6 +1322,19 @@ bfd_elf_generic_reloc (bfd *abfd ATTRIBUTE_UNUSED,
>        return bfd_reloc_ok;
>      }
>  
> +  /* In some cases the relocation should be treated as input section
> +     relative, as when linking ELF DWARF into PE COFF.  Many ELF
> +     targets lack section relative relocations and instead use
> +     ordinary absolute relocations for references between DWARF
> +     sections.  That is arguably a bug in those targets but it happens
> +     to work for the usual case of linking to non-loaded ELF debug
> +     sections with VMAs forced to zero.  PE COFF on the other hand
> +     doesn't allow a section VMA of zero.  */
> +  if (output_bfd == NULL
> +      && (symbol->section->flags & SEC_DEBUGGING) != 0
> +      && (input_section->flags & SEC_DEBUGGING) != 0)
> +    reloc_entry->addend -= symbol->section->output_section->vma;

... already in your reduced replacement suggestion to my change
to bfd_perform_relocation() you didn't only drop the "just to
be on the safe side" checks, but also the pc-relative one. Are
you sure there aren't any cases where such relocations might be
used, particularly when the relocation points back to the same
section?

Jan