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: Update x86-64/sysdep.h


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


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