Hi Mark,

attached updated patch; no code changes were made, only wording got updated.


On Sun, 17 Dec 2006 14:06:08 +0100, Mark Kettenis wrote:
> >  On Wed, 13 Dec 2006 23:11:11 +0100, Jan Kratochvil wrote:
> >
> >  On the other hand as a legacy compatibility I wrote the attached patch to
> >  fix the GDB excessive non-CFI amd64 `clone' unwind as 0x0.  Other
> >  platforms(and functions?) could be carbon-copied from this one.
> 
> Do people indeed think the legacy support is worth the extra complication?

The patch was provided AS IS.  It is a mandatory and useful part of the
customer support.  It may not be worth the maintenance for the upstream
development but this decision does not belong to me.


> >  I consider it the same approach as the way GDB currently on amd64 detects
> >  signal frames by checking the instructions - the CFI for signal frames
> >  has been also recently checked to the development glibc as was the
> > `clone' CFI.
> 
> Hmm, now if the CFI on 'clone' is incorrect and doesn't mark the return
> address as undefined, things will still fail.  You're sure there are no
> official glibc releases with bogus CFI for 'clone'?

This question is very intentionally out of the scope of this patch.
This patch only fixes clearly broken unwind of ``clone' without any CFI'.

Another patch may address a broken `clone' CFI but I do not have
a bugreport/reproducibility for such case.


> Anyway, most of it is irrelevant for ChangeLog.  New functions, defines, etc.
> should just be listed as New function, new define, etc.

Thanks [6.8.1 Change Log Concepts]; it is really not easy to perceive all the
GNU Coding Standards details.


> >  +static const unsigned char linux_clone_code[] =
> >  +{
> >  +/* libc/sysdeps/unix/sysv/linux/x86_64/clone.S */
> >  +/* #ifdef RESET_PID */
> >  +/* ... */
> >  +/* 	movl	$SYS_ify(getpid), %eax */
> >  +  0x48, 0xc7, 0xc0, 0x27, 0x00, 0x00, 0x00,
> >  +/* 	syscall */
> >  +  0x0f, 0x05,
> >  +/* 	movl	%eax, %fs:PID */
> >  +  0x64, 0x89, 0x04, 0x25, 0x94, 0x00, 0x00, 0x00,
> >  +/* 	movl	%eax, %fs:TID */
> >  +  0x64, 0x89, 0x04, 0x25, 0x90, 0x00, 0x00, 0x00,
> >  +/* #endif */
> >  +/* 	|* Set up arguments for the function call.  *| */
> >  +/* 	popq	%rax		|* Function to call.  *| */
> >  +  0x58,
> >  +/* 	popq	%rdi		|* Argument.  *| */
> >  +  0x5f,
> >  +/* 	call	*%rax$   */
> >  +  0xff, 0xd0
> >  +};
> 
> The #ifdefs here make me a bit nervous

Regarding GDB compilation or GLIBC compatibility?

I tried to verbatim copy the original source code.  It suggests the possible
point of a failure for systems with glibc compiled `!RESET_PID'.


> Sorry, again I don't understand your explanation here.  Do you have a
> collegue who is a native english speaker who can help you with writing
> english?

My code comments always have been a second grade text as nobody was reading it
before.  I understand it should change if submitting the code for review.



Regards,
Jan