PATCH: Add x32 dl-machine.h

[H.J. Lu]

On Mon, May 14, 2012 at 11:09 AM, Roland McGrath <roland@hack.frob.com> wrote:
>> +#include <sysdeps/x86_64/dl-machine.h>
>> +
>> +#ifndef x32_dl_machine_h
>> +#define x32_dl_machine_h
>
> Though this is done inconsistently already, I think for new files we should
> be consistent and use an all-caps identifier starting with underscore.

Done.

> I see no reason the #include has to be outside the guard.  I know it
> doesn't hurt since it's separately guarded, but it is unusual to see any
> non-comment contents outside a file's guard, so please move it inside.

I can't since dl-machine.h is included more than once. See
#ifdef RESOLVE_MAP in sysdeps/x86_64/dl-machine.h.


>> +#define RTLD_START asm ("\n\
>> +.text\n\
>> +     .align 16\n\
>
> Always use .p2align or .balign in new code.

Done.

>> +     # Pop the original argument count.\n\
>> +     movl (%rsp), %edx\n\
>> +     addl $4,%esp\n\
>> +     # Adjust the stack pointer to skip _dl_skip_args words.\n\
>> +     lea (%rsp,%rax,4), %esp\n\
>
> Just use "lea 4(%rsp,%rax,4), %esp" rather than two instructions.

Good point.

>> +     # Push argc back on the stack.\n\
>> +     subl $4,%esp\n\
>
> Space after comma.

Done.

>> +     # And align stack for the _dl_init_internal call. \n\
>
> Excess space before \n.

Done.

>> +     # Call the function to run the initializers.\n\
>> +     call _dl_init_internal@PLT\n\
>
> Why does this need @PLT when the _dl_start call above didn't?
> It seems especially odd since it's calling the INTUSE name whose
> whole purpose is not to need the PLT.
>

Done.

Here is the updated patch.  Tested on Linux/x32.  OK to install?

Thanks.


-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0061-Add-x32-dl-machine.h.patch
Type: application/octet-stream
Size: 3201 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20120514/38ff13b5/attachment.obj>