[kvm:queue 27/38] arch/x86/kvm/hyperv.c:186:41: sparse: incorrect type in argument 2 (different modifiers)

[kvm:queue 27/38] arch/x86/kvm/hyperv.c:186:41: sparse: incorrect type in argument 2 (different modifiers)

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/virt/kvm/kvm.git queue
head:   ed393e4134de0dd02d8ba98ca8ce3ae65d1eb567
commit: 46f4c309534b10ca1026273abe38955d3cff4023 [27/38] kvm/x86: Hyper-V HV_X64_MSR_VP_RUNTIME support
reproduce:
  # apt-get install sparse
  git checkout 46f4c309534b10ca1026273abe38955d3cff4023
  make ARCH=x86_64 allmodconfig
  make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> arch/x86/kvm/hyperv.c:186:41: sparse: incorrect type in argument 2 (different modifiers)
   arch/x86/kvm/hyperv.c:186:41:    expected unsigned long [nocast] [usertype] *ut
   arch/x86/kvm/hyperv.c:186:41:    got unsigned long *<noident>
>> arch/x86/kvm/hyperv.c:186:41: sparse: implicit cast to nocast type
>> arch/x86/kvm/hyperv.c:186:49: sparse: incorrect type in argument 3 (different modifiers)
   arch/x86/kvm/hyperv.c:186:49:    expected unsigned long [nocast] [usertype] *st
   arch/x86/kvm/hyperv.c:186:49:    got unsigned long *<noident>
   arch/x86/kvm/hyperv.c:186:49: sparse: implicit cast to nocast type

vim +186 arch/x86/kvm/hyperv.c

   170				kvm_make_request(KVM_REQ_HV_RESET, vcpu);
   171			}
   172			break;
   173		default:
   174			vcpu_unimpl(vcpu, "Hyper-V uhandled wrmsr: 0x%x data 0x%llx\n",
   175				    msr, data);
   176			return 1;
   177		}
   178		return 0;
   179	}
   180	
   181	/* Calculate cpu time spent by current task in 100ns units */
   182	static u64 current_task_runtime_100ns(void)
   183	{
   184		cputime_t utime, stime;
   185	
 > 186		task_cputime_adjusted(current, &utime, &stime);
   187		return div_u64(cputime_to_nsecs(utime + stime), 100);
   188	}
   189	
   190	static int kvm_hv_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host)
   191	{
   192		struct kvm_vcpu_hv *hv = &vcpu->arch.hyperv;
   193	
   194		switch (msr) {

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [kvm:queue 27/38] arch/x86/kvm/hyperv.c:186:41: sparse: incorrect type in argument 2 (different modifiers)

[Denis V. Lunev]

On 09/18/2015 04:39 PM, kbuild test robot wrote:
> tree:   https://git.kernel.org/pub/scm/virt/kvm/kvm.git queue
> head:   ed393e4134de0dd02d8ba98ca8ce3ae65d1eb567
> commit: 46f4c309534b10ca1026273abe38955d3cff4023 [27/38] kvm/x86: Hyper-V HV_X64_MSR_VP_RUNTIME support
> reproduce:
>    # apt-get install sparse
>    git checkout 46f4c309534b10ca1026273abe38955d3cff4023
>    make ARCH=x86_64 allmodconfig
>    make C=1 CF=-D__CHECK_ENDIAN__
>
>
> sparse warnings: (new ones prefixed by >>)
>
>>> arch/x86/kvm/hyperv.c:186:41: sparse: incorrect type in argument 2 (different modifiers)
>     arch/x86/kvm/hyperv.c:186:41:    expected unsigned long [nocast] [usertype] *ut
>     arch/x86/kvm/hyperv.c:186:41:    got unsigned long *<noident>
>>> arch/x86/kvm/hyperv.c:186:41: sparse: implicit cast to nocast type
>>> arch/x86/kvm/hyperv.c:186:49: sparse: incorrect type in argument 3 (different modifiers)
>     arch/x86/kvm/hyperv.c:186:49:    expected unsigned long [nocast] [usertype] *st
>     arch/x86/kvm/hyperv.c:186:49:    got unsigned long *<noident>
>     arch/x86/kvm/hyperv.c:186:49: sparse: implicit cast to nocast type
>
> vim +186 arch/x86/kvm/hyperv.c
>
>     170				kvm_make_request(KVM_REQ_HV_RESET, vcpu);
>     171			}
>     172			break;
>     173		default:
>     174			vcpu_unimpl(vcpu, "Hyper-V uhandled wrmsr: 0x%x data 0x%llx\n",
>     175				    msr, data);
>     176			return 1;
>     177		}
>     178		return 0;
>     179	}
>     180	
>     181	/* Calculate cpu time spent by current task in 100ns units */
>     182	static u64 current_task_runtime_100ns(void)
>     183	{
>     184		cputime_t utime, stime;
>     185	
>   > 186		task_cputime_adjusted(current, &utime, &stime);
>     187		return div_u64(cputime_to_nsecs(utime + stime), 100);
>     188	}
>     189	
>     190	static int kvm_hv_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host)
>     191	{
>     192		struct kvm_vcpu_hv *hv = &vcpu->arch.hyperv;
>     193	
>     194		switch (msr) {
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
can not get an idea what is this warning about...
For me it looks pretty lame.

Den

Re: [kvm:queue 27/38] arch/x86/kvm/hyperv.c:186:41: sparse: incorrect type in argument 2 (different modifiers)

[Paolo Bonzini]

On 18/09/2015 15:51, Denis V. Lunev wrote:
>>     185   
>>   > 186        task_cputime_adjusted(current, &utime, &stime);
>>     187        return div_u64(cputime_to_nsecs(utime + stime), 100);
>>     188    }
>>     189   
>>     190    static int kvm_hv_set_msr(struct kvm_vcpu *vcpu, u32 msr,
>> u64 data, bool host)
>>     191    {
>>     192        struct kvm_vcpu_hv *hv = &vcpu->arch.hyperv;
>>     193   
>>     194        switch (msr) {
>>
>> ---
>> 0-DAY kernel test infrastructure                Open Source Technology
>> Center
>> https://lists.01.org/pipermail/kbuild-all                   Intel
>> Corporation
> can not get an idea what is this warning about...
> For me it looks pretty lame.

I think it wants you to do

-	return div_u64(cputime_to_nsecs(utime + stime), 100);
+	return div_u64(cputime_to_nsecs(utime) +
+		       cputime_to_nsecs(stime), 100);

Paolo

Re: [kvm:queue 27/38] arch/x86/kvm/hyperv.c:186:41: sparse: incorrect type in argument 2 (different modifiers)

[Denis V. Lunev]

On 09/18/2015 04:55 PM, Paolo Bonzini wrote:
>
> On 18/09/2015 15:51, Denis V. Lunev wrote:
>>>      185
>>>    > 186        task_cputime_adjusted(current, &utime, &stime);
>>>      187        return div_u64(cputime_to_nsecs(utime + stime), 100);
>>>      188    }
>>>      189
>>>      190    static int kvm_hv_set_msr(struct kvm_vcpu *vcpu, u32 msr,
>>> u64 data, bool host)
>>>      191    {
>>>      192        struct kvm_vcpu_hv *hv = &vcpu->arch.hyperv;
>>>      193
>>>      194        switch (msr) {
>>>
>>> ---
>>> 0-DAY kernel test infrastructure                Open Source Technology
>>> Center
>>> https://lists.01.org/pipermail/kbuild-all                   Intel
>>> Corporation
>> can not get an idea what is this warning about...
>> For me it looks pretty lame.
> I think it wants you to do
>
> -	return div_u64(cputime_to_nsecs(utime + stime), 100);
> +	return div_u64(cputime_to_nsecs(utime) +
> +		       cputime_to_nsecs(stime), 100);
>
> Paolo
ok, I'll check but warning points here

   > 186        task_cputime_adjusted(current, &utime, &stime);

and says about parameters 2 and 3. There is no parameter 3
in div_u64 call :)

Re: [kvm:queue 27/38] arch/x86/kvm/hyperv.c:186:41: sparse: incorrect type in argument 2 (different modifiers)

[Roman Kagan]

On Fri, Sep 18, 2015 at 03:55:00PM +0200, Paolo Bonzini wrote:
> 
> 
> On 18/09/2015 15:51, Denis V. Lunev wrote:
> >>     185   
> >>   > 186        task_cputime_adjusted(current, &utime, &stime);
> >>     187        return div_u64(cputime_to_nsecs(utime + stime), 100);
> >>     188    }
> >>     189   
> >>     190    static int kvm_hv_set_msr(struct kvm_vcpu *vcpu, u32 msr,
> >> u64 data, bool host)
> >>     191    {
> >>     192        struct kvm_vcpu_hv *hv = &vcpu->arch.hyperv;
> >>     193   
> >>     194        switch (msr) {
> >>
> >> ---
> >> 0-DAY kernel test infrastructure                Open Source Technology
> >> Center
> >> https://lists.01.org/pipermail/kbuild-all                   Intel
> >> Corporation
> > can not get an idea what is this warning about...
> > For me it looks pretty lame.
> 
> I think it wants you to do
> 
> -	return div_u64(cputime_to_nsecs(utime + stime), 100);
> +	return div_u64(cputime_to_nsecs(utime) +
> +		       cputime_to_nsecs(stime), 100);

The warning is pretty specific about the point where it triggered.

I have reduced it to the following standalone reproducer:

%%% cat x.c
#ifdef __CHECKER__
# define __nocast       __attribute__((nocast))
#else
# define __nocast
#endif

typedef unsigned long __nocast cputime_t;

extern void task_cputime_adjusted(cputime_t *);
extern void current_task_runtime_100ns(void);

void current_task_runtime_100ns(void)
{
        cputime_t utime;

        task_cputime_adjusted(&utime);
}
%%% gcc -c x.c -Wall -Werror -O2; echo $?
0
%%% sparse x.c
x.c:16:32: warning: incorrect type in argument 1 (different modifiers)
x.c:16:32:    expected unsigned long [nocast] [usertype] *<noident>
x.c:16:32:    got unsigned long *<noident>
x.c:16:32: warning: implicit cast to nocast type

Looks like a sparse bug to me.

Roman.

Re: [kvm:queue 27/38] arch/x86/kvm/hyperv.c:186:41: sparse: incorrect type in argument 2 (different modifiers)

[Paolo Bonzini]

On 18/09/2015 16:40, Roman Kagan wrote:
> typedef unsigned long __nocast cputime_t;
> 
> extern void task_cputime_adjusted(cputime_t *);
> extern void current_task_runtime_100ns(void);
> 
> void current_task_runtime_100ns(void)
> {
>         cputime_t utime;
> 
>         task_cputime_adjusted(&utime);
> }
> %%% gcc -c x.c -Wall -Werror -O2; echo $?
> 0
> %%% sparse x.c
> x.c:16:32: warning: incorrect type in argument 1 (different modifiers)
> x.c:16:32:    expected unsigned long [nocast] [usertype] *<noident>
> x.c:16:32:    got unsigned long *<noident>
> x.c:16:32: warning: implicit cast to nocast type
> 
> Looks like a sparse bug to me.

Indeed...

Paolo

Re: [kbuild-all] [kvm:queue 27/38] arch/x86/kvm/hyperv.c:186:41: sparse: incorrect type in argument 2 (different modifiers)

[Fengguang Wu]

[CC sparse people]

On Fri, Sep 18, 2015 at 04:41:56PM +0200, Paolo Bonzini wrote:
> 
> 
> On 18/09/2015 16:40, Roman Kagan wrote:
> > typedef unsigned long __nocast cputime_t;
> > 
> > extern void task_cputime_adjusted(cputime_t *);
> > extern void current_task_runtime_100ns(void);
> > 
> > void current_task_runtime_100ns(void)
> > {
> >         cputime_t utime;
> > 
> >         task_cputime_adjusted(&utime);
> > }
> > %%% gcc -c x.c -Wall -Werror -O2; echo $?
> > 0
> > %%% sparse x.c
> > x.c:16:32: warning: incorrect type in argument 1 (different modifiers)
> > x.c:16:32:    expected unsigned long [nocast] [usertype] *<noident>
> > x.c:16:32:    got unsigned long *<noident>
> > x.c:16:32: warning: implicit cast to nocast type
> > 
> > Looks like a sparse bug to me.
> 
> Indeed...
> 
> Paolo
> _______________________________________________
> kbuild-all mailing list
> kbuild-all@lists.01.org
> https://lists.01.org/mailman/listinfo/kbuild-all

Re: [kbuild-all] [kvm:queue 27/38] arch/x86/kvm/hyperv.c:186:41: sparse: incorrect type in argument 2 (different modifiers)

[Luc Van Oostenryck]

On Fri, Sep 18, 2015 at 11:06:44PM +0800, Fengguang Wu wrote:
> [CC sparse people]
> 
> On Fri, Sep 18, 2015 at 04:41:56PM +0200, Paolo Bonzini wrote:
> > 
> > 
> > On 18/09/2015 16:40, Roman Kagan wrote:
> > > typedef unsigned long __nocast cputime_t;
> > > 
> > > extern void task_cputime_adjusted(cputime_t *);
> > > extern void current_task_runtime_100ns(void);
> > > 
> > > void current_task_runtime_100ns(void)
> > > {
> > >         cputime_t utime;
> > > 
> > >         task_cputime_adjusted(&utime);
> > > }
> > > %%% gcc -c x.c -Wall -Werror -O2; echo $?
> > > 0
> > > %%% sparse x.c
> > > x.c:16:32: warning: incorrect type in argument 1 (different modifiers)
> > > x.c:16:32:    expected unsigned long [nocast] [usertype] *<noident>
> > > x.c:16:32:    got unsigned long *<noident>
> > > x.c:16:32: warning: implicit cast to nocast type
> > > 
> > > Looks like a sparse bug to me.
> > 
> > Indeed...
> > 
> > Paolo

The problem is that the intent and semantic of 'nocast' is not clear at all.
There is an explanation about 'nocast' vs. 'bitwise' here:
	https://git.kernel.org/cgit/devel/sparse/sparse.git/tree/Documentation/sparse.txt
but it doesn't give much info about the exact wanted behaviour.

Since for the kernel 'nocast' is only used for cputime_t & cputime64_t,
I think it should be clarified and fixed if needed.
A patch proposal is following.

Regards,
Luc

[PATCH] Do not drop 'nocast' modifier when taking the address.

[Luc Van Oostenryck]

With the following code:
	typedef unsigned long __nocast cputime_t;

	void task_cputime_adjusted(cputime_t *);

	void current_task_runtime_100ns(void)
	{
	        cputime_t utime;

	        task_cputime_adjusted(&utime);
	}

sparse emits the following message:
	x.c:16:32: warning: incorrect type in argument 1 (different modifiers)
	x.c:16:32:    expected unsigned long [nocast] [usertype] *<noident>
	x.c:16:32:    got unsigned long *<noident>
	x.c:16:32: warning: implicit cast to nocast type

In other words, when taking the address of 'utime', sparse drops the 'nocast'
modifier and then complains that task_cputime_adjusted() is not given a
'nocast' pointer as expected ...

This feels wrong to me.

The proposed fix is to simply not dropping the 'nocast' modifier when
taking the address, like done for a normal type qualifier.
This gives very reasonable behaviour for all the test cases I could
think of:
	- taking the address or dereferencing doesn't drop the nocast
	- arithmetic operations on nocast give a nocast result.
	- implicit to/from cast is OK only if the base type are the same
	- implicit to/from pointer cast is OK only if the base type are the same

This still gives a "leaky" semantic: the nocast modifier can be lost via
an implicit cast to a non-qualified value.

Explicit cast to or from nocast values doesn't give any warning, maybe
it's OK, maybe it's not but it's orthogonal to the current issue.

Is this the wanted semantic for the nocast modifier?

Reported-by: Roman Kagan <rkagan@virtuozzo.com>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 symbol.h            |   2 +-
 validation/nocast.c | 197 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 198 insertions(+), 1 deletion(-)
 create mode 100644 validation/nocast.c

diff --git a/symbol.h b/symbol.h
index ccb5dcb9..9b3f1604 100644
--- a/symbol.h
+++ b/symbol.h
@@ -247,7 +247,7 @@ struct symbol {
 #define MOD_SIZE	(MOD_CHAR | MOD_SHORT | MOD_LONG_ALL)
 #define MOD_IGNORE (MOD_TOPLEVEL | MOD_STORAGE | MOD_ADDRESSABLE |	\
 	MOD_ASSIGNED | MOD_USERTYPE | MOD_ACCESSED | MOD_EXPLICITLY_SIGNED)
-#define MOD_PTRINHERIT (MOD_VOLATILE | MOD_CONST | MOD_NODEREF | MOD_STORAGE | MOD_NORETURN)
+#define MOD_PTRINHERIT (MOD_VOLATILE | MOD_CONST | MOD_NODEREF | MOD_STORAGE | MOD_NORETURN | MOD_NOCAST)
 
 
 /* Current parsing/evaluation function */
diff --git a/validation/nocast.c b/validation/nocast.c
new file mode 100644
index 00000000..c28676a3
--- /dev/null
+++ b/validation/nocast.c
@@ -0,0 +1,197 @@
+#define	__nocast	__attribute__((nocast))
+typedef unsigned long __nocast ulong_nc_t;
+
+extern void use_val(ulong_nc_t);
+extern void use_ptr(ulong_nc_t *);
+
+/* use address */
+static void good_use_address(void)
+{
+	ulong_nc_t t;
+
+	use_ptr(&t);
+}
+
+static ulong_nc_t *good_ret_address(void)
+{
+	static ulong_nc_t t;
+
+	return &t;
+}
+
+static ulong_nc_t good_deref(ulong_nc_t *t)
+{
+	return *t;
+}
+
+/* assign value */
+static ulong_nc_t t;
+static ulong_nc_t good_assign_self = t;
+static unsigned long good_assign_sametype = t;
+
+/* assign pointer */
+static ulong_nc_t *good_ptr = &t;
+static ulong_nc_t *bad_ptr_to = 1UL;
+static unsigned long *bad_ptr_from = &t;
+
+/* arithmetic operation */
+static ulong_nc_t good_arith(ulong_nc_t t, unsigned int n)
+{
+	return t + n;
+}
+
+/* implicit cast to other types */
+static unsigned long good_ret_samecast(ulong_nc_t t)
+{
+	return t;
+}
+static unsigned long long bad_ret_biggercast(ulong_nc_t t)
+{
+	return t;
+}
+static long bad_ret_signcast(ulong_nc_t t)
+{
+	return t;
+}
+static short bad_ret_smallercast(ulong_nc_t t)
+{
+	return t;
+}
+
+static void assign_val(ulong_nc_t t)
+{
+	ulong_nc_t good_c = t;
+	unsigned long good_ul = t;
+	unsigned long long bad_ull = t;
+	long bad_l = t;
+	short bad_i = t;
+}
+
+static void assign_via_ptr(ulong_nc_t *t)
+{
+	ulong_nc_t good_c = *t;
+	unsigned long good_ul = *t;
+	unsigned long long bad_ull = *t;
+	long bad_l = *t;
+	short bad_i = *t;
+}
+
+static void assign_ptr(ulong_nc_t *t)
+{
+	ulong_nc_t *good_same_type = t;
+	unsigned long *bad_mod = t;
+	unsigned long long __nocast *bad_size = t;
+	short __nocast *bad_i = t;
+	long __nocast *bad_l = t;
+}
+
+/* implicit cast to nocast */
+static void implicit_assign_to(void)
+{
+	ulong_nc_t t;
+	unsigned long ul = 1;
+	unsigned short us = 1;
+	unsigned long long ull = 1;
+	long l = 1;
+
+	t = ul;		/* implicit to nocast from same type: OK? */
+	t = us;
+	t = ull;
+	t = l;
+}
+
+static void bad_implicit_arg_to(void)
+{
+	unsigned long ul = 1;
+	unsigned short us = 1;
+	unsigned long long ull = 1;
+	long l = 1;
+
+	use_val(ul);	/* implicit to nocast from same type: OK? */
+	use_val(us);
+	use_val(ull);
+	use_val(l);
+}
+
+/* implicit cast from nocast */
+static unsigned long good_implicit_ret_ul(ulong_nc_t t)
+{
+	return t;	/* implicit to nocast from same type: OK? */
+}
+
+static unsigned short bad_implicit_ret_us(ulong_nc_t t)
+{
+	return t;
+}
+
+static unsigned long long bad_implicit_ret_ull(ulong_nc_t t)
+{
+	return t;
+}
+
+static long bad_implicit_ret_l(ulong_nc_t t)
+{
+	return t;
+}
+
+/* FIXME: explicit cast: should we complain? */
+static ulong_nc_t good_samecast(ulong_nc_t v)
+{
+	return (ulong_nc_t) v;
+}
+
+static ulong_nc_t bad_tocast(unsigned long v)
+{
+	return (ulong_nc_t) v;
+}
+
+static unsigned long bad_fromcast(ulong_nc_t v)
+{
+	return (unsigned long) v;
+}
+
+/*
+ * check-name: nocast.c
+ *
+ * check-error-start
+nocast.c:34:33: warning: incorrect type in initializer (different base types)
+nocast.c:34:33:    expected unsigned long [nocast] [usertype] *static [toplevel] bad_ptr_to
+nocast.c:34:33:    got unsigned long
+nocast.c:34:33: warning: implicit cast to nocast type
+nocast.c:35:39: warning: incorrect type in initializer (different modifiers)
+nocast.c:35:39:    expected unsigned long *static [toplevel] bad_ptr_from
+nocast.c:35:39:    got unsigned long static [nocast] [toplevel] *<noident>
+nocast.c:35:39: warning: implicit cast from nocast type
+nocast.c:50:16: warning: implicit cast from nocast type
+nocast.c:54:16: warning: implicit cast from nocast type
+nocast.c:58:16: warning: implicit cast from nocast type
+nocast.c:65:38: warning: implicit cast from nocast type
+nocast.c:66:22: warning: implicit cast from nocast type
+nocast.c:67:23: warning: implicit cast from nocast type
+nocast.c:74:38: warning: implicit cast from nocast type
+nocast.c:75:22: warning: implicit cast from nocast type
+nocast.c:76:23: warning: implicit cast from nocast type
+nocast.c:82:34: warning: incorrect type in initializer (different modifiers)
+nocast.c:82:34:    expected unsigned long *bad_mod
+nocast.c:82:34:    got unsigned long [nocast] [usertype] *t
+nocast.c:82:34: warning: implicit cast from nocast type
+nocast.c:83:49: warning: incorrect type in initializer (different type sizes)
+nocast.c:83:49:    expected unsigned long long [nocast] *bad_size
+nocast.c:83:49:    got unsigned long [nocast] [usertype] *t
+nocast.c:83:49: warning: implicit cast to/from nocast type
+nocast.c:84:33: warning: incorrect type in initializer (different type sizes)
+nocast.c:84:33:    expected short [nocast] *bad_i
+nocast.c:84:33:    got unsigned long [nocast] [usertype] *t
+nocast.c:84:33: warning: implicit cast to/from nocast type
+nocast.c:85:32: warning: implicit cast to/from nocast type
+nocast.c:98:13: warning: implicit cast to nocast type
+nocast.c:99:13: warning: implicit cast to nocast type
+nocast.c:100:13: warning: implicit cast to nocast type
+nocast.c:111:17: warning: implicit cast to nocast type
+nocast.c:112:17: warning: implicit cast to nocast type
+nocast.c:113:17: warning: implicit cast to nocast type
+nocast.c:124:16: warning: implicit cast from nocast type
+nocast.c:129:16: warning: implicit cast from nocast type
+nocast.c:134:16: warning: implicit cast from nocast type
+ * check-error-end
+ */
-- 
2.6.4

Re: [PATCH] Do not drop 'nocast' modifier when taking the address.

[Christopher Li]

On Wed, Jan 6, 2016 at 12:25 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> With the following code:
>         typedef unsigned long __nocast cputime_t;
>
>         void task_cputime_adjusted(cputime_t *);
>
>         void current_task_runtime_100ns(void)
>         {
>                 cputime_t utime;
>
>                 task_cputime_adjusted(&utime);
>         }
>
> sparse emits the following message:
>         x.c:16:32: warning: incorrect type in argument 1 (different modifiers)
>         x.c:16:32:    expected unsigned long [nocast] [usertype] *<noident>
>         x.c:16:32:    got unsigned long *<noident>
>         x.c:16:32: warning: implicit cast to nocast type
>
> In other words, when taking the address of 'utime', sparse drops the 'nocast'
> modifier and then complains that task_cputime_adjusted() is not given a
> 'nocast' pointer as expected ...

I think there is a bug some where else. In the above example,
"cputime_t *" and "&utime" should have the same type regardless
pointer inherent the nocast attribute or not. I haven't fully understand
where the nocast attribute get dropped.

Chris

Re: [PATCH] Do not drop 'nocast' modifier when taking the address.

[Luc Van Oostenryck]

On Wed, Feb 03, 2016 at 04:25:44AM +0800, Christopher Li wrote:
> On Wed, Jan 6, 2016 at 12:25 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > With the following code:
> >         typedef unsigned long __nocast cputime_t;
> >
> >         void task_cputime_adjusted(cputime_t *);
> >
> >         void current_task_runtime_100ns(void)
> >         {
> >                 cputime_t utime;
> >
> >                 task_cputime_adjusted(&utime);
> >         }
> >
> > sparse emits the following message:
> >         x.c:16:32: warning: incorrect type in argument 1 (different modifiers)
> >         x.c:16:32:    expected unsigned long [nocast] [usertype] *<noident>
> >         x.c:16:32:    got unsigned long *<noident>
> >         x.c:16:32: warning: implicit cast to nocast type
> >
> > In other words, when taking the address of 'utime', sparse drops the 'nocast'
> > modifier and then complains that task_cputime_adjusted() is not given a
> > 'nocast' pointer as expected ...
> 
> I think there is a bug some where else. In the above example,
> "cputime_t *" and "&utime" should have the same type regardless
> pointer inherent the nocast attribute or not. I haven't fully understand
> where the nocast attribute get dropped.

The nocast mod is dropped and lost in the function create_pointer().
In the example above, "cputime_t *" has type :
	unsigned long [nocast] [usertype] *
while &utime is just:
	unsigned long *

So, for sparse and its extended notion of type, the type we get when
taking the address of a [variable of some] type X is not the same as
directly using a pointer to the type X.
Which is very fine, just that MOD_NOCAST is dropped while the example
shows that it should not.
OTOH, MOD_STORAGE is kept but I think should be dropped; but that's
another story.


Regards,
Luc

Re: [PATCH] Do not drop 'nocast' modifier when taking the address.

[Christopher Li]

On Wed, Feb 3, 2016 at 11:43 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
pped.
>
> The nocast mod is dropped and lost in the function create_pointer().
> In the example above, "cputime_t *" has type :
>         unsigned long [nocast] [usertype] *
> while &utime is just:
>         unsigned long *

That is my point. Why does "&utime" get drop but "cputime_t *" does not?
They both are pointer of a base type. They both create pointers.

It seems to me the bug is sparse not treating this two case consistently.

> So, for sparse and its extended notion of type, the type we get when
> taking the address of a [variable of some] type X is not the same as
> directly using a pointer to the type X.

In C language type system,  these two should be the same type. It is a
bug in sparse if they are not. I would rather get that bug fixed.

> Which is very fine, just that MOD_NOCAST is dropped while the example
> shows that it should not.

I think that is a separate issue weather MOD_NOCAST should be inherent
from pointer base type. Same with MOD_STORAGE.

Chris

Re: [PATCH] Do not drop 'nocast' modifier when taking the address.

[Luc Van Oostenryck]

On Wed, Feb 03, 2016 at 12:09:16PM +0800, Christopher Li wrote:
> On Wed, Feb 3, 2016 at 11:43 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> pped.
> >
> > The nocast mod is dropped and lost in the function create_pointer().
> > In the example above, "cputime_t *" has type :
> >         unsigned long [nocast] [usertype] *
> > while &utime is just:
> >         unsigned long *
> 
> That is my point. Why does "&utime" get drop but "cputime_t *" does not?
> They both are pointer of a base type. They both create pointers.

Well with "cputime_t *" you got the pointer directly, by its own declaration;
with "&utime" you really _create_ one with the "&"/"addressof" operator.

The function "create_pointer()" is only called when evaluating an expression
using the addressof operator or when when an array of a function is degenerated
into a pointer.

> It seems to me the bug is sparse not treating this two case consistently.
> 
> > So, for sparse and its extended notion of type, the type we get when
> > taking the address of a [variable of some] type X is not the same as
> > directly using a pointer to the type X.
> 
> In C language type system,  these two should be the same type. It is a
> bug in sparse if they are not. I would rather get that bug fixed.

They _have_ the same type if we limit ourselves to the pure C type system,
but they differ once we look also at the sparse & gcc extension to the
type system, like the nocast attribute here.
 
Now, whether they should be the same or not is a question of defining the
semantic of the addressof operator on sparse's type extension.

> > Which is very fine, just that MOD_NOCAST is dropped while the example
> > shows that it should not.
> 
> I think that is a separate issue weather MOD_NOCAST should be inherent
> from pointer base type.

I'm not sure to understand you here.

> Same with MOD_STORAGE.
> 
> Chris

Luc

Re: [PATCH] Do not drop 'nocast' modifier when taking the address.

[Christopher Li]

On Wed, Feb 3, 2016 at 5:15 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> Well with "cputime_t *" you got the pointer directly, by its own declaration;
> with "&utime" you really _create_ one with the "&"/"addressof" operator.
>
> The function "create_pointer()" is only called when evaluating an expression
> using the addressof operator or when when an array of a function is degenerated
> into a pointer.

Yes, I take a closer look. My previous understanding of how the MOD_PTRINHERIT
works was wrong.

Now I see the pointer is created differently. In the parsing stage, it is
created by the "pointer" function. In the evaluation stage it is created
by the "create_pointer" function. The "pointer" function has no treatment for
MOD_PTRINHERIT at all.

Ideally it would be nice to unify the pointer create some how. It is
easier to have
inconsistent modifier other than MOD_NOCAST. As a simply fix, your patch is
fine. Applied.

>
> They _have_ the same type if we limit ourselves to the pure C type system,
> but they differ once we look also at the sparse & gcc extension to the
> type system, like the nocast attribute here.
>
> Now, whether they should be the same or not is a question of defining the
> semantic of the addressof operator on sparse's type extension.

I think sparse should treat them with same semantic. It does not make sense
if they are not the same.

Thanks.

Chris