PATCH: Update x86-64/sysdep.h

[Andreas Jaeger]

On 05/22/2012 10:11 AM, Andreas Jaeger wrote:
> 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.

I reviewed all usage of pseudo end in sysdeps, only the two 
sched_getcpu.S versions were wrong.

HJ, please prepare a patch for both i386 and x86-64 to use ret instead 
of jmp and then send it again for review.

Thanks,
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