From owner-svn-src-head@FreeBSD.ORG Wed Feb 26 20:58:29 2014 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 49A36369; Wed, 26 Feb 2014 20:58:29 +0000 (UTC) Received: from mail-ee0-x22c.google.com (mail-ee0-x22c.google.com [IPv6:2a00:1450:4013:c00::22c]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 5CA921ACE; Wed, 26 Feb 2014 20:58:28 +0000 (UTC) Received: by mail-ee0-f44.google.com with SMTP id d49so878180eek.31 for ; Wed, 26 Feb 2014 12:58:25 -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:date:message-id:subject :from:to:cc:content-type; bh=DkQZPPfaS9BS4iCvfRR/LTS3dU5A7GD2SHgCwYsfNMI=; b=tXOJ6uqYON4NV/gP9Rp1ohkekWSFFeneQU4wmLCe+BSJX/fSIAdfvIOcod0xiG0bdD tDHi/xLNWUmC5kdSrYVor//Q5lJIlZVpIzGpDwmQMI7oK4kw3OoI+VKUfqTi7gUPsBOw r9SDyrtaC8IJKXHdWL8XTQB2kGxKh9jRO67WIRXextEPo1vy19LaRKiN9W9Pxq2Q3mg6 Xr50VLZSWeVNU8+gz0mS7wikorZzdViCJEp9bZCndEwLM/XXj9JeIQ4rVxW4RZ20tuGh x66Fc39Z/wpIN4Hdhp+6qO/FPaCg1Ra5b9ZM5pXDVdidW1TjTYc3Y/TNlSGmzjX7CSit BybA== MIME-Version: 1.0 X-Received: by 10.204.96.205 with SMTP id i13mr3220499bkn.20.1393448305690; Wed, 26 Feb 2014 12:58:25 -0800 (PST) Sender: chmeeedalf@gmail.com Received: by 10.205.21.68 with HTTP; Wed, 26 Feb 2014 12:58:25 -0800 (PST) In-Reply-To: References: <201402250258.s1P2wCDd060659@svn.freebsd.org> <20140226013550.GA16841@raichu> Date: Wed, 26 Feb 2014 12:58:25 -0800 X-Google-Sender-Auth: Ui2BXv9t99dxJ_4BWEoSJY-ox5k Message-ID: Subject: Re: svn commit: r262466 - head/sys/cddl/dev/systrace From: Justin Hibbits To: Mark Johnston Content-Type: text/plain; charset=UTF-8 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 26 Feb 2014 20:58:29 -0000 On Wed, Feb 26, 2014 at 9:52 AM, Justin Hibbits wrote: > On Tue, Feb 25, 2014 at 5:35 PM, Mark Johnston wrote: >> On Tue, Feb 25, 2014 at 03:17:56PM -0800, Justin Hibbits wrote: >>> I think this broke powerpc building. I see the following build failure: >>> >>> cc1: warnings being treated as errors >>> /home/chmeee/freebsd/head/sys/modules/dtrace/systrace/../../../cddl/dev/systrace/systrace.c: >>> In function 'systrace_probe': >>> /home/chmeee/freebsd/head/sys/modules/dtrace/systrace/../../../cddl/dev/systrace/systrace.c:218: >>> warning: function called through a non-compatible type >>> /home/chmeee/freebsd/head/sys/modules/dtrace/systrace/../../../cddl/dev/systrace/systrace.c:218: >>> note: if this code is reached, the program will abort >>> >> >> Hi Justin, >> >> Sorry about this. I've reverted the commit. >> >> I realize that the change introduced undefined behaviour, but a similar >> trick is used elsewhere in the DTrace code to pass extra arguments at a >> probe site. Calling dtrace_probe() through a function pointer (patch >> below) makes the warning go away, but I don't really understand why. >> clang doesn't emit warnings in either case. >> >> Thanks, >> -Mark >> >> diff --git a/sys/cddl/dev/systrace/systrace.c b/sys/cddl/dev/systrace/systrace.c >> index 83f0793..5f4b82f 100644 >> --- a/sys/cddl/dev/systrace/systrace.c >> +++ b/sys/cddl/dev/systrace/systrace.c >> @@ -168,8 +168,8 @@ static dtrace_pops_t systrace_pops = { >> static struct cdev *systrace_cdev; >> static dtrace_provider_id_t systrace_id; >> >> -typedef void (*systrace_dtrace_probe)(dtrace_id_t, uintptr_t, uintptr_t, >> - uintptr_t, uintptr_t, uintptr_t, uintptr_t, uintptr_t, uintptr_t); >> +typedef void (*systrace_probe_t)(dtrace_id_t, uintptr_t, uintptr_t, uintptr_t, >> + uintptr_t, uintptr_t, uintptr_t, uintptr_t, uintptr_t); >> >> #if !defined(LINUX_SYSTRACE) >> /* >> @@ -214,8 +214,9 @@ systrace_probe(u_int32_t id, int sysnum, struct sysent *sysent, void *params, >> } >> >> /* Process the probe using the converted argments. */ >> - ((systrace_dtrace_probe)(dtrace_probe))(id, uargs[0], uargs[1], >> - uargs[2], uargs[3], uargs[4], uargs[5], uargs[6], uargs[7]); >> + systrace_probe_t probe = (systrace_probe_t)dtrace_probe; >> + probe(id, uargs[0], uargs[1], uargs[2], uargs[3], uargs[4], >> + uargs[5], uargs[6], uargs[7]); >> } >> >> #endif > > > Hi Mark, > > I think this patch works because it circumvents gcc's variable > tracking. With the first patch, gcc knew the function that was being > called, and knew it was undefined behavior. With the second patch, it > only knows at that time that you're calling through a function > pointer. It's completely forgotten that the function pointer is > pointing to that function. Just a guess. I'll give it a go and let > you know if it still complains. > > Thanks! > > - Justin Just tested and confirmed it builds successfully. - Justin