DTrace fixes for node.js
Fedor Indutny
fedor at indutny.com
Tue Dec 2 00:32:10 UTC 2014
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 <markj at freebsd.org> 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 <markj at freebsd.org>
> 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 <fedor at indutny.com>
> > > wrote:
> > > >
> > > > > Update sent, thank you!
> > > > >
> > > > > On Thu, Feb 27, 2014 at 6:39 PM, Mark Johnston <markj at freebsd.org>
> > > wrote:
> > > > > > On Thu, Feb 27, 2014 at 3:30 AM, Fedor Indutny <
> fedor at 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 at 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
> > > > > >>>
> > > > >
> > >
>
More information about the freebsd-dtrace
mailing list