This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: PATCH: Update x86-64/sysdep.h
- From: Andreas Jaeger <aj at suse dot com>
- To: "H.J. Lu" <hjl dot tools at gmail dot com>
- Cc: Roland McGrath <roland at hack dot frob dot com>,GNU C Library <libc-alpha at sourceware dot org>
- Date: Tue, 22 May 2012 10:11:40 +0200
- Subject: Re: PATCH: Update x86-64/sysdep.h
- References: <CAMe9rOpVAxxZ2AWOqrMwikZF7QakL6Gc1raHPMeeV0b=it=zew@mail.gmail.com>
On Tuesday, May 22, 2012 00:12:09 H.J. Lu wrote:
> On Mon, May 21, 2012 at 2:22 PM, Roland McGrath <roland@hack.frob.com>
wrote:
> >> Here is what I implemented. Since I need to refine
> >> SYSCALL_ERROR_HANDLER anyway, I use it to define my own error
> >> handler. Does it look OK?
> >>
> >> I noticed that SYSCALL_ERROR_HANDLER has
> >>
> >> xorl %edx, %edx; \
> >> subq %rax, %rdx; \
> >> movl %edx, (%rcx); \
> >> orq $-1, %rax; \
> >> jmp L(pseudo_end);
> >>
> >> Why not simply "neg %rax"?
> >
> > No idea. It appears it was done that way in the first version of this
> > file, added by AJ.
> >
> >> Also why not just "ret" instead of jmp?
I don't remember why I did this but see the same code in i386/sysdep.h. If
we change the x86-64 code, I suggest to revisit the i386 one as well.
This part looks fine to me.
> >
> > Likewise. This one seems downright wrong, since theoretically the
> > code at the label might not be just "ret". That code (i.e. whatever
> > comes after the PSEUDO macro invocation) was always meant to be only
> > the code for the success case.
It's the same code as in i386/sysdep.h - let's change both if we want that.
But I'm not sure whether this change is really correct, have a look at
sysdeps/unix/sysv/linux/x86_64/sched_getcpu.S
It contains:
L(pseudo_end):
add $0x8, %rsp
cfi_adjust_cfa_offset(-8)
ret
A simple return in the error case would be wrong.
Since both i386 and x86-64 do this, I would not make the change Roland
proposes unless we have reviewed all code that does cleanup after the
pseudo_end label.
So, please do not put this in for now.
> >
> > Those two changes are unrelated to x32, so you can submit them
> > separately and see if anybody has any comment.
>
> Here is the patch. Tested on Linux/x86-64 and Linux/x32. Any
> objections?
Andreas
--
Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn,Jennifer Guild,Felix Imendörffer,HRB16746 (AG Nürnberg)
GPG fingerprint = 93A3 365E CE47 B889 DF7F FED1 389A 563C C272 A126