Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 22 Nov 2014 18:18:56 -0800
From:      Mark Johnston <markj@FreeBSD.org>
To:        Rui Paulo <rpaulo@me.com>
Cc:        Shrikanth Kamath <shrikanth07@gmail.com>, John Baldwin <jhb@freebsd.org>, 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:  <20141123021856.GA54708@raichu>
In-Reply-To: <AE2A9617-8E59-4F44-9C1F-8344EF5B05C7@me.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>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Nov 13, 2014 at 07:49:27PM -0800, Rui Paulo wrote:
> On Nov 13, 2014, at 10:36, John Baldwin <jhb@freebsd.org> wrote:
> > Why have the #ifdef?  In theory other platforms besides amd64 could use
> > sys/kern/link_elf_obj.c.  It doesn't hurt to just let the code always accept
> > both ET_DYN and ET_REL does it?
> 
> No, it doesn't hurt.

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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20141123021856.GA54708>