| Message ID | 20170601202724.77597-4-luc.vanoostenryck@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 01/06/17 21:27, Luc Van Oostenryck wrote: > By default, sparse will warn if memcpy() (or memset(), > copy_from_user(), copy_to_user()) is called with a very large > static byte-count. > > But the limit is currently fixed at 100000, which may be fine > for some uses but not for others. For example, this value is > too low for sparse to be used on the git tree where, for example, > some array used to sort the index is cleared with memset(). > > Change this by making the limit configurable via a new flag: > -fmemcpy-max-count. > > Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> > --- > lib.c | 16 ++++++++++++++++ > lib.h | 1 + > sparse.1 | 10 ++++++++++ > sparse.c | 3 +-- > 4 files changed, 28 insertions(+), 2 deletions(-) > > diff --git a/lib.c b/lib.c > index 90fd2b494..1378cc243 100644 > --- a/lib.c > +++ b/lib.c > @@ -256,6 +256,7 @@ int dbg_dead = 0; > > int fmem_report = 0; > int fdump_linearize; > +unsigned long fmemcpy_max_count = 100000; > > int preprocess_only; > > @@ -670,6 +671,19 @@ static char **handle_switch_O(char *arg, char **next) > return next; > } > > +static char **handle_switch_fmemcpy_max_count(char *arg, char **next) > +{ > + unsigned long val; > + char *end; > + > + val = strtoul(arg, &end, 0); > + if (*end != '\0' || end == arg) > + die("error: missing argument to \"-fmemcpy-max-count=\""); > + > + fmemcpy_max_count = val; > + return next; > +} > + > static char **handle_switch_ftabstop(char *arg, char **next) > { > char *end; > @@ -713,6 +727,8 @@ static char **handle_switch_f(char *arg, char **next) > return handle_switch_ftabstop(arg+8, next); > if (!strncmp(arg, "dump-", 5)) > return handle_switch_fdump(arg+5, next); > + if (!strncmp(arg, "memcpy-max-count=", 17)) > + return handle_switch_fmemcpy_max_count(arg+17, next); > > /* handle switches w/ arguments above, boolean and only boolean below */ > if (handle_simple_switch(arg, "mem-report", &fmem_report)) > diff --git a/lib.h b/lib.h > index 8090fe247..b7cb451e0 100644 > --- a/lib.h > +++ b/lib.h > @@ -143,6 +143,7 @@ extern int dbg_dead; > > extern int fmem_report; > extern int fdump_linearize; > +extern unsigned long fmemcpy_max_count; > > extern int arch_m64; > > diff --git a/sparse.1 b/sparse.1 > index efbd78d01..932ac82ef 100644 > --- a/sparse.1 > +++ b/sparse.1 > @@ -216,6 +216,9 @@ Warn about call of \fBmemset()\fR, \fBmemset()\fR, \fBcopy_from_user()\fR, or > > Sparse issues these warnings by default. To turn them off, use > \fB\-Wno\-memcpy\-max\-count\fR. > + > +The limit can be changed with \fB\-fmemcpy\-max\-count=COUNT\fR, > +the default being \fB100000\fR. > . > .TP > .B \-Wnon\-pointer\-null > @@ -364,6 +367,13 @@ Report some statistics about memory allocation used by the tool. > . > .SH OTHER OPTIONS > .TP > +.B \-fmemcpy-limit=COUNT > +By default, sparse will warn if \fBmemcpy()\fR (or \fBmemset()\fR, > +\fBcopy_from_user()\fR, copy_to_user()\fR) is called with a very large > +(known at compile-time) byte-count. COUNT is the value under which > +no such warning will be given. The default limit is 100000. > +. > +.TP So, in addition to -Wno-memcpy-max-count, you could turn the warning off with just -fmemcpy-limit=0. cool. Thanks! ATB, Ramsay Jones > .B \-ftabstop=WIDTH > Set the distance between tab stops. This helps sparse report correct > column numbers in warnings or errors. If the value is less than 1 or > diff --git a/sparse.c b/sparse.c > index aa5979f1a..bceacd94e 100644 > --- a/sparse.c > +++ b/sparse.c > @@ -153,8 +153,7 @@ static void check_byte_count(struct instruction *insn, pseudo_t count) > return; > if (count->type == PSEUDO_VAL) { > unsigned long long val = count->value; > - if (Wmemcpy_max_count && val > 100000ULL) > - > + if (Wmemcpy_max_count && val > fmemcpy_max_count) > warning(insn->pos, "%s with byte count of %llu", > show_ident(insn->func->sym->ident), val); > return; > -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/06/17 01:30, Ramsay Jones wrote: > > > On 01/06/17 21:27, Luc Van Oostenryck wrote: >> By default, sparse will warn if memcpy() (or memset(), >> copy_from_user(), copy_to_user()) is called with a very large >> static byte-count. >> >> But the limit is currently fixed at 100000, which may be fine >> for some uses but not for others. For example, this value is >> too low for sparse to be used on the git tree where, for example, >> some array used to sort the index is cleared with memset(). >> >> Change this by making the limit configurable via a new flag: >> -fmemcpy-max-count. >> >> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> >> --- >> lib.c | 16 ++++++++++++++++ >> lib.h | 1 + >> sparse.1 | 10 ++++++++++ >> sparse.c | 3 +-- >> 4 files changed, 28 insertions(+), 2 deletions(-) >> >> diff --git a/lib.c b/lib.c >> index 90fd2b494..1378cc243 100644 >> --- a/lib.c >> +++ b/lib.c >> @@ -256,6 +256,7 @@ int dbg_dead = 0; >> >> int fmem_report = 0; >> int fdump_linearize; >> +unsigned long fmemcpy_max_count = 100000; >> >> int preprocess_only; >> >> @@ -670,6 +671,19 @@ static char **handle_switch_O(char *arg, char **next) >> return next; >> } >> >> +static char **handle_switch_fmemcpy_max_count(char *arg, char **next) >> +{ >> + unsigned long val; >> + char *end; >> + >> + val = strtoul(arg, &end, 0); >> + if (*end != '\0' || end == arg) >> + die("error: missing argument to \"-fmemcpy-max-count=\""); >> + >> + fmemcpy_max_count = val; >> + return next; >> +} >> + >> static char **handle_switch_ftabstop(char *arg, char **next) >> { >> char *end; >> @@ -713,6 +727,8 @@ static char **handle_switch_f(char *arg, char **next) >> return handle_switch_ftabstop(arg+8, next); >> if (!strncmp(arg, "dump-", 5)) >> return handle_switch_fdump(arg+5, next); >> + if (!strncmp(arg, "memcpy-max-count=", 17)) >> + return handle_switch_fmemcpy_max_count(arg+17, next); >> >> /* handle switches w/ arguments above, boolean and only boolean below */ >> if (handle_simple_switch(arg, "mem-report", &fmem_report)) >> diff --git a/lib.h b/lib.h >> index 8090fe247..b7cb451e0 100644 >> --- a/lib.h >> +++ b/lib.h >> @@ -143,6 +143,7 @@ extern int dbg_dead; >> >> extern int fmem_report; >> extern int fdump_linearize; >> +extern unsigned long fmemcpy_max_count; >> >> extern int arch_m64; >> >> diff --git a/sparse.1 b/sparse.1 >> index efbd78d01..932ac82ef 100644 >> --- a/sparse.1 >> +++ b/sparse.1 >> @@ -216,6 +216,9 @@ Warn about call of \fBmemset()\fR, \fBmemset()\fR, \fBcopy_from_user()\fR, or >> >> Sparse issues these warnings by default. To turn them off, use >> \fB\-Wno\-memcpy\-max\-count\fR. >> + >> +The limit can be changed with \fB\-fmemcpy\-max\-count=COUNT\fR, >> +the default being \fB100000\fR. >> . >> .TP >> .B \-Wnon\-pointer\-null >> @@ -364,6 +367,13 @@ Report some statistics about memory allocation used by the tool. >> . >> .SH OTHER OPTIONS >> .TP >> +.B \-fmemcpy-limit=COUNT >> +By default, sparse will warn if \fBmemcpy()\fR (or \fBmemset()\fR, >> +\fBcopy_from_user()\fR, copy_to_user()\fR) is called with a very large >> +(known at compile-time) byte-count. COUNT is the value under which >> +no such warning will be given. The default limit is 100000. >> +. >> +.TP > > So, in addition to -Wno-memcpy-max-count, you could turn the warning > off with just -fmemcpy-limit=0. cool. heh, so I obviously didn't read the code! Ahem. :-D Thanks again. ATB, Ramsay Jones >> .B \-ftabstop=WIDTH >> Set the distance between tab stops. This helps sparse report correct >> column numbers in warnings or errors. If the value is less than 1 or >> diff --git a/sparse.c b/sparse.c >> index aa5979f1a..bceacd94e 100644 >> --- a/sparse.c >> +++ b/sparse.c >> @@ -153,8 +153,7 @@ static void check_byte_count(struct instruction *insn, pseudo_t count) >> return; >> if (count->type == PSEUDO_VAL) { >> unsigned long long val = count->value; >> - if (Wmemcpy_max_count && val > 100000ULL) >> - >> + if (Wmemcpy_max_count && val > fmemcpy_max_count) >> warning(insn->pos, "%s with byte count of %llu", >> show_ident(insn->func->sym->ident), val); >> return; >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-sparse" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jun 2, 2017 at 2:30 AM, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote: >> .SH OTHER OPTIONS >> .TP >> +.B \-fmemcpy-limit=COUNT >> +By default, sparse will warn if \fBmemcpy()\fR (or \fBmemset()\fR, >> +\fBcopy_from_user()\fR, copy_to_user()\fR) is called with a very large >> +(known at compile-time) byte-count. COUNT is the value under which >> +no such warning will be given. The default limit is 100000. >> +. >> +.TP > > So, in addition to -Wno-memcpy-max-count, you could turn the warning > off with just -fmemcpy-limit=0. cool. > > Thanks! > > ATB, > Ramsay Jones Well, for now setting the limit to 0 would just warn about any non-zero memcpy/memset but it's something that could very easily be added, sure. -- Luc -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/06/17 01:38, Luc Van Oostenryck wrote: > On Fri, Jun 2, 2017 at 2:30 AM, Ramsay Jones > <ramsay@ramsayjones.plus.com> wrote: > >>> .SH OTHER OPTIONS >>> .TP >>> +.B \-fmemcpy-limit=COUNT >>> +By default, sparse will warn if \fBmemcpy()\fR (or \fBmemset()\fR, >>> +\fBcopy_from_user()\fR, copy_to_user()\fR) is called with a very large >>> +(known at compile-time) byte-count. COUNT is the value under which >>> +no such warning will be given. The default limit is 100000. >>> +. >>> +.TP >> >> So, in addition to -Wno-memcpy-max-count, you could turn the warning >> off with just -fmemcpy-limit=0. cool. >> >> Thanks! >> >> ATB, >> Ramsay Jones > > Well, for now setting the limit to 0 would just warn about any > non-zero memcpy/memset > but it's something that could very easily be added, sure. Yes, as I noted in another email, I didn't read the code correctly! (and in my patch I had a single -Wmem-limit=n argument which _did_ disable the check when n = 0). Naming issues aside (and I'm bad at naming, so don't listen to me on that), I like your -W[no-]memcpy-max-count and -fmemcpy-limit=n split. [You may want to remove the '=COUNT' from the commit message of the 2/3 patch]. Thanks again. ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jun 2, 2017 at 3:42 AM, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote: >> Well, for now setting the limit to 0 would just warn about any >> non-zero memcpy/memset >> but it's something that could very easily be added, sure. > > Yes, as I noted in another email, I didn't read the code correctly! > (and in my patch I had a single -Wmem-limit=n argument which _did_ > disable the check when n = 0). > > Naming issues aside (and I'm bad at naming, so don't listen to me > on that), I like your -W[no-]memcpy-max-count and -fmemcpy-limit=n > split. > > [You may want to remove the '=COUNT' from the commit message of > the 2/3 patch]. Oh yes, indeed. Thanks. -- Luc -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/lib.c b/lib.c index 90fd2b494..1378cc243 100644 --- a/lib.c +++ b/lib.c @@ -256,6 +256,7 @@ int dbg_dead = 0; int fmem_report = 0; int fdump_linearize; +unsigned long fmemcpy_max_count = 100000; int preprocess_only; @@ -670,6 +671,19 @@ static char **handle_switch_O(char *arg, char **next) return next; } +static char **handle_switch_fmemcpy_max_count(char *arg, char **next) +{ + unsigned long val; + char *end; + + val = strtoul(arg, &end, 0); + if (*end != '\0' || end == arg) + die("error: missing argument to \"-fmemcpy-max-count=\""); + + fmemcpy_max_count = val; + return next; +} + static char **handle_switch_ftabstop(char *arg, char **next) { char *end; @@ -713,6 +727,8 @@ static char **handle_switch_f(char *arg, char **next) return handle_switch_ftabstop(arg+8, next); if (!strncmp(arg, "dump-", 5)) return handle_switch_fdump(arg+5, next); + if (!strncmp(arg, "memcpy-max-count=", 17)) + return handle_switch_fmemcpy_max_count(arg+17, next); /* handle switches w/ arguments above, boolean and only boolean below */ if (handle_simple_switch(arg, "mem-report", &fmem_report)) diff --git a/lib.h b/lib.h index 8090fe247..b7cb451e0 100644 --- a/lib.h +++ b/lib.h @@ -143,6 +143,7 @@ extern int dbg_dead; extern int fmem_report; extern int fdump_linearize; +extern unsigned long fmemcpy_max_count; extern int arch_m64; diff --git a/sparse.1 b/sparse.1 index efbd78d01..932ac82ef 100644 --- a/sparse.1 +++ b/sparse.1 @@ -216,6 +216,9 @@ Warn about call of \fBmemset()\fR, \fBmemset()\fR, \fBcopy_from_user()\fR, or Sparse issues these warnings by default. To turn them off, use \fB\-Wno\-memcpy\-max\-count\fR. + +The limit can be changed with \fB\-fmemcpy\-max\-count=COUNT\fR, +the default being \fB100000\fR. . .TP .B \-Wnon\-pointer\-null @@ -364,6 +367,13 @@ Report some statistics about memory allocation used by the tool. . .SH OTHER OPTIONS .TP +.B \-fmemcpy-limit=COUNT +By default, sparse will warn if \fBmemcpy()\fR (or \fBmemset()\fR, +\fBcopy_from_user()\fR, copy_to_user()\fR) is called with a very large +(known at compile-time) byte-count. COUNT is the value under which +no such warning will be given. The default limit is 100000. +. +.TP .B \-ftabstop=WIDTH Set the distance between tab stops. This helps sparse report correct column numbers in warnings or errors. If the value is less than 1 or diff --git a/sparse.c b/sparse.c index aa5979f1a..bceacd94e 100644 --- a/sparse.c +++ b/sparse.c @@ -153,8 +153,7 @@ static void check_byte_count(struct instruction *insn, pseudo_t count) return; if (count->type == PSEUDO_VAL) { unsigned long long val = count->value; - if (Wmemcpy_max_count && val > 100000ULL) - + if (Wmemcpy_max_count && val > fmemcpy_max_count) warning(insn->pos, "%s with byte count of %llu", show_ident(insn->func->sym->ident), val); return;
By default, sparse will warn if memcpy() (or memset(), copy_from_user(), copy_to_user()) is called with a very large static byte-count. But the limit is currently fixed at 100000, which may be fine for some uses but not for others. For example, this value is too low for sparse to be used on the git tree where, for example, some array used to sort the index is cleared with memset(). Change this by making the limit configurable via a new flag: -fmemcpy-max-count. Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> --- lib.c | 16 ++++++++++++++++ lib.h | 1 + sparse.1 | 10 ++++++++++ sparse.c | 3 +-- 4 files changed, 28 insertions(+), 2 deletions(-)