Date: Wed, 26 Feb 2014 12:58:25 -0800 From: Justin Hibbits <jhibbits@freebsd.org> To: Mark Johnston <markj@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers <src-committers@freebsd.org> Subject: Re: svn commit: r262466 - head/sys/cddl/dev/systrace Message-ID: <CAHSQbTCdFR42sv5dkLt8TFT_J1teMRviRHf2-gpMdgnn5menhA@mail.gmail.com> In-Reply-To: <CAHSQbTBxHV=zrbYh62AS2q0HB0tkLgo_3AOR76BD5tgZBTdmTQ@mail.gmail.com> References: <201402250258.s1P2wCDd060659@svn.freebsd.org> <CAHSQbTCWRToOxwZOnCi-TzC_4vHUjppH5Kpdp238oL7HWNFc0A@mail.gmail.com> <20140226013550.GA16841@raichu> <CAHSQbTBxHV=zrbYh62AS2q0HB0tkLgo_3AOR76BD5tgZBTdmTQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Feb 26, 2014 at 9:52 AM, Justin Hibbits <jhibbits@freebsd.org> wrote:
> On Tue, Feb 25, 2014 at 5:35 PM, Mark Johnston <markj@freebsd.org> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAHSQbTCdFR42sv5dkLt8TFT_J1teMRviRHf2-gpMdgnn5menhA>