linux-sparse.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
To: Ben Dooks <ben.dooks@codethink.co.uk>
Cc: linux-sparse@vger.kernel.org
Subject: Re: [PATCH 0/8] format check tweaks
Date: Tue, 6 Oct 2020 01:47:49 +0200	[thread overview]
Message-ID: <20201005234749.dfw34pvgb2uavuko@ltop.local> (raw)
In-Reply-To: <58d771f9-7560-f682-3173-78dea0f83711@codethink.co.uk>

On Mon, Oct 05, 2020 at 10:04:43AM +0100, Ben Dooks wrote:
> On 05/10/2020 02:59, Luc Van Oostenryck wrote:
> > Ben, these are small changes I think should be applied with
> > your series, on top of the patches I send yesterday.
> > I've pushed everything at:
> > 	git://github.com/lucvoo/sparse-dev.git format-check
> > If you're fine with these, I can squeeze them with the series.
> > 
> > There are also a few tests I don't agree with:
> > 	const void *ptr = ...
> > 	printf("%s", ptr);
> > These tests silently accept this, but they should warn.
> > But this can be fixed at a later step.
> 
> ok, thanks.

Hi,

I've now pushed it on a separate branch on the official tree:
  git://git.kernel.org/pub/scm/devel/sparse/sparse.git format-check

It will thus not be rebased anymore and any changes will need to be
incrementally on top of it.

> I'm going to try and work out the best way to deal with the kernel
> extra funsies. I have a few ideas but yet to make a coherent document
> about them.

Well, the reason I've not yet merged it with the main branch is
because enabling -Wformat creates really a lot of warnings in
the kernel and people definitively use this.

Most of these warnings (if not all) is caused by using
check_assignment_types() which was good as a quick & dirty solution
in the early stages but isn't at all adequate as a true solution
and this for several reasons. I've also begin to take a look at
this with a relatively satisfying result by adopting the following
strategy:
	Since no 'checking' function in evaluate.c has the needed
	characteristics, simplest is to create in verify-format.c
	what is needed.
This also gives the flexibility needed to support things like
-Wformat-signedness.

I add here below only small extract because it's for now quite
messy and need quite a bit of polishing.

-- Luc


enum {
	CLASS_OTHER,
	CLASS_INT,
	CLASS_BITWISE,
	CLASS_FLOAT,
	CLASS_PTR,
};

///
// retrieve a 'type class' for quick testing and extract the base type
static int type_class(struct symbol *type, struct symbol **base)
{
	if (type->type == SYM_NODE)
		type = type->ctype.base_type;
	if (type->type == SYM_ENUM)
		type = type->ctype.base_type;
	*base = type;
	if (type->type == SYM_BASETYPE) {
		struct symbol *kind = type->ctype.base_type;
		if (kind == &int_type)
			return CLASS_INT;
		if (kind == &fp_type)
			return CLASS_FLOAT;
	}
	if (type->type == SYM_PTR)
		return CLASS_PTR;
	if (type->type == SYM_RESTRICT)
		return CLASS_BITWISE;
	return CLASS_OTHER;
}

static const char *printf_fmt_flt(struct format_type *fmt, struct symbol *source)
{
	const char *typediff = "different base types";
	struct symbol *target = fmt->type;
	struct symbol *base;

	if (type_class(source, &base) != CLASS_FLOAT)
		return typediff;
	if (base == target)
		return NULL;
	return typediff;
}

static const char *printf_fmt_string(struct format_type *fmt, struct symbol *source)
{
	const char *typediff = "different base types";
	struct symbol *target = fmt->type;

	examine_pointer_target(target);
	examine_pointer_target(source);

	if (type_class(source, &source) != CLASS_PTR)
		return typediff;
	if (source->ctype.as)
		return "different address spaces";

	// get the base type
	type_class(source->ctype.base_type, &source);
	type_class(source, &source);

	if (target == &const_string_ctype) {
		if (source == &char_ctype)
			return NULL;
		if (source == &uchar_ctype)
			return NULL;
		if (source == &schar_ctype)
			return NULL;
	} if (target == &const_wstring_ctype) {
		if (source == wchar_ctype)
			return NULL;
	}
	return typediff;
}

  reply	other threads:[~2020-10-05 23:47 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-05  1:59 [PATCH 0/8] format check tweaks Luc Van Oostenryck
2020-10-05  1:59 ` [PATCH 1/8] need to strip SYM_NODE before comparing types Luc Van Oostenryck
2020-10-05  1:59 ` [PATCH 2/8] add helper get_nth_expression() Luc Van Oostenryck
2020-10-05  1:59 ` [PATCH 3/8] move the definition of FMT_{PRINTF,SCANF} Luc Van Oostenryck
2020-10-05  1:59 ` [PATCH 4/8] parse format attribute less verbosely Luc Van Oostenryck
2020-10-05  1:59 ` [PATCH 5/8] call verify_format_attribute() unconditionally Luc Van Oostenryck
2020-10-05  2:00 ` [PATCH 6/8] s/data/type/ for struct format_type Luc Van Oostenryck
2020-10-05  2:00 ` [PATCH 7/8] add support for "%ls" Luc Van Oostenryck
2020-10-05  2:00 ` [PATCH 8/8] add support for "%Lx" (and "%Ls") Luc Van Oostenryck
2020-10-05  9:04 ` [PATCH 0/8] format check tweaks Ben Dooks
2020-10-05 23:47   ` Luc Van Oostenryck [this message]
2020-10-11 19:51     ` Ben Dooks
2020-10-12 22:38       ` 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=20201005234749.dfw34pvgb2uavuko@ltop.local \
    --to=luc.vanoostenryck@gmail.com \
    --cc=ben.dooks@codethink.co.uk \
    --cc=linux-sparse@vger.kernel.org \
    /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;
as well as URLs for NNTP newsgroup(s).