From patchwork Mon Dec 12 05:29:09 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Keno Fischer X-Patchwork-Id: 18380 Received: (qmail 117880 invoked by alias); 12 Dec 2016 05:31:14 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 116911 invoked by uid 89); 12 Dec 2016 05:29:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy=unwinders, unwinder, quest, imagination X-HELO: mail-qt0-f181.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:mime-version :content-disposition:user-agent; bh=e4szuVJsL2Hfs5C/6O8253OvTL7VJFeCWwvXJmPWSmQ=; b=OJRVJE7KxqUXVEBrpfukq5AsCXM7QnXHVTNwK48K6ODO9tyeBdVBWpeNknPypLp8bU oGO4wdZnkcxnVYqEPK/rXVg12hw8YxhbZh4GasEIDgHM5EgNYbIe8wZSCTg+S74rJ6CB rUeAnGwsOFAZZKIkxojkw45HjDiXpFQWUCT6joFHGABeLIGJhNGbBr+sTMOSicwtETOZ tosgqSVi9gN08Rr9HWziae2SnMwXg54ALF0mG3a3X6IMtq9fb4D3qlCzSGHrLwzLbriw SuSg3NnSF9ZnlqKQBvFpitGXh9ZgbjiRJty88GPTRHFUHXr2lNsAy+tUZNJ2XE/NGkGL RdTA== X-Gm-Message-State: AKaTC00mTuICey1P9tJv5R6SKqdSWwsW4uKHLhbLlKxg5YyTdoQMEqPHqEPB7yWLQg2D7ExE X-Received: by 10.200.58.199 with SMTP id x65mr77757164qte.152.1481520552042; Sun, 11 Dec 2016 21:29:12 -0800 (PST) Date: Mon, 12 Dec 2016 00:29:09 -0500 From: Keno Fischer To: libc-alpha@sourceware.org Cc: hjl.tools@gmail.com Subject: [PATCH] x86_64: Expand CFI to cover clone after the syscall Message-ID: <20161212052909.GA11750@juliacomputing.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.6.1 (2016-04-27) The CFI used to terminate after the syscall, leaving unwinding for the rest of the function up to the debugger's imagination. The comment states that the reason for this is that the unwind info would be incorrect in the child. However, without unwind info, both the child and the parent process may fail to unwind properly after the syscall (though some debuggers still have heuristics that work for the parent case). To improve this situation, use a DWARF impression that checks %rax, and if non-zero (i.e. we're in the parent, behaves like the CFI for the rest of the function). If %rax==0, the CFI indicates to unwind to thread_start. Ideally, I would have liked to have it undefine %rip, but it does not look like that is possible. However, even if not entirely correct, I think this version is a strict improvement over what was there before. E.g. in GDB: Backtrace before: #0 0x00007f14a1119081 in clone () from /lib/x86_64-linux-gnu/libc.so.6 #1 0x00007f14a13df640 in ?? () from /lib/x86_64-linux-gnu/libpthread.so.0 #2 0x00007f14a0e0c700 in ?? () #3 0x0000000000000000 in ?? () Backtrace after: #0 clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:105 #1 0x00007f377330485b in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:120 Another option would be to create another global symbol e.g. `new_thread_from_clone` and have the unwind pretend to unwind there. --- I'm on a somewhat long-term quest here to improve the accuracy of unwind info in the basic libraries and the toolchain. Right now we're in a situation where e.g. invalid memory accesses by unwinders are often ignored, because there are sufficiently many places without unwind info, or with incorrect unwind info that it's more likely that than an unwinder bug. I'd like to get into a position I can consider an invalid memory access during unwinding a bug, but first, I need to cleanup some of the more common cases where one runs into invalid unwind info. sysdeps/unix/sysv/linux/x86_64/clone.S | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/sysdeps/unix/sysv/linux/x86_64/clone.S b/sysdeps/unix/sysv/linux/x86_64/clone.S index 66f4b11..1bd2eed 100644 --- a/sysdeps/unix/sysv/linux/x86_64/clone.S +++ b/sysdeps/unix/sysv/linux/x86_64/clone.S @@ -72,16 +72,38 @@ ENTRY (__clone) mov 8(%rsp), %R10_LP movl $SYS_ify(clone),%eax - /* End FDE now, because in the child the unwind info will be - wrong. */ - cfi_endproc; syscall + /* Best effort unwind info that works for both the parent and the child. + Ideally, we'd have cfi_undefiend(%rip) in the child and keep everything + the same in the parent, but we can't do that (since there's no way for + an expression to return undefined). Instead, we pretend that the child + came from thread_start. This isn't quite correct, but at least better than + whatever the debugger heuristics would have come up with in the absence + of unwind info */ + + /* Encodes: parent %rip = %rax == 0 ? %rip + (2f - .) : *%rsp + DW_CFA_val_expression %rip DW_OP_breg16(2f-1b) DW_OP_lit0 DW_OP_breg0(0) + DW_OP_eq DW_OP_bra(0x0003) DW_OP_breg7(0) DW_OP_deref */ +#define CFI_CHILD_RIP_IS_THREAD_START \ + .cfi_escape 0x16, 0x10, 0xc, 0x80, 2f-., 0x30, 0x70, 0x0, 0x29,\ + 0x28, 0x03, 0x0, 0x77, 0x0, 0x6; + + /* encode parent %rsp = %rsp + (%rax != 0 ? 8 : 0) + DW_CFA_val_expression %rsp DW_OP_breg7(0) DW_OP_lit0 DW_OP_breg0(0) + DW_OP_ne DW_OP_lit3 DW_OP_shl DW_OP_plus*/ + .cfi_escape 0x16, 0x7, 0x9, 0x77, 0x0, 0x30, 0x70, 0x00, 0x2e, 0x33, + 0x24, 0x22; + + CFI_CHILD_RIP_IS_THREAD_START testq %rax,%rax + CFI_CHILD_RIP_IS_THREAD_START jl SYSCALL_ERROR_LABEL + CFI_CHILD_RIP_IS_THREAD_START jz L(thread_start) ret + cfi_endproc; L(thread_start): cfi_startproc; @@ -90,7 +112,7 @@ L(thread_start): /* Clear the frame pointer. The ABI suggests this be done, to mark the outermost frame obviously. */ xorl %ebp, %ebp - +2: andq $CLONE_VM, %rdi jne 1f movl $SYS_ify(getpid), %eax