public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "hjl.tools at gmail dot com" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug target/102952] New code-gen options for retpolines and straight line speculation
Date: Thu, 28 Oct 2021 22:26:13 +0000 [thread overview]
Message-ID: <bug-102952-4-734gwYlC6h@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-102952-4@http.gcc.gnu.org/bugzilla/>
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102952
H.J. Lu <hjl.tools at gmail dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|WAITING |NEW
--- Comment #23 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to Andrew Cooper from comment #22)
> One curious thing I have discovered. While auditing the -mharden-sls=all
> code generation in Xen, I found examples where I got "ret int3 ret int3"
> with no intervening instructions.
>
> It turns out this is not a regression in this change. It is a pre-existing
> missing optimisation, which is made more obvious when every ret is extended
> with an int3.
>
> It occurs for functions with either no stack frame at all, or functions
> which have an early exit before setting up the stack frame. Some examples
> which occur at -O1 do not occur at -O2.
>
> One curious example which does still repro at -O2 is this. We have a hash
> lookup function:
>
> struct context *sidtab_search(struct sidtab *s, u32 sid)
> {
> int hvalue;
> struct sidtab_node *cur;
>
> if ( !s )
> return NULL;
>
> hvalue = SIDTAB_HASH(sid);
> cur = s->htable[hvalue];
> while ( cur != NULL && sid > cur->sid )
> cur = cur->next;
>
> if ( cur == NULL || sid != cur->sid )
> {
> /* Remap invalid SIDs to the unlabeled SID. */
> sid = SECINITSID_UNLABELED;
> hvalue = SIDTAB_HASH(sid);
> cur = s->htable[hvalue];
> while ( cur != NULL && sid > cur->sid )
> cur = cur->next;
> if ( !cur || sid != cur->sid )
> return NULL;
> }
>
> return &cur->context;
> }
>
> which compiles (reformatted a little for width - unmodified:
> https://paste.debian.net/hidden/7bf675d6/) to:
>
> <sidtab_search>:
> 48 85 ff test %rdi,%rdi
> /------- 74 63 je <sidtab_search+0x68>
> | 48 8b 17 mov (%rdi),%rdx
> | 89 f0 mov %esi,%eax
> | 83 e0 7f and $0x7f,%eax
> | 48 8b 04 c2 mov (%rdx,%rax,8),%rax
> | 48 85 c0 test %rax,%rax
> | /--- 75 13 jne <sidtab_search+0x29>
> | /|--- eb 17 jmp <sidtab_search+0x2f>
> | || 0f 1f 84 00 00 00 00 nopl 0x0(%rax,%rax,1)
> | || 00
> | ||/-> 48 8b 40 48 mov 0x48(%rax),%rax
> | ||| 48 85 c0 test %rax,%rax
> | +||-- 74 06 je <sidtab_search+0x2f>
> | |\|-> 39 30 cmp %esi,(%rax)
> | | \-- 72 f3 jb <sidtab_search+0x20>
> | /|---- 74 24 je <sidtab_search+0x53>
> | |\---> 48 8b 42 28 mov 0x28(%rdx),%rax
> | | 48 85 c0 test %rax,%rax
> | | /--- 75 11 jne <sidtab_search+0x49>
> |/|-|--- eb 32 jmp <sidtab_search+0x6c> // (1)
> ||| | 66 0f 1f 44 00 00 nopw 0x0(%rax,%rax,1)
> ||| |/-> 48 8b 40 48 mov 0x48(%rax),%rax
> ||| || 48 85 c0 test %rax,%rax
> |||/||-- 74 17 je <sidtab_search+0x60> // (2)
> ||||\|-> 83 38 04 cmpl $0x4,(%rax)
> |||| \-- 76 f2 jbe <sidtab_search+0x40>
> |||| 83 38 05 cmpl $0x5,(%rax)
> +|||---- 75 15 jne <sidtab_search+0x68>
> ||\|---> 48 83 c0 08 add $0x8,%rax
> || | c3 retq
> || | cc int3
> || | 0f 1f 80 00 00 00 00 nopl 0x0(%rax)
> || \---> c3 retq // Target of (2)
> || cc int3
> || 66 0f 1f 44 00 00 nopw 0x0(%rax,%rax,1)
> \|-----> 31 c0 xor %eax,%eax
> | c3 retq
> | cc int3
> \-----> c3 retq // Target of (1)
> cc int3
> 66 90 xchg %ax,%ax
>
> There are 4 exits in total. Two have to set up %eax, so they can't usefully
> be merged.
>
> However, the unconditional jmp at (1) is 2 bytes, and could fully contain
> its target ret;int3 without even impacting the surrounding padding. Whether
> it inlines or merges, this drops 4 bytes.
>
> The conditional jump at (2) could be folded in to any of the other exit
> paths, dropping 16 bytes from the total size size.
>
> I have no idea how easy/hard this may be to track down, or whether it is
> worth pursuing urgently, but it probably does want looking at, seeing as SLS
> hardening doubles the hit.
Please open a separate bug to track it. Should shrink-wrap handle it?
next prev parent reply other threads:[~2021-10-28 22:26 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-26 15:54 [Bug c/102952] New: " andrew.cooper3 at citrix dot com
2021-10-26 15:56 ` [Bug c/102952] " andrew.cooper3 at citrix dot com
2021-10-26 17:11 ` [Bug target/102952] " andrew.cooper3 at citrix dot com
2021-10-27 7:00 ` rguenth at gcc dot gnu.org
2021-10-27 14:50 ` hjl.tools at gmail dot com
2021-10-27 15:01 ` hjl.tools at gmail dot com
2021-10-27 15:03 ` hjl.tools at gmail dot com
2021-10-27 17:55 ` hjl.tools at gmail dot com
2021-10-27 19:59 ` peterz at infradead dot org
2021-10-27 20:00 ` hjl.tools at gmail dot com
2021-10-27 20:14 ` peterz at infradead dot org
2021-10-27 20:20 ` peterz at infradead dot org
2021-10-27 21:42 ` hjl.tools at gmail dot com
2021-10-27 21:42 ` hjl.tools at gmail dot com
2021-10-27 22:07 ` peterz at infradead dot org
2021-10-27 22:12 ` hjl.tools at gmail dot com
2021-10-27 22:16 ` andrew.cooper3 at citrix dot com
2021-10-27 22:39 ` hjl.tools at gmail dot com
2021-10-27 22:42 ` hjl.tools at gmail dot com
2021-10-27 22:46 ` andrew.cooper3 at citrix dot com
2021-10-27 22:53 ` hjl.tools at gmail dot com
2021-10-28 7:30 ` peterz at infradead dot org
2021-10-28 7:43 ` peterz at infradead dot org
2021-10-28 22:07 ` andrew.cooper3 at citrix dot com
2021-10-28 22:26 ` hjl.tools at gmail dot com [this message]
2021-11-15 14:27 ` hjl.tools at gmail dot com
2021-11-16 12:57 ` peterz at infradead dot org
2021-11-16 18:52 ` hjl.tools at gmail dot com
2021-11-17 21:35 ` cvs-commit at gcc dot gnu.org
2021-11-18 16:26 ` cvs-commit at gcc dot gnu.org
2021-11-18 18:30 ` hjl.tools at gmail dot com
2022-01-06 9:51 ` andrew.cooper3 at citrix dot com
2022-01-06 13:21 ` hjl.tools at gmail dot com
2022-01-06 13:23 ` hjl.tools at gmail dot com
2022-01-06 18:13 ` andrew.cooper3 at citrix dot com
2022-01-06 19:53 ` cvs-commit at gcc dot gnu.org
2022-01-06 20:12 ` hjl.tools at gmail dot com
2022-01-31 8:04 ` rguenth at gcc dot gnu.org
2022-01-31 13:34 ` hjl.tools at gmail dot com
2022-01-31 15:43 ` hjl.tools at gmail dot com
2022-01-31 18:56 ` hjl.tools at gmail dot com
2022-02-07 23:06 ` andrew.cooper3 at citrix dot com
2022-02-16 14:01 ` cvs-commit at gcc dot gnu.org
2022-02-16 14:01 ` cvs-commit at gcc dot gnu.org
2022-02-16 14:01 ` cvs-commit at gcc dot gnu.org
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=bug-102952-4-734gwYlC6h@http.gcc.gnu.org/bugzilla/ \
--to=gcc-bugzilla@gcc.gnu.org \
--cc=gcc-bugs@gcc.gnu.org \
/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.
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 read-only IMAP folder(s) and NNTP newsgroup(s).