Date: Mon, 24 Nov 2014 23:12:12 -0800 From: Mark Johnston <markj@freebsd.org> To: Shrikanth Kamath <shrikanth07@gmail.com> Cc: Rui Paulo <rpaulo@me.com>, freebsd-hackers@freebsd.org, avg@freebsd.org, Konstantin Belousov <kostikbel@gmail.com>, freebsd-dtrace@freebsd.org Subject: Re: DTrace: stack() does not print kernel module functions for i386 Message-ID: <20141125071212.GA2005@raichu> In-Reply-To: <CAEOAkMUK_81u0ChOr9H3oRiEhoJuK3LNn8f7dBszYrTkf5YxTw@mail.gmail.com> References: <CAEOAkMXnwqC42gZKc0f80cppff077pYGjs5PUPht0DBcyEi8Jw@mail.gmail.com> <20141109093632.GV53947@kib.kiev.ua> <9011F920-3092-4E61-9CDC-68FD9092BB7D@me.com> <201411131336.12334.jhb@freebsd.org> <AE2A9617-8E59-4F44-9C1F-8344EF5B05C7@me.com> <20141123021856.GA54708@raichu> <CAEOAkMUK_81u0ChOr9H3oRiEhoJuK3LNn8f7dBszYrTkf5YxTw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Nov 24, 2014 at 06:36:26PM -0800, Shrikanth Kamath wrote: > > The suggested patch doesn't seem quite right; there are other functions > > in dt_module.c with the same assignment (i.e. > > "is_elf_obj = ehdr.e_type == ET_REL"), but the same modification is not > > correct in all cases - fixing it everywhere breaks stack() again - and > > "is_elf_obj" seems like the wrong name if DSOs are counted as well. > > > > The root of the problem is that dmp->dm_*_va offsets don't have the kld > > load address taken into account on i386, since they're currently set based > > only on the ELF section addresses. This is handled by > > dmp->dm_reloc_offset for symbols, but that's a separate case. > > > > When is_elf_obj is true, we include the load address when setting the > > dmp->dm_*_va fields. I suggest we do that unconditionally, and only set > > elements of dmp->dm_sec_offsets if is_elf_obj is true. This fixes the > > bug for me on i386. Any opinions? > > > > Thanks, > > -Mark > > > > diff --git a/cddl/contrib/opensolaris/lib/libdtrace/common/dt_module.c b/cddl/contrib/opensolaris/lib/libdtrace/common/dt_module.c > > index e3905c1..9dd52b5 100644 > > --- a/cddl/contrib/opensolaris/lib/libdtrace/common/dt_module.c > > +++ b/cddl/contrib/opensolaris/lib/libdtrace/common/dt_module.c > > @@ -1211,13 +1211,13 @@ dt_module_update(dtrace_hdl_t *dtp, struct kld_file_stat *k_stat) > > #if defined(__FreeBSD__) > > if (sh.sh_size == 0) > > continue; > > - if (is_elf_obj && (sh.sh_type == SHT_PROGBITS || > > - sh.sh_type == SHT_NOBITS)) { > > + if (sh.sh_type == SHT_PROGBITS || sh.sh_type == SHT_NOBITS) { > > alignmask = sh.sh_addralign - 1; > > mapbase += alignmask; > > mapbase &= ~alignmask; > > sh.sh_addr = mapbase; > > - dmp->dm_sec_offsets[elf_ndxscn(sp)] = sh.sh_addr; > > + if (is_elf_obj) > > + dmp->dm_sec_offsets[elf_ndxscn(sp)] = sh.sh_addr; > > mapbase += sh.sh_size; > > } > > #endif > > Hello Mark, > I tested the patch and works good, did some basic testing. Marcel > too pointed to the other occurrence of is_elf_obj in the file and that > it should not include ET_DYN which is when I realized it was not the > right thing to do, i was trying to read up more on this, guess this > patch takes care of it. Thanks for fixing this. I've committed it as r275011. Thanks for the report.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20141125071212.GA2005>