mailing list of musl libc
 help / color / mirror / code / Atom feed
From: Rich Felker <dalias@libc.org>
To: musl@lists.openwall.com
Subject: Re: [musl] [PATCH 1/3] setjmp: fix x86-64 longjmp argument adjustment
Date: Tue, 11 Aug 2020 14:45:28 -0400	[thread overview]
Message-ID: <20200811184527.GM3265@brightrain.aerifal.cx> (raw)
In-Reply-To: <20200811181116.8433-1-amonakov@ispras.ru>

On Tue, Aug 11, 2020 at 09:11:14PM +0300, Alexander Monakov wrote:
> longjmp 'val' argument is an int, but the assembly is referencing 64-bit
> registers as if the argument was a long, or the caller was responsible
> for extending the argument. Though the psABI is not clear on this, the
> interpretation in GCC is that high bits may be arbitrary and the callee
> is responsible for sign/zero-extending the value as needed (likewise for
> return values: callers must anticipate that high bits may be garbage).
> 
> Therefore testing %rax is a functional bug: setjmp would wrongly return
> zero if longjmp was called with val==0, but high bits of %rsi happened
> to be non-zero.
> 
> Rewrite the prologue to refer to 32-bit registers. In passing, change
> 'test' to use %rsi, as there's no advantage to using %rax and the new
> form is cheaper on processors that do not perform move elimination.
> ---
>  src/setjmp/x32/longjmp.s    | 6 +++---
>  src/setjmp/x86_64/longjmp.s | 6 +++---
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/src/setjmp/x32/longjmp.s b/src/setjmp/x32/longjmp.s
> index e175a4b9..e709acad 100644
> --- a/src/setjmp/x32/longjmp.s
> +++ b/src/setjmp/x32/longjmp.s
> @@ -5,10 +5,10 @@
>  .type longjmp,@function
>  _longjmp:
>  longjmp:
> -	mov %rsi,%rax           /* val will be longjmp return */
> -	test %rax,%rax
> +	mov %esi,%eax           /* val will be longjmp return */
> +	test %esi,%esi
>  	jnz 1f
> -	inc %rax                /* if val==0, val=1 per longjmp semantics */
> +	inc %eax                /* if val==0, val=1 per longjmp semantics */
>  1:
>  	mov (%rdi),%rbx         /* rdi is the jmp_buf, restore regs from it */
>  	mov 8(%rdi),%rbp
> diff --git a/src/setjmp/x86_64/longjmp.s b/src/setjmp/x86_64/longjmp.s
> index e175a4b9..e709acad 100644
> --- a/src/setjmp/x86_64/longjmp.s
> +++ b/src/setjmp/x86_64/longjmp.s
> @@ -5,10 +5,10 @@
>  .type longjmp,@function
>  _longjmp:
>  longjmp:
> -	mov %rsi,%rax           /* val will be longjmp return */
> -	test %rax,%rax
> +	mov %esi,%eax           /* val will be longjmp return */
> +	test %esi,%esi
>  	jnz 1f
> -	inc %rax                /* if val==0, val=1 per longjmp semantics */
> +	inc %eax                /* if val==0, val=1 per longjmp semantics */
>  1:
>  	mov (%rdi),%rbx         /* rdi is the jmp_buf, restore regs from it */
>  	mov 8(%rdi),%rbp
> -- 
> 2.11.0

Thanks! This whole series looks good and I'm applying it now.

Re: optimizing the tails, I wonder why they were ever written the way
they were to begin with. At least the i386 one was written by me so
there's nobody else to ask, but do you have any insight into what I
might have been thinking?

Rich

  parent reply	other threads:[~2020-08-11 18:45 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-11 18:11 Alexander Monakov
2020-08-11 18:11 ` [musl] [PATCH 2/3] setjmp: avoid useless REX-prefix on xor %eax, %eax Alexander Monakov
2020-08-11 18:11 ` [musl] [PATCH 3/3] setjmp: optimize x86 longjmp epilogues Alexander Monakov
2020-08-11 18:45 ` Rich Felker [this message]
2020-08-12 13:29   ` [musl] [PATCH 1/3] setjmp: fix x86-64 longjmp argument adjustment Alexander Monakov
2020-08-12 21:44     ` Szabolcs Nagy
2020-08-12 22:13       ` Szabolcs Nagy
2020-08-13  8:01     ` Jens Gustedt
2020-08-13  8:53       ` Alexander Monakov
2020-08-11 23:10 ` [musl] [PATCH] setjmp: optimize x86-64 longjmp prologues Alexander Monakov
2020-08-12 11:34   ` [musl] [PATCH 4/3] setjmp: optimize " Alexander Monakov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200811184527.GM3265@brightrain.aerifal.cx \
    --to=dalias@libc.org \
    --cc=musl@lists.openwall.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).