From owner-freebsd-dtrace@FreeBSD.ORG Tue Dec 2 00:32:09 2014 Return-Path: Delivered-To: freebsd-dtrace@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id D6478317; Tue, 2 Dec 2014 00:32:09 +0000 (UTC) Received: from mail-wi0-x22c.google.com (mail-wi0-x22c.google.com [IPv6:2a00:1450:400c:c05::22c]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 5677AB0; Tue, 2 Dec 2014 00:32:09 +0000 (UTC) Received: by mail-wi0-f172.google.com with SMTP id n3so26463447wiv.5 for ; Mon, 01 Dec 2014 16:32:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc:content-type; bh=LqpY0wFUNTyMQxypVABXILg7zSNgTEcqVFY2mSuUYiE=; b=mGcmRs7gr7sKF1OFvuJkroYbV+G5tlAXTBFGiWETv/CdTqGOhmb4ZmYv6pNi92i6Ib lRzgPPm+LnnqS5bIZz5YzXQ4WC+eAKZxpK+WrebTPO7qQO7zGeS2q5+W+u0aXEOKyxr3 JqAge3s2RpYSjcg7GALvUYS58kmYfRZa2dCxNcUXv5V+x0czLSIdSCuBS0R3jTjVrIfC rWhcpgHoBz6eLAAqcJPgg8NSHHYV4dhKedjxIVbYA9/3i3esnt87lz6qe4u+w3ZtiD6Z GLVQDUYh+z/xDyVhmrN8jexJJQZ3dF5DsJoCHM+xqmqbgNqTeXn8uNyCAHN+44ZEoWZZ G9ug== X-Received: by 10.194.83.8 with SMTP id m8mr99402489wjy.58.1417480327734; Mon, 01 Dec 2014 16:32:07 -0800 (PST) MIME-Version: 1.0 Sender: fedor.indutny@gmail.com Received: by 10.27.48.2 with HTTP; Mon, 1 Dec 2014 16:31:47 -0800 (PST) In-Reply-To: <20141202001342.GH48232@charmander.picturesperfect.net> References: <20140227050136.GB28089@raichu> <20141201215430.GD48232@charmander.picturesperfect.net> <20141202001342.GH48232@charmander.picturesperfect.net> From: Fedor Indutny Date: Tue, 2 Dec 2014 04:31:47 +0400 X-Google-Sender-Auth: gt3Qtznkh2QAvHS23m6f5tNDU8s Message-ID: Subject: Re: DTrace fixes for node.js To: Mark Johnston Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.18-1 Cc: "freebsd-dtrace@freebsd.org" X-BeenThere: freebsd-dtrace@freebsd.org X-Mailman-Version: 2.1.18-1 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: Tue, 02 Dec 2014 00:32:10 -0000 Mark, I just realised that 10.1 drti is indeed looking for PROGBITS. You are totally right! Please ignore this particular part of change then, as it has not much to do with the real problem. However, alignment problems could still break the drti loader. I just built latest drti on my FreeBSD instance and expected it to work, and then found out it is not looking for PROGBITS. :) Thank you, Fedor. On Tue, Dec 2, 2014 at 3:13 AM, Mark Johnston wrote: > On Tue, Dec 02, 2014 at 03:02:19AM +0400, Fedor Indutny wrote: > > Mark, > > > > Yes, 271413 is the one fixing PROGBITS. Anyway, my patch should > > support both PROGBITS and DOFW_sun, making it quite universal. > > But why is it necessary? As of r271413, the DOF section type should > always be SUNW_dof. If not, that's unexpected, and I'd like to understand > what's going on. > > On 10.1, we're still creating a section of type PROGBITS, and that's > what drti.o is looking for. Is there a regression? > > Thanks, > -Mark > > > > > Thank you for your time! > > > > Fedor. > > > > On Tue, Dec 2, 2014 at 12:54 AM, Mark Johnston > wrote: > > > > > On Mon, Dec 01, 2014 at 04:59:02PM +0400, Fedor Indutny wrote: > > > > Hello Mark, DTrace people! > > > > > > > > > > Hi Fedor, > > > > > > > Hope you are doing well. > > > > > > > > Looks like FreeBSD 10.1 decided to play some not that funny games > > > > with drti.c again :) > > > > > > > > I have submitted a [patch][0] as a problem report. > > > > > > Thank you. I'll test and commit it if I don't run into any problems. > > > > > > > > > > > It fixes iteration over aligned DOFs, and accounts for a > > > > non-master-branch FreeBSD stuff in: > > > > > > > > cddl/contrib/opensolaris/lib/libdtrace/common/dt_link.c > > > > > > > > Which is currently producing SHT_PROGBITS section instead of > > > > expected SHT_SUNW_dof, thus confusing the drti.c file. > > > > > > Hm, are you referring to this change: > > > https://svnweb.freebsd.org/base?view=revision&revision=271413 > > > ? > > > > > > I'm a bit confused, as it hasn't been merged to 10 yet: it exposed > > > another dtrace bug that caused build failures in some USDT-enabled > > > ports, so I'm waiting for the subsequent fixes to settle. > > > > > > -Mark > > > > > > > > > > > Thank you, > > > > Fedor. > > > > > > > > [0]: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=195555 > > > > > > > > > > > > On Thu, Feb 27, 2014 at 6:50 PM, Fedor Indutny > > > wrote: > > > > > > > > > Update sent, thank you! > > > > > > > > > > On Thu, Feb 27, 2014 at 6:39 PM, Mark Johnston > > > wrote: > > > > > > On Thu, Feb 27, 2014 at 3:30 AM, Fedor Indutny < > fedor@indutny.com> > > > > > wrote: > > > > > >> Mark, > > > > > >> > > > > > >> Thanks for looking into this. I just tried your patch and it (no > > > > > >> surprise) builds fine. Node.js DOF symbols seems to be resolving > > > > > >> properly too! > > > > > >> > > > > > >> Do you want me to squash this changes into my patch, and post > them > > > on > > > > > >> that ticket? > > > > > >> > > > > > > > > > > > > That would be good, thanks. When I have some time I'll do more > > > testing > > > > > > and commit the change. > > > > > > > > > > > > -Mark > > > > > > > > > > > >> On Thu, Feb 27, 2014 at 9:01 AM, Mark Johnston < > markj@freebsd.org> > > > > > wrote: > > > > > >>> On Tue, Feb 25, 2014 at 06:16:15PM +0400, Fedor Indutny wrote: > > > > > >>>> Hello devs! > > > > > >>>> > > > > > >>>> I have made some fixes to fix DTrace support for node.js in > > > FreeBSD: > > > > > >>>> > > > > > >>>> * http://www.freebsd.org/cgi/query-pr.cgi?pr=186821 > > > > > >>>> * http://www.freebsd.org/cgi/query-pr.cgi?pr=187027 > > > > > >>>> > > > > > >>>> Here is a blog post with a bit of explanation of why this is > > > needed > > > > > >>>> and what is fixed: https://blog.indutny.com/7.freebsd-dtrace > > > > > >>>> > > > > > >>>> Please let me know if I could be any help in reviewing it. > > > > > >>> > > > > > >>> Hi Fedor, > > > > > >>> > > > > > >>> The DOF limit change looks fine to me. I note that the illumos > guys > > > > > have > > > > > >>> just pushed a change to illumos-gate which bumps > > > dtrace_dof_maxsize, > > > > > but > > > > > >>> it's good to have the sysctl as well. > > > > > >>> > > > > > >>> The drti change looks mostly good to me. The real problem > there is > > > that > > > > > >>> our linker doesn't know how to merge DOF, so it just > concatenates > > > the > > > > > >>> tables into one SUNW_dof section. So we should really fix our > > > linker, > > > > > >>> but it doesn't hurt to also handle the problem in drti.o. > > > > > >>> > > > > > >>> There are a couple of bugs in the patch. First, the "break" > added > > > after > > > > > >>> finding the DOF section causes problems if we haven't yet seen > the > > > > > >>> symbol table. Second, fixedprobes needs to be reset at the > > > beginning of > > > > > >>> each iteration of the while loop that you added, else we may > not > > > try > > > > > >>> searching the dynamic symbol table when fixing the probe > addresses. > > > > > I've > > > > > >>> pasted a patch below; could you test it and make sure things > still > > > work > > > > > >>> properly with node? > > > > > >>> > > > > > >>> Thanks for the detailed blog post and problem description, they > > > were > > > > > >>> very helpful. :) > > > > > >>> > > > > > >>> -Mark > > > > > >>> > > > > > >>> diff --git > a/cddl/contrib/opensolaris/lib/libdtrace/common/drti.c > > > > > b/cddl/contrib/opensolaris/lib/libdtrace/common/drti.c > > > > > >>> index e47cfb4d..bb02d8c 100644 > > > > > >>> --- a/cddl/contrib/opensolaris/lib/libdtrace/common/drti.c > > > > > >>> +++ b/cddl/contrib/opensolaris/lib/libdtrace/common/drti.c > > > > > >>> @@ -162,7 +162,7 @@ dtrace_dof_init(void) > > > > > >>> char *dofstrtabraw; > > > > > >>> size_t shstridx, symtabidx = 0, dynsymidx = 0; > > > > > >>> unsigned char *buf; > > > > > >>> - int fixedprobes = 0; > > > > > >>> + int fixedprobes; > > > > > >>> #endif > > > > > >>> > > > > > >>> if (getenv("DTRACE_DOF_INIT_DISABLE") != NULL) > > > > > >>> @@ -214,7 +214,6 @@ dtrace_dof_init(void) > > > > > >>> if (s && strcmp(s, ".SUNW_dof") == 0) > { > > > > > >>> dofdata = elf_getdata(scn, > NULL); > > > > > >>> dof = dofdata->d_buf; > > > > > >>> - break; > > > > > >>> } > > > > > >>> } > > > > > >>> } > > > > > >>> @@ -226,6 +225,7 @@ dtrace_dof_init(void) > > > > > >>> } > > > > > >>> > > > > > >>> while ((char *) dof < (char *) dofdata->d_buf + > > > > > dofdata->d_size) { > > > > > >>> + fixedprobes = 0; > > > > > >>> dof_next = (void *) ((char *) dof + > > > dof->dofh_filesz); > > > > > >>> #endif > > > > > >>> > > > > > > > > >