From: Josh Triplett <josh@joshtriplett.org>
To: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Cc: linux-sparse@vger.kernel.org
Subject: Re: [PATCH] quiet memset() warning with sizeof(VLA)
Date: Sat, 17 Feb 2018 11:18:26 -0800 [thread overview]
Message-ID: <20180217191826.GA22695@localhost> (raw)
In-Reply-To: <20180217165839.61981-1-luc.vanoostenryck@gmail.com>
On Sat, Feb 17, 2018 at 05:58:39PM +0100, Luc Van Oostenryck wrote:
> Currently, sparse doesn't handle yet VLA's sizeof(). The size
> of a VLA is considered as zero and the result of sizeof() on
> a VLA is treated as an error with a value of -1.
>
> That's a problem with the check done by the sparse tool, which
> warn when operations like memset() are done with a static count
> which is above some limit. Of course, all this is done with
> unsigned numbers and the -1 from sizeof(VLA) is then considered
> as off-limit.
>
> Sure, size of VLAs should be supported but it's longer term.
> One short term solution would be to do the check with signed
> numbers but that would eat the upper bit of the limit which
> may well be the bound that we would like to check.
>
> So, check instead for sizes of -1, which must come from some
> previous errors that must have already been reported, and do
> not issue the memset() warning in this case.
I'm concerned about generically ignoring this warning for *all* -1
sizes, because -1 seems like a very common value to slip through for
other reasons. Losing those very real warnings just to avoid getting a
second warning on a VLA doesn't seem worth it.
(Also, at the very *least* this would need a comment explaining *why* it
ignores -1.)
Could you somehow propagate a taintedness on that value that causes the
memset to ignore it? Or just change the dummy error value to 0?
> sparse.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/sparse.c b/sparse.c
> index bceacd94e..5f53a9b9b 100644
> --- a/sparse.c
> +++ b/sparse.c
> @@ -153,6 +153,8 @@ static void check_byte_count(struct instruction *insn, pseudo_t count)
> return;
> if (count->type == PSEUDO_VAL) {
> unsigned long long val = count->value;
> + if (val == -1ULL)
> + return;
> if (Wmemcpy_max_count && val > fmemcpy_max_count)
> warning(insn->pos, "%s with byte count of %llu",
> show_ident(insn->func->sym->ident), val);
> --
> 2.16.0
>
> --
> 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
next prev parent reply other threads:[~2018-02-17 19:18 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-17 16:58 [PATCH] quiet memset() warning with sizeof(VLA) Luc Van Oostenryck
2018-02-17 19:18 ` Josh Triplett [this message]
2018-02-17 19:57 ` Luc Van Oostenryck
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180217191826.GA22695@localhost \
--to=josh@joshtriplett.org \
--cc=linux-sparse@vger.kernel.org \
--cc=luc.vanoostenryck@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox