From owner-svn-src-head@FreeBSD.ORG Wed Feb 26 17:52:53 2014 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id BDB647A1; Wed, 26 Feb 2014 17:52:53 +0000 (UTC) Received: from mail-ee0-x229.google.com (mail-ee0-x229.google.com [IPv6:2a00:1450:4013:c00::229]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id D2DBD1562; Wed, 26 Feb 2014 17:52:52 +0000 (UTC) Received: by mail-ee0-f41.google.com with SMTP id b15so748952eek.14 for ; Wed, 26 Feb 2014 09:52:51 -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=pVe2jDCxhOUGIA17gXl/LCrn0/zFC6r8aUcjWo7xOM0=; b=teo7Qd51d639S1hMI3KGeN7KUbMJUtIekeAYghz+IWjYqh/jueDd0kjkKUv5bDFNaN 6dkEQfH+PqIaVs9IVvix+2vvm0akvcrto6x0SP5qe8dkGAeZFHIQv9ZlKhI6nWt6g+Rm faD11j6Avr3s7iZfaPMjavNJQFr3YRwzaN952a7FZPpebXQngPBbXWwLKXdfs/iUuzOl 7ytYc6QAQv7QOgRNZnQzYKuH5++KGWD+cf3WtOjSg4zl/IlonbFy3G9/Idx7hrDSYz0t bxTrq7BunPQglG/VljuO5WHvhdzu0gdE3xNsKOi9HCP7QNKV/QdG0pn8TMg7fBU4S2za wrvQ== MIME-Version: 1.0 X-Received: by 10.204.26.69 with SMTP id d5mr83688bkc.47.1393437171252; Wed, 26 Feb 2014 09:52:51 -0800 (PST) Sender: chmeeedalf@gmail.com Received: by 10.205.21.68 with HTTP; Wed, 26 Feb 2014 09:52:51 -0800 (PST) In-Reply-To: <20140226013550.GA16841@raichu> References: <201402250258.s1P2wCDd060659@svn.freebsd.org> <20140226013550.GA16841@raichu> Date: Wed, 26 Feb 2014 09:52:51 -0800 X-Google-Sender-Auth: sbsQz6OmbPcygjhnqxcG48qxlPM 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 17:52:53 -0000 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