Re: don't assume reloc_addr passed to elf_machine_rel* is aligned

[Alexandre Oliva <aoliva at redhat dot com>]

On Jun 24, 2003, Roland McGrath <roland@redhat.com> wrote:

>> I find explicit casts ugly, and I don't know whether every
>> architecture I touched may have unaligned relocations.  

> I was not suggesting introducing explicit casts in functions which had none
> at all, but rather consistently using one cast instead of two (one
> implicit, one explicit) in functions that do that after your patch.

I'm afraid I still don't understand what you mean :-(  Care to post an
example?

>> My patch was a syntactical-only change.

> Ok.  I don't have sufficient knowledge of the aliasing rules or the
> compiler's workings to be confident just from looking that they are in fact
> guaranteed such.

It's not about aliasing rules.  It's about preventing the pointer from
ever being cast to ElfW(Addr)* if it might not be properly aligned,
because, after the cast, even if it is cast back, you may get
different alignment assumptions.  Casting to a type that requires
additional alignment may lose information (e.g., on some odd
architectures, it may actually lose the least-significant bits), and
no amount of casting back will recover it.  The compiler may,
therefore, make assumptions about the alignment of cast-back pointers,
even if the architecture doesn't drop bits in such casts.

The problem is that elf_machine_rel() takes an ElfW(Addr)*, which
prevents the implementation of ABIs that demand relocations to be
applicable to unaligned addresses, since elf_machine_rel() may get an
argument that has already lost the relevant bits or, that has
incorrect alignment assumptions associated with it.

Note that, with my patch, I'm just changing the interface of
elf_machine_rel(), such that it matches a prototype that would enable
architectures with such requirements to be implemented, without
modifying any of the existing architectures other than by
re-introducing a variable with the same name and type that they
formerly had as an argument before.  This is a purely mechanical
change, that means no actual change in behavior may have been
introduced with the patch, barring possible compiler bugs.

> But if you're quite sure they are, that's good enough for me.
> However, the "reloc_addr_" variable name is just too ugly.

Would reloc__addr be any better?  If not, what do you suggest instead?

> It's easy enough to notice machines where there is certainly no
> alignment issue, such as i386 where all the stores do use the same
> 32-bit type.

It's not that simple: i386 unaligned loads and stores work, but incur
a performance penalty, and if there was code that, e.g., tested
whether the lower bits indicated unaligned, to report a warning, the
test could be legally optimized away, because of the implied alignment
of the pointer type.

And, again, the issue is not about what is currently implemented, it's
about what each ABI mandates regarding alignment of addresses to which
relocations are applied.  Some ABIs do it on a per-relocation basis,
some require uniform alignment, disallowing relocations to unaligned
addresses, and some allow relocations to unaligned addresses
uniformly.  I don't know which requirements are imposed to each
architecture, and I'm not looking forward to trying to find that out
:-)  I see that SH has code to support unaligned relocations, and I
think I saw something in the alpha port as well.  I'm not worried.
GCC doesn't seem to have issues with the code that way it is now, even
if the standard grants it more freedom in making alignment assumptions
for the sake of optimizing ``unnecessary'' code away.  If it did, SH
would certainly be in trouble.

>> It didn't mean to fix any bug in any architecture

> But you have a generous spirit much too large for us to have beaten it all
> out of you yet.

I wish...  I have no pleasure in working in glibc any longer.  My
(over?)reaction to Uli's postings is a clear symptom of this.  I just
want to finish this work that I have been asked to do and move on.
Maybe some day I'll come back, but my interest has faded, and this
may take a while to mend.

-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                 aoliva@{redhat.com, gcc.gnu.org}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist                Professional serial bug killer