|
Message-ID: <72c68934-4445-c83d-7bbc-004953b2f9e9@bitwagon.com> Date: Sat, 30 Dec 2017 20:15:41 -0800 From: John Reiser <jreiser@...wagon.com> To: musl@...ts.openwall.com Subject: Re: [PATCH] Add comments to i386 assembly source On 12/23/2017 09:45 UTC, Markus Wichmann wrote: > But then there's i386. Without comments, and pulling off some very black > magic, I thought it would be worth commenting the files at least in the > threads directory. > - mov $120,%al > + mov $120,%al /* __NR_clone */ Using an actual symbol is clearer and easier to maintain or modify: +__NR_clone = 120 + mov $__NR_clone,%al Constant arguments to system calls (including the system call number) should be loaded last in order to provide the least constraints for computing non-constant arguments. Also, it is not obvious that as values (%eax == %al). The value in %eax was set by "xor %eax,%eax; ...; mov %gs,%ax; ...; shr $3,%eax"; what guarantees that (%gs <= (255 << 3)) ? %gs could be as high as (8191 << 3). So _that_ deserves a comment; else for safety all of %eax should be set: + push $__NR_clone; pop %eax /* 3 bytes; __NR_clone < 128 */ + int $128 /* clone(flags, stack, TID pointer, {.index = current gs index, .base = thread pointer, .limit=0xfffff, .seg32_bit, .limit_in_pages, .usable}, td pointer) */ Clarity can be improved by using a symbol: NBPW = 4 /* Number of Bytes Per Word */ mov 3*NBPW(%ebp),%ecx /* ecx = stack */ mov 4*NBPW(%ebp),%ebx /* ebx = flags */ etc. Incorrect comment: > + sub $16,%ecx /* align stack */ Perhaps you meant "/* allocate space for returned segment descriptor */"? The alignment is performed by: and $-4*NBPW,%ecx /* align for stack */ If you are aiming for small space then + mov %eax,%ebx /* exit(rv from function) */ can be implemented one byte smaller as: + xchg %eax,%ebx /* syscall arg %ebx = rv from function; %eax = do not care */ --
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.