From owner-freebsd-dtrace@FreeBSD.ORG Sun Nov 23 02:19:02 2014 Return-Path: Delivered-To: freebsd-dtrace@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 39CC040B; Sun, 23 Nov 2014 02:19:02 +0000 (UTC) Received: from mail-pd0-x22a.google.com (mail-pd0-x22a.google.com [IPv6:2607:f8b0:400e:c02::22a]) (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 02A1E2E1; Sun, 23 Nov 2014 02:19:02 +0000 (UTC) Received: by mail-pd0-f170.google.com with SMTP id fp1so7644240pdb.15 for ; Sat, 22 Nov 2014 18:19:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=WaThMR+dukbH29fNT1r4EJZvXYUHoFv/hjprvhgCeyM=; b=Q+itBFCOPFhGGTnQiqg/S/sNY1LWyRJBXPLqz4kWk+8emewA4XdejjdbYwBpQMbuny HRjMbJrTNdgIdZjV4k/ZUs2X830pj8ZvePi1FE0DZeELP7cBZzHQ7eVe6G7GHHfXhA3L g42b4X+q7EKNYob/mUyVuon4Iqc4JGv6fBBhokuEv0ltmRlDggVkUpnEx6vRrUb6dUdW GvAH3UfHpiRI7QfEHDGR81Gj06rBqR4OJ/45+cR2SC3gJL4tLcmGiVb10w5H+7Q0Yl66 Lg7OALzlUx6NJdx9PFxu0fK2+2RqoyenuKs913QupGOPDfwCc7QFhM3BAx7+EpfrAlhk hWbA== X-Received: by 10.69.16.99 with SMTP id fv3mr20258545pbd.43.1416709141581; Sat, 22 Nov 2014 18:19:01 -0800 (PST) Received: from raichu ([198.244.104.6]) by mx.google.com with ESMTPSA id oq6sm8557860pdb.45.2014.11.22.18.19.00 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 22 Nov 2014 18:19:00 -0800 (PST) Sender: Mark Johnston Date: Sat, 22 Nov 2014 18:18:56 -0800 From: Mark Johnston To: Rui Paulo Subject: Re: DTrace: stack() does not print kernel module functions for i386 Message-ID: <20141123021856.GA54708@raichu> References: <20141109093632.GV53947@kib.kiev.ua> <9011F920-3092-4E61-9CDC-68FD9092BB7D@me.com> <201411131336.12334.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Cc: Shrikanth Kamath , John Baldwin , freebsd-hackers@freebsd.org, avg@freebsd.org, Konstantin Belousov , 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: Sun, 23 Nov 2014 02:19:02 -0000 On Thu, Nov 13, 2014 at 07:49:27PM -0800, Rui Paulo wrote: > On Nov 13, 2014, at 10:36, John Baldwin 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 From owner-freebsd-dtrace@FreeBSD.ORG Mon Nov 24 21:22:06 2014 Return-Path: Delivered-To: freebsd-dtrace@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 4C5492DE; Mon, 24 Nov 2014 21:22:06 +0000 (UTC) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id E97263D7; Mon, 24 Nov 2014 21:22:04 +0000 (UTC) Received: from porto.starpoint.kiev.ua (porto-e.starpoint.kiev.ua [212.40.38.100]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id XAA08789; Mon, 24 Nov 2014 23:23:55 +0200 (EET) (envelope-from avg@FreeBSD.org) Received: from localhost ([127.0.0.1]) by porto.starpoint.kiev.ua with esmtp (Exim 4.34 (FreeBSD)) id 1Xt157-000ClE-Jl; Mon, 24 Nov 2014 23:22:01 +0200 Message-ID: <5473A156.6070502@FreeBSD.org> Date: Mon, 24 Nov 2014 23:21:26 +0200 From: Andriy Gapon User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Mark Johnston , Rui Paulo Subject: Re: DTrace: stack() does not print kernel module functions for i386 References: <20141109093632.GV53947@kib.kiev.ua> <9011F920-3092-4E61-9CDC-68FD9092BB7D@me.com> <201411131336.12334.jhb@freebsd.org> <20141123021856.GA54708@raichu> In-Reply-To: <20141123021856.GA54708@raichu> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Cc: Konstantin Belousov , freebsd-hackers@FreeBSD.org, freebsd-dtrace@FreeBSD.org, John Baldwin , Shrikanth Kamath 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: Mon, 24 Nov 2014 21:22:06 -0000 On 23/11/2014 04:18, Mark Johnston wrote: > On Thu, Nov 13, 2014 at 07:49:27PM -0800, Rui Paulo wrote: >> On Nov 13, 2014, at 10:36, John Baldwin 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? This totally makes sense to me. Thank you! > 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 > -- Andriy Gapon From owner-freebsd-dtrace@FreeBSD.ORG Tue Nov 25 02:36:28 2014 Return-Path: Delivered-To: freebsd-dtrace@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id DA9C3923; Tue, 25 Nov 2014 02:36:27 +0000 (UTC) Received: from mail-ig0-x22d.google.com (mail-ig0-x22d.google.com [IPv6:2607:f8b0:4001:c05::22d]) (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 9FC59A01; Tue, 25 Nov 2014 02:36:27 +0000 (UTC) Received: by mail-ig0-f173.google.com with SMTP id r2so4281568igi.6 for ; Mon, 24 Nov 2014 18:36:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=LCZxffryjKxSWWqZi7tOdjpt0mQZ8pX6UG2MprIt+qU=; b=QDjD78Ham9Hrj8C+XnAOzOSFSETjlfO9BDPPrHaQHtF2u9F11SEl9yKg2AGZsIXBGG afjgPyZTjBONSx/3hIQtEvm4ogTg8vg0BlF0iq0dOA05ObzrhyKSwKLaT8STvCr1/unJ UWwPEHhShw5mlNHB8rYqlYPKHE3EuF+8UGmC1cxnRBX4ZghOfu4+dHDyNETI6eC/TDGw xFOAnSRxtVIaILv99IzsOc62RXXb1ZyMiDndWkXLBPcgpd89msJnzCeDcy3PtQVCGtaL qZ/uPD+8IkxqgkxwPJCB6uMM6AHOoWHhDcITOi0lWdsmT+oh93uABlvcJAIpQG1awBO5 EMMw== MIME-Version: 1.0 X-Received: by 10.107.26.193 with SMTP id a184mr7247435ioa.80.1416882986858; Mon, 24 Nov 2014 18:36:26 -0800 (PST) Received: by 10.107.9.13 with HTTP; Mon, 24 Nov 2014 18:36:26 -0800 (PST) In-Reply-To: <20141123021856.GA54708@raichu> References: <20141109093632.GV53947@kib.kiev.ua> <9011F920-3092-4E61-9CDC-68FD9092BB7D@me.com> <201411131336.12334.jhb@freebsd.org> <20141123021856.GA54708@raichu> Date: Mon, 24 Nov 2014 18:36:26 -0800 Message-ID: Subject: Re: DTrace: stack() does not print kernel module functions for i386 From: Shrikanth Kamath To: Mark Johnston Content-Type: text/plain; charset=UTF-8 Cc: John Baldwin , Rui Paulo , freebsd-hackers@freebsd.org, avg@freebsd.org, Konstantin Belousov , 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, 25 Nov 2014 02:36:28 -0000 > 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. Thanks, -- Shrikanth R K From owner-freebsd-dtrace@FreeBSD.ORG Tue Nov 25 07:12:19 2014 Return-Path: Delivered-To: freebsd-dtrace@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 942F3290; Tue, 25 Nov 2014 07:12:19 +0000 (UTC) Received: from mail-pa0-x234.google.com (mail-pa0-x234.google.com [IPv6:2607:f8b0:400e:c03::234]) (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 5BF28A0D; Tue, 25 Nov 2014 07:12:19 +0000 (UTC) Received: by mail-pa0-f52.google.com with SMTP id eu11so11253886pac.39 for ; Mon, 24 Nov 2014 23:12:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=qiQxsLSS+mkWQlY2YMRQFChaZExiZ0GFQBXZkncBoEk=; b=AuuB382m3MPd1rntQPTJ8915sOcpoI14EjK7I7GR9pcpKYh9TxpIsD2Ew9oRokzLS+ i8Vrr4MnFS1j/mjukW9fW08p3++iKrl5yjXbZZEY81t9dJ4gte8wvDREU7UJ14gVR+dU 3ByysQCQhmPc4mBf4fH4f3DwxYbO4B+NXYxj2x78BBNO560rcfMOLrVn3NM9Y6ShZA6K Fx0hrqOg/8Ozd9JUHF6gneLuepa8zzBMeORLN2VIKBM7Znx+Nx5dbqBSnWeXRH+EvJat 5uML2fQXOlbGtrzVRtETsAou5MS2EOivEGqQ2l70A08JeCvZmpnD9XQNQzDtv3qDHbUS rzJw== X-Received: by 10.66.140.76 with SMTP id re12mr39289193pab.147.1416899538917; Mon, 24 Nov 2014 23:12:18 -0800 (PST) Received: from raichu ([198.244.104.6]) by mx.google.com with ESMTPSA id je4sm486555pbd.94.2014.11.24.23.12.17 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 24 Nov 2014 23:12:17 -0800 (PST) Sender: Mark Johnston Date: Mon, 24 Nov 2014 23:12:12 -0800 From: Mark Johnston To: Shrikanth Kamath Subject: Re: DTrace: stack() does not print kernel module functions for i386 Message-ID: <20141125071212.GA2005@raichu> References: <20141109093632.GV53947@kib.kiev.ua> <9011F920-3092-4E61-9CDC-68FD9092BB7D@me.com> <201411131336.12334.jhb@freebsd.org> <20141123021856.GA54708@raichu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Cc: John Baldwin , Rui Paulo , freebsd-hackers@freebsd.org, avg@freebsd.org, Konstantin Belousov , 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, 25 Nov 2014 07:12:19 -0000 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.