Linux SPARSE checker discussions
 help / color / mirror / Atom feed
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

  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