* Re: [next:akpm 798/1000] drivers/rtc/rtc-ds1286.c:343:24: sparse: incorrect type in argument 1 (different address spaces) [not found] <5171d93a.0NZAGYYKNj8hjsAs%fengguang.wu@intel.com> @ 2013-04-22 23:56 ` Andrew Morton 2013-04-23 2:51 ` Fengguang Wu 2013-04-23 2:56 ` Christopher Li 0 siblings, 2 replies; 6+ messages in thread From: Andrew Morton @ 2013-04-22 23:56 UTC (permalink / raw) To: kbuild test robot; +Cc: Jingoo Han, kbuild-all, linux-sparse, linux-kernel On Sat, 20 Apr 2013 07:54:34 +0800 kbuild test robot <fengguang.wu@intel.com> wrote: > tree: git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git akpm > head: c9941b7ec7840ad33f5822c7f238157558d40132 > commit: d5e42b5769899607e1e4b0c9200340d24f370e8c [798/1000] rtc: rtc-ds1286: use devm_*() functions > > > sparse warnings: (new ones prefixed by >>) > > >> drivers/rtc/rtc-ds1286.c:343:24: sparse: incorrect type in argument 1 (different address spaces) > drivers/rtc/rtc-ds1286.c:343:24: expected void const *ptr > drivers/rtc/rtc-ds1286.c:343:24: got unsigned int [noderef] [usertype] <asn:2>*rtcregs > >> drivers/rtc/rtc-ds1286.c:344:36: sparse: incorrect type in argument 1 (different address spaces) > drivers/rtc/rtc-ds1286.c:344:36: expected void const *ptr > drivers/rtc/rtc-ds1286.c:344:36: got unsigned int [noderef] [usertype] <asn:2>*rtcregs > > vim +343 drivers/rtc/rtc-ds1286.c > > 337 return -ENODEV; > 338 priv = devm_kzalloc(&pdev->dev, sizeof(struct ds1286_priv), GFP_KERNEL); > 339 if (!priv) > 340 return -ENOMEM; > 341 > 342 priv->rtcregs = devm_ioremap_resource(&pdev->dev, res); > > 343 if (IS_ERR(priv->rtcregs)) > > 344 return PTR_ERR(priv->rtcregs); > 345 > 346 spin_lock_init(&priv->lock); > 347 platform_set_drvdata(pdev, priv); I think doing IS_ERR() and PTR_ERR() on __iomem pointers is a natural thing, and we should be able to do this without adding call-site trickery to make sparse happy. Is there some sort of annotation which we can add to the IS_ERR()/PTR_ERR() definitions so that sparse will stop warning about this usage? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [next:akpm 798/1000] drivers/rtc/rtc-ds1286.c:343:24: sparse: incorrect type in argument 1 (different address spaces) 2013-04-22 23:56 ` [next:akpm 798/1000] drivers/rtc/rtc-ds1286.c:343:24: sparse: incorrect type in argument 1 (different address spaces) Andrew Morton @ 2013-04-23 2:51 ` Fengguang Wu 2013-04-23 2:56 ` Christopher Li 1 sibling, 0 replies; 6+ messages in thread From: Fengguang Wu @ 2013-04-23 2:51 UTC (permalink / raw) To: Andrew Morton; +Cc: Jingoo Han, kbuild-all, linux-sparse, linux-kernel On Mon, Apr 22, 2013 at 04:56:29PM -0700, Andrew Morton wrote: > On Sat, 20 Apr 2013 07:54:34 +0800 kbuild test robot <fengguang.wu@intel.com> wrote: > > > tree: git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git akpm > > head: c9941b7ec7840ad33f5822c7f238157558d40132 > > commit: d5e42b5769899607e1e4b0c9200340d24f370e8c [798/1000] rtc: rtc-ds1286: use devm_*() functions > > > > > > sparse warnings: (new ones prefixed by >>) > > > > >> drivers/rtc/rtc-ds1286.c:343:24: sparse: incorrect type in argument 1 (different address spaces) > > drivers/rtc/rtc-ds1286.c:343:24: expected void const *ptr > > drivers/rtc/rtc-ds1286.c:343:24: got unsigned int [noderef] [usertype] <asn:2>*rtcregs > > >> drivers/rtc/rtc-ds1286.c:344:36: sparse: incorrect type in argument 1 (different address spaces) > > drivers/rtc/rtc-ds1286.c:344:36: expected void const *ptr > > drivers/rtc/rtc-ds1286.c:344:36: got unsigned int [noderef] [usertype] <asn:2>*rtcregs > > > > vim +343 drivers/rtc/rtc-ds1286.c > > > > 337 return -ENODEV; > > 338 priv = devm_kzalloc(&pdev->dev, sizeof(struct ds1286_priv), GFP_KERNEL); > > 339 if (!priv) > > 340 return -ENOMEM; > > 341 > > 342 priv->rtcregs = devm_ioremap_resource(&pdev->dev, res); > > > 343 if (IS_ERR(priv->rtcregs)) > > > 344 return PTR_ERR(priv->rtcregs); > > 345 > > 346 spin_lock_init(&priv->lock); > > 347 platform_set_drvdata(pdev, priv); > > I think doing IS_ERR() and PTR_ERR() on __iomem pointers is a natural > thing, and we should be able to do this without adding call-site > trickery to make sparse happy. > > Is there some sort of annotation which we can add to the > IS_ERR()/PTR_ERR() definitions so that sparse will stop warning about > this usage? If it's too hard to fix in sparse, I can add a check in my scripts, ignoring all "parse: incorrect type in argument 1 (different address spaces)" warnings in the IS_ERR/PTR_ERR lines. Thanks, Fengguang ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [next:akpm 798/1000] drivers/rtc/rtc-ds1286.c:343:24: sparse: incorrect type in argument 1 (different address spaces) 2013-04-22 23:56 ` [next:akpm 798/1000] drivers/rtc/rtc-ds1286.c:343:24: sparse: incorrect type in argument 1 (different address spaces) Andrew Morton 2013-04-23 2:51 ` Fengguang Wu @ 2013-04-23 2:56 ` Christopher Li 2013-04-23 6:16 ` Dan Carpenter 1 sibling, 1 reply; 6+ messages in thread From: Christopher Li @ 2013-04-23 2:56 UTC (permalink / raw) To: Andrew Morton Cc: kbuild test robot, Jingoo Han, kbuild-all, Linux-Sparse, linux-kernel On Mon, Apr 22, 2013 at 4:56 PM, Andrew Morton <akpm@linux-foundation.org> wrote: > I think doing IS_ERR() and PTR_ERR() on __iomem pointers is a natural > thing, and we should be able to do this without adding call-site > trickery to make sparse happy. > > Is there some sort of annotation which we can add to the > IS_ERR()/PTR_ERR() definitions so that sparse will stop warning about > this usage? Yes, the force attribute should silent the address check on conversion. Can some one try this patch (totally untested). Chris diff --git a/include/linux/err.h b/include/linux/err.h index f2edce2..d226a3c 100644 --- a/include/linux/err.h +++ b/include/linux/err.h @@ -26,17 +26,17 @@ static inline void * __must_check ERR_PTR(long error) static inline long __must_check PTR_ERR(const void *ptr) { - return (long) ptr; + return (__force long) ptr; } static inline long __must_check IS_ERR(const void *ptr) { - return IS_ERR_VALUE((unsigned long)ptr); + return IS_ERR_VALUE((__force unsigned long)ptr); } static inline long __must_check IS_ERR_OR_NULL(const void *ptr) { - return !ptr || IS_ERR_VALUE((unsigned long)ptr); + return !ptr || IS_ERR_VALUE((__force unsigned long)ptr); } /** ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [next:akpm 798/1000] drivers/rtc/rtc-ds1286.c:343:24: sparse: incorrect type in argument 1 (different address spaces) 2013-04-23 2:56 ` Christopher Li @ 2013-04-23 6:16 ` Dan Carpenter 2013-04-26 2:09 ` [PATCH] forced argument Was " Christopher Li 0 siblings, 1 reply; 6+ messages in thread From: Dan Carpenter @ 2013-04-23 6:16 UTC (permalink / raw) To: Christopher Li Cc: Andrew Morton, kbuild test robot, Jingoo Han, kbuild-all, Linux-Sparse, linux-kernel On Mon, Apr 22, 2013 at 07:56:19PM -0700, Christopher Li wrote: > On Mon, Apr 22, 2013 at 4:56 PM, Andrew Morton > <akpm@linux-foundation.org> wrote: > > I think doing IS_ERR() and PTR_ERR() on __iomem pointers is a natural > > thing, and we should be able to do this without adding call-site > > trickery to make sparse happy. > > > > Is there some sort of annotation which we can add to the > > IS_ERR()/PTR_ERR() definitions so that sparse will stop warning about > > this usage? > > Yes, the force attribute should silent the address check on conversion. > > Can some one try this patch (totally untested). > That didn't work. It's the the void * in the parameter list that's the problem. We'd need to do something like the patch below: Otherwise we could add "__ok_to_cast" thing to Sparse maybe? regards, dan carpenter diff --git a/include/linux/err.h b/include/linux/err.h index f2edce2..2cbe8fb 100644 --- a/include/linux/err.h +++ b/include/linux/err.h @@ -24,20 +24,23 @@ static inline void * __must_check ERR_PTR(long error) return (void *) error; } -static inline long __must_check PTR_ERR(const void *ptr) +static inline long __must_check _PTR_ERR(const void *ptr) { return (long) ptr; } +#define PTR_ERR(x) _PTR_ERR((const void __force *)(x)) -static inline long __must_check IS_ERR(const void *ptr) +static inline long __must_check _IS_ERR(const void *ptr) { return IS_ERR_VALUE((unsigned long)ptr); } +#define IS_ERR(x) _IS_ERR((const void __force *)(x)) -static inline long __must_check IS_ERR_OR_NULL(const void *ptr) +static inline long __must_check _IS_ERR_OR_NULL(const void *ptr) { return !ptr || IS_ERR_VALUE((unsigned long)ptr); } +#define IS_ERR_OR_NULL(x) _IS_ERR_OR_NULL((const void __force *)(x)) /** * ERR_CAST - Explicitly cast an error-valued pointer to another pointer type @@ -46,19 +49,21 @@ static inline long __must_check IS_ERR_OR_NULL(const void *ptr) * Explicitly cast an error-valued pointer to another pointer type in such a * way as to make it clear that's what's going on. */ -static inline void * __must_check ERR_CAST(const void *ptr) +static inline void * __must_check _ERR_CAST(const void *ptr) { /* cast away the const */ return (void *) ptr; } +#define ERR_CAST(x) _ERR_CAST((const void __force *)(x)) -static inline int __must_check PTR_RET(const void *ptr) +static inline int __must_check _PTR_RET(const void *ptr) { if (IS_ERR(ptr)) return PTR_ERR(ptr); else return 0; } +#define PTR_RET(x) _PTR_RET((const void __force *)(x)) #endif ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] forced argument Was Re: sparse: incorrect type in argument 1 (different address spaces) 2013-04-23 6:16 ` Dan Carpenter @ 2013-04-26 2:09 ` Christopher Li 2013-04-26 6:35 ` Dan Carpenter 0 siblings, 1 reply; 6+ messages in thread From: Christopher Li @ 2013-04-26 2:09 UTC (permalink / raw) To: Dan Carpenter Cc: Andrew Morton, kbuild test robot, Jingoo Han, kbuild-all, Linux-Sparse, linux-kernel [-- Attachment #1: Type: text/plain, Size: 722 bytes --] On 04/22/2013 11:16 PM, Dan Carpenter wrote: > That didn't work. It's the the void * in the parameter list that's > the problem. We'd need to do something like the patch below: > > Otherwise we could add "__ok_to_cast" thing to Sparse maybe? Thanks for the insight. I make a small patch to test the __ok_to_cast feature. The syntax is adding the force attribute to the argument declaration. it will look like this: static inline long __must_check PTR_ERR( __force const void *ptr) That means the "ptr" argument will perform a forced cast when receiving the argument. It is OK to pass __iomem pointer to "ptr". The example are in the patch. It need to patch both sparse and the Linux tree. What do you say? Chris [-- Attachment #2: 0001-Allow-forced-attribute-in-function-argument.patch --] [-- Type: text/x-patch, Size: 2071 bytes --] From a0974ed0fc1e67c41608c780b748c205622956b8 Mon Sep 17 00:00:00 2001 From: Christopher Li <sparse@chrisli.org> Date: Thu, 25 Apr 2013 18:09:43 -0700 Subject: [PATCH] Allow forced attribute in function argument It will indicate this argument will skip the compatible check. --- evaluate.c | 2 +- parse.c | 1 + symbol.h | 3 ++- validation/fored_arg.c | 18 ++++++++++++++++++ 4 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 validation/fored_arg.c diff --git a/evaluate.c b/evaluate.c index 9f2c4ac..0dfa519 100644 --- a/evaluate.c +++ b/evaluate.c @@ -2137,7 +2137,7 @@ static int evaluate_arguments(struct symbol *f, struct symbol *fn, struct expres else degenerate(expr); } - } else { + } else if (!target->forced_arg){ static char where[30]; examine_symbol_type(target); sprintf(where, "argument %d", i); diff --git a/parse.c b/parse.c index 45ffc10..890e56b 100644 --- a/parse.c +++ b/parse.c @@ -1841,6 +1841,7 @@ static struct token *parameter_declaration(struct token *token, struct symbol *s sym->ctype = ctx.ctype; sym->ctype.modifiers |= storage_modifiers(&ctx); sym->endpos = token->pos; + sym->forced_arg = ctx.storage_class == SForced; return token; } diff --git a/symbol.h b/symbol.h index 1e74579..1c6ad66 100644 --- a/symbol.h +++ b/symbol.h @@ -157,7 +157,8 @@ struct symbol { expanding:1, evaluated:1, string:1, - designated_init:1; + designated_init:1, + forced_arg:1; struct expression *array_size; struct ctype ctype; struct symbol_list *arguments; diff --git a/validation/fored_arg.c b/validation/fored_arg.c new file mode 100644 index 0000000..4ab7141 --- /dev/null +++ b/validation/fored_arg.c @@ -0,0 +1,18 @@ +/* + * check-name: Forced function argument type. + */ + +#define __iomem __attribute__((noderef, address_space(2))) +#define __force __attribute__((force)) + +static void foo(__force void * addr) +{ +} + + +static void bar(void) +{ + void __iomem *a; + foo(a); +} + -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] forced argument Was Re: sparse: incorrect type in argument 1 (different address spaces) 2013-04-26 2:09 ` [PATCH] forced argument Was " Christopher Li @ 2013-04-26 6:35 ` Dan Carpenter 0 siblings, 0 replies; 6+ messages in thread From: Dan Carpenter @ 2013-04-26 6:35 UTC (permalink / raw) To: Christopher Li Cc: Andrew Morton, kbuild test robot, Jingoo Han, kbuild-all, Linux-Sparse, linux-kernel On Thu, Apr 25, 2013 at 07:09:37PM -0700, Christopher Li wrote: > On 04/22/2013 11:16 PM, Dan Carpenter wrote: > > That didn't work. It's the the void * in the parameter list that's > > the problem. We'd need to do something like the patch below: > > > > Otherwise we could add "__ok_to_cast" thing to Sparse maybe? > > Thanks for the insight. I make a small patch to test the __ok_to_cast > feature. The syntax is adding the force attribute to the argument > declaration. > > it will look like this: > static inline long __must_check PTR_ERR( __force const void *ptr) > > That means the "ptr" argument will perform a forced cast when receiving > the argument. It is OK to pass __iomem pointer to "ptr". > > The example are in the patch. It need to patch both sparse and the > Linux tree. > > What do you say? That's looks great. :) I tested a patched kernel with an unpatched kernel as well and that doesn't cause any new problems. regards, dan carpenter ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-04-26 6:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <5171d93a.0NZAGYYKNj8hjsAs%fengguang.wu@intel.com>
2013-04-22 23:56 ` [next:akpm 798/1000] drivers/rtc/rtc-ds1286.c:343:24: sparse: incorrect type in argument 1 (different address spaces) Andrew Morton
2013-04-23 2:51 ` Fengguang Wu
2013-04-23 2:56 ` Christopher Li
2013-04-23 6:16 ` Dan Carpenter
2013-04-26 2:09 ` [PATCH] forced argument Was " Christopher Li
2013-04-26 6:35 ` Dan Carpenter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).