This is the mail archive of the libc-alpha@sourceware.org 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: PATCH: Add x32 dl-machine.h


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.

Attachment: 0061-Add-x32-dl-machine.h.patch
Description: Binary data


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