This is the mail archive of the libc-alpha@sources.redhat.com mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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

s390/s390-32/dl-machine.h for example uses Elf32_Addr *reloc_addr for most
of its reloc types, but casts it to char* or short* for some relocs.  As I
understand it, the safer thing would be to use void *reloc_addr and use
`*(Elf32_Addr *) reloc_addr' each place plain `*reloc_addr' now appears.
(sparc32 does the same, probably several others; I haven't looked at them all).

Even in a case like i386, I wonder if the use as void * in memcpy for
R_386_COPY relocs might be a possible source of trouble.  The compiler is
free to turn that memcpy into inline checks for sizes like 4 and use a word
store for that.  On x86 this is permissible since there are no hard
alignment constraints at all, but I mean a case whose elf_machine_rel*
function uses just one pointer type for all its reloc stores but the
machine doesn't permit unaligned word stores.  (I don't know for sure there
is such a case, not having checked every single machine's function just now.)
Am I wrong about this?

> 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.  

That's the kind of thing I meant.  I was being imprecise.  Your precision
proves you know the issue better than I do. ;-)

> This is a purely mechanical change, that means no actual change in
> behavior may have been introduced with the patch, barring possible
> compiler bugs.

I understand that.  I am trying to take this opportunity to weed out future
problems that may be around the corner with the next compiler version.

> Would reloc__addr be any better?

You're kidding, right?

> If not, what do you suggest instead?

If it's proper for the code to have a local variable with the other type
and the argument is never used, the canonical thing is something like:

func (void *foobar_arg)
{
  othertype *foobar = foobar_arg;
  ...

If the argument void * is actually used elsewhere in the function (as it
should be if we fix the potential problems in cases like sparc32) and for
some reason it still makes sense to have the othertype* variable, use:

func (void *reloc_addr)
{
  Elf32_Addr *reloc32_addr = reloc_addr;
  ...

or something like that.  It just ain't hard to think up a more descriptive
way to modify a variable name than inserting an underscore.

> It's not that simple: [...]

It is that simple to see that i386 doesn't have such an issue, because all
occurences of `reloc_addr' in the function look like `*reloc_addr'.
(Leaving aside the memcpy for copy relocs, which I hadn't thought about
when I wrote the last message.)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]