https://sourceware.org/bugzilla/show_bug.cgi?id=3298
--- Comment #9 from Joern Rennecke <amylaar at gcc dot gnu.org> ---
(In reply to Quentin Boswank from comment #7)
> (In reply to Joern Rennecke from comment #3)
> > One thing that I just noticed while browsing through
> > sh_elf_relax_delete_bytes
> > is that we don't process R_SH_ALIGN properly; i.e. when we stop at a larger
> > alignment, we should should check if we should now remove bytes of the full
> > size of the alignment.
>
> Could you please elaborate on that? I was not able to find a problem in the
> current handling of aligns, maybe an example would help me.
Hmm, it seems I have missed the code at the end of the function, after the
comment:
/* See if we can move the ALIGN reloc forward. We have adjusted
r_offset for it already. */ .
In my defence, it's about 450 lines down from the code that initially handled
the
alignment, and the comment for the latter talks about stopping the deletion,
without
a hint that it's going to be taken up later.
/* The deletion must stop at the next ALIGN reloc for an alignment
power larger than the number of bytes we are deleting. */
Maybe that comment could be expanded upon, something like that?
/* The deletion must stop at the next ALIGN reloc for an alignment
power larger than the number of bytes we are deleting; we'll check
later if such an alignment can be trimmed too. */
Also, looking at the code, it occurs to me that it is a bit brittle. Although
this can't happen at the moment, if some new relaxation called
sh_elf_relax_delete_bytes
with a count value that is not a power of two (e.g. six), we could end up with
R_SH_ALIGN relocs that are not aligning properly.
We could add an assert, like:
if (ELF32_R_TYPE (irel->r_info) == (int) R_SH_ALIGN
&& irel->r_offset > addr)
{
if (count < (1 << irel->r_addend))
{
irelalign = irel;
toaddr = irel->r_offset;
break;
}
BFD_ASSERT ((count & ((1 << irel->r_addend) - 1)) == 0);
}
or simpler, just handle the hypothetical situation - although we'd lack
testing,
and the comment should probably be adjusted too:
if (ELF32_R_TYPE (irel->r_info) == (int) R_SH_ALIGN
&& irel->r_offset > addr
&& (count & ((1 << irel->r_addend) - 1)) != 0)
{
irelalign = irel;
toaddr = irel->r_offset;
break;
}
N.B. I'm not going to test any of this, I have no current connection with the
SH port.
--
You are receiving this mail because:
You are on the CC list for the bug.
