From owner-freebsd-dtrace@FreeBSD.ORG Mon Oct 28 05:04:07 2013 Return-Path: Delivered-To: dtrace@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id 68455DC1 for ; Mon, 28 Oct 2013 05:04:07 +0000 (UTC) (envelope-from adam.leventhal@delphix.com) Received: from mail-ob0-x236.google.com (mail-ob0-x236.google.com [IPv6:2607:f8b0:4003:c01::236]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 317862198 for ; Mon, 28 Oct 2013 05:04:06 +0000 (UTC) Received: by mail-ob0-f182.google.com with SMTP id wn1so3149972obc.13 for ; Sun, 27 Oct 2013 22:04:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=delphix.com; s=google; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=JwuMFjOV3deMWreGeTXl+2oMm9/OioS5WaX9HO04PIs=; b=WBdUd/W3PKJK/xU617CermQpMo5AfwNLB5xK8Xfb40VYrlat7J+Iri/L4Ez0vFM4Xr 85MSgd8iYA/NAdvYB8vVreJypR5JQXGHDIMcfBx6qAS5rPkfHEBpETZbE/OwTVXH+xqB 7jlOYz28b1CItE1VKYkxrCjTXPBrhgoAKaw1U= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=JwuMFjOV3deMWreGeTXl+2oMm9/OioS5WaX9HO04PIs=; b=aWESX12F6avpaLWKvyP3prP3V8T9XrAgCw8D4fZY11D7l/pOtq/jm35GCxjdBuVWCE aFgwMywVcjFwnU4EJOLYIh/ld8W8VxRxwoSz6oQh+fPXByMdARyHZZ6MJqhz7WFNk1b/ Gnsw3MvRm1bTMnbZmcETtsWXIyjfcdddzJ5bPBtbp++YNQAjIkiOY4YaAMVsoQuyLHmI /tBFysJRbrva4aKoqGSlUZVAVwGuCl+d0dcAeIlasdjxdc1BnJSyc6cVMAaAxT4WLIV3 6/thouRHOtf6szigKwO3EtoHb5tt9VKB9lWqPy6UzOBJ/xqURRus4M+WxjV8788gRFBn V/tQ== X-Gm-Message-State: ALoCoQkM+bXDAt+iabDgh15O7lOVbmG034tL0PeWhHdOlaf/etfwLlLvEfQt2ypbyvQa9PaVZT9Y MIME-Version: 1.0 X-Received: by 10.182.121.137 with SMTP id lk9mr13574290obb.32.1382936646257; Sun, 27 Oct 2013 22:04:06 -0700 (PDT) Received: by 10.76.92.67 with HTTP; Sun, 27 Oct 2013 22:04:06 -0700 (PDT) In-Reply-To: <20131024025902.GA2286@charmander> References: <20131023203009.GA92945@lemon> <20131024025902.GA2286@charmander> Date: Sun, 27 Oct 2013 22:04:06 -0700 Message-ID: Subject: Re: Firefox crash during dtrace attach under -CURRENT From: Adam Leventhal To: Mark Johnston Content-Type: text/plain; charset=ISO-8859-1 Cc: dtrace@freebsd.org X-BeenThere: freebsd-dtrace@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "A discussion list for developers working on DTrace in FreeBSD." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Oct 2013 05:04:07 -0000 Hey Mark, For reference, here's the relevant code from illumos: #ifdef __amd64 if (p->p_model == DATAMODEL_NATIVE) { #endif addr = rp->r_sp - sizeof (uintptr_t); ret = fasttrap_sulword((void *)addr, rp->r_fp); #ifdef __amd64 } else { addr = rp->r_sp - sizeof (uint32_t); ret = fasttrap_suword32((void *)addr, (uint32_t)rp->r_fp); } #endif > For anyone interested, the bug is that fasttrap's ebp push instruction > emulation code is just wrong: it's supposed to save %rbp at %rsp - 8. > But instead it tries to save %rsp at %rsp - 8, and also reverses the > uaddr/kaddr arguments to copyout(), resulting in strange crashes. I > managed to narrow in on the problem with a test program that prints %rbp > immediately before and after a tracepoint. Perhaps rp->r_fp was mistakenly swapped for &rp->r_rsp. > Can anyone review this diff? I'd like to check it in soon, assuming > that I haven't also made a mistake somewhere. :) > diff --git a/sys/cddl/contrib/opensolaris/uts/intel/dtrace/fasttrap_isa.c b/sys/cddl/contrib/opensolaris/uts/intel/dtrace/fasttrap_isa.c > index 8b5ce9f..bb5c9af 100644 > --- a/sys/cddl/contrib/opensolaris/uts/intel/dtrace/fasttrap_isa.c > +++ b/sys/cddl/contrib/opensolaris/uts/intel/dtrace/fasttrap_isa.c > @@ -1399,12 +1399,12 @@ fasttrap_pid_probe(struct reg *rp) > #ifdef __amd64 > if (p->p_model == DATAMODEL_NATIVE) { > addr = rp->r_rsp - sizeof (uintptr_t); > - ret = fasttrap_sulword((void *)addr, &rp->r_rsp); > + ret = fasttrap_sulword(&rp->r_rbp, (void *)addr); > } else { > #endif > #ifdef __i386__ > addr = rp->r_rsp - sizeof (uint32_t); > - ret = fasttrap_suword32((void *)addr, &rp->r_rsp); > + ret = fasttrap_suword32(&rp->r_rbp, (void *)addr); > #endif > #ifdef __amd64 > } This looks correct, but I'd suggest you might want to instead change the code in fasttrap_isa.c to look more like the illumos version, and change the macro definition: -#define fasttrap_suword64(_k, _u) copyout((_k), (_u), sizeof(uint64_t)) +#define fasttrap_suword64(_u, _k) copyout((_k), &(_u), sizeof(uint64_t)) Adam -- Adam Leventhal CTO, Delphix http://blog.delphix.com/ahl