From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754972AbbL3OGt (ORCPT ); Wed, 30 Dec 2015 09:06:49 -0500 Received: from mail2-relais-roc.national.inria.fr ([192.134.164.83]:13383 "EHLO mail2-relais-roc.national.inria.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754929AbbL3OGc (ORCPT ); Wed, 30 Dec 2015 09:06:32 -0500 X-IronPort-AV: E=Sophos;i="5.20,500,1444687200"; d="scan'208";a="195004726" Date: Wed, 30 Dec 2015 15:06:29 +0100 (CET) From: Julia Lawall X-X-Sender: jll@localhost6.localdomain6 To: Andrzej Hajda cc: Julia Lawall , Bartlomiej Zolnierkiewicz , Marek Szyprowski , Gilles Muller , Nicolas Palix , Michal Marek , open list , "moderated list:COCCINELLE/Semantic Patches (SmPL)" Subject: Re: [PATCH v5] coccinelle: tests: unsigned value cannot be lesser than zero In-Reply-To: <1451481943-20282-1-git-send-email-a.hajda@samsung.com> Message-ID: References: <5683CF1A.1030109@samsung.com> <1451481943-20282-1-git-send-email-a.hajda@samsung.com> User-Agent: Alpine 2.02 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org I would suggest to change rule r to the following. You had ... before and after the big disjunction with the ||s, but that will limit the detection of the problem to functions that contain only one occurrence. ( v = f(...)@vs; ... when != v = e; * (\( v@p <=@e 0 \| v@p >@e 0 \)) ... when any | ( (\( v@p < 0 \| v@p <= 0 \)) || ... || (\( v >= c \| v > c \)) | (\( v >= c \| v > c \)) || ... || (\( v@p < 0 \| v@p <= 0 \)) | (\( v@p >= 0 \| v@p > 0 \)) && ... && (\( v < c \| v <= c \)) | ((\( v < c \| v <= c \) && ... && \( v@p >= 0 \| v@p > 0 \))) | * (\( v@p <@e 0 \| v@p >=@e 0 \)) ) ) julia On Wed, 30 Dec 2015, Andrzej Hajda wrote: > Unsigned expressions cannot be lesser than zero. Presence of comparisons > 'unsigned (<|<=|>|>=) 0' often indicates a bug, usually wrong type of variable. > The patch beside finding such comparisons tries to eliminate false positives, > mainly by bypassing range checks. > > gcc can detect such comparisons also using -Wtype-limits switch, but it warns > also in correct cases, making too much noise. > > Signed-off-by: Andrzej Hajda > --- > v5: improved range check detection > v4: added range check detection, added full check in case value holds a result > of signed function > v3: added bool type > v2: added --all-includes option > --- > .../tests/unsigned_lesser_than_zero.cocci | 77 ++++++++++++++++++++++ > 1 file changed, 77 insertions(+) > create mode 100644 scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci > > diff --git a/scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci b/scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci > new file mode 100644 > index 0000000..b843392 > --- /dev/null > +++ b/scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci > @@ -0,0 +1,77 @@ > +/// Unsigned expressions cannot be lesser than zero. Presence of > +/// comparisons 'unsigned (<|<=|>|>=) 0' often indicates a bug, > +/// usually wrong type of variable. > +/// > +/// To reduce number of false positives following tests have been added: > +/// - parts of range checks are skipped, eg. "if (u < 0 || u > 15) ...", > +/// developers prefer to keep such code, > +/// - comparisons "<= 0" and "> 0" are performed only on results of > +/// signed functions/macros, > +/// - hardcoded list of signed functions/macros with always non-negative > +/// result is used to avoid false positives difficult to detect by other ways > +/// > +// Confidence: Average > +// Copyright: (C) 2015 Andrzej Hajda, Samsung Electronics Co., Ltd. GPLv2. > +// URL: http://coccinelle.lip6.fr/ > +// Options: --all-includes > + > +virtual context > +virtual org > +virtual report > + > +@r_cmp@ > +position p; > +typedef bool, u8, u16, u32, u64; > +{unsigned char, unsigned short, unsigned int, unsigned long, unsigned long long, > + size_t, bool, u8, u16, u32, u64} v; > +expression e; > +@@ > + \( v = e \| &v \) > + ... > + (\( v@p < 0 \| v@p <= 0 \| v@p >= 0 \| v@p > 0 \)) > + > +@r@ > +position r_cmp.p; > +typedef s8, s16, s32, s64; > +{char, short, int, long, long long, ssize_t, s8, s16, s32, s64} vs; > +expression c, e, v; > +identifier f !~ "^(ata_id_queue_depth|btrfs_copy_from_user|dma_map_sg|dma_map_sg_attrs|fls|fls64|gameport_time|get_write_extents|nla_len|ntoh24|of_flat_dt_match|of_get_child_count|uart_circ_chars_pending|[A-Z0-9_]+)$"; > +@@ > + > +( > + ... > +( > + (\( v@p < 0 \| v@p <= 0 \)) || ... || (\( v >= c \| v > c \)) > +| > + (\( v >= c \| v > c \)) || ... || (\( v@p < 0 \| v@p <= 0 \)) > +| > + (\( v@p >= 0 \| v@p > 0 \)) && ... && (\( v < c \| v <= c \)) > +| > + ((\( v < c \| v <= c \) && ... && \( v@p >= 0 \| v@p > 0 \))) > +| > +* (\( v@p <@e 0 \| v@p >=@e 0 \)) > +) > + ... > +| > + v = f(...)@vs; > + ... when != v = e; > +* (\( v@p <=@e 0 \| v@p >@e 0 \)) > + ... > +) > + > +@script:python depends on org@ > +p << r_cmp.p; > +e << r.e = ""; > +@@ > + > +msg = "WARNING: Unsigned expression compared with zero: %s" % (e) > +coccilib.org.print_todo(p[0], msg) > + > +@script:python depends on report@ > +p << r_cmp.p; > +e << r.e = ""; > +@@ > + > +msg = "WARNING: Unsigned expression compared with zero: %s" % (e) > +if e: > + coccilib.report.print_report(p[0], msg) > -- > 1.9.1 > > From mboxrd@z Thu Jan 1 00:00:00 1970 From: julia.lawall@lip6.fr (Julia Lawall) Date: Wed, 30 Dec 2015 15:06:29 +0100 (CET) Subject: [Cocci] [PATCH v5] coccinelle: tests: unsigned value cannot be lesser than zero In-Reply-To: <1451481943-20282-1-git-send-email-a.hajda@samsung.com> References: <5683CF1A.1030109@samsung.com> <1451481943-20282-1-git-send-email-a.hajda@samsung.com> Message-ID: To: cocci@systeme.lip6.fr List-Id: cocci@systeme.lip6.fr I would suggest to change rule r to the following. You had ... before and after the big disjunction with the ||s, but that will limit the detection of the problem to functions that contain only one occurrence. ( v = f(...)@vs; ... when != v = e; * (\( v at p <=@e 0 \| v@p >@e 0 \)) ... when any | ( (\( v at p < 0 \| v@p <= 0 \)) || ... || (\( v >= c \| v > c \)) | (\( v >= c \| v > c \)) || ... || (\( v at p < 0 \| v at p <= 0 \)) | (\( v@p >= 0 \| v at p > 0 \)) && ... && (\( v < c \| v <= c \)) | ((\( v < c \| v <= c \) && ... && \( v@p >= 0 \| v at p > 0 \))) | * (\( v at p <@e 0 \| v@p >=@e 0 \)) ) ) julia On Wed, 30 Dec 2015, Andrzej Hajda wrote: > Unsigned expressions cannot be lesser than zero. Presence of comparisons > 'unsigned (<|<=|>|>=) 0' often indicates a bug, usually wrong type of variable. > The patch beside finding such comparisons tries to eliminate false positives, > mainly by bypassing range checks. > > gcc can detect such comparisons also using -Wtype-limits switch, but it warns > also in correct cases, making too much noise. > > Signed-off-by: Andrzej Hajda > --- > v5: improved range check detection > v4: added range check detection, added full check in case value holds a result > of signed function > v3: added bool type > v2: added --all-includes option > --- > .../tests/unsigned_lesser_than_zero.cocci | 77 ++++++++++++++++++++++ > 1 file changed, 77 insertions(+) > create mode 100644 scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci > > diff --git a/scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci b/scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci > new file mode 100644 > index 0000000..b843392 > --- /dev/null > +++ b/scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci > @@ -0,0 +1,77 @@ > +/// Unsigned expressions cannot be lesser than zero. Presence of > +/// comparisons 'unsigned (<|<=|>|>=) 0' often indicates a bug, > +/// usually wrong type of variable. > +/// > +/// To reduce number of false positives following tests have been added: > +/// - parts of range checks are skipped, eg. "if (u < 0 || u > 15) ...", > +/// developers prefer to keep such code, > +/// - comparisons "<= 0" and "> 0" are performed only on results of > +/// signed functions/macros, > +/// - hardcoded list of signed functions/macros with always non-negative > +/// result is used to avoid false positives difficult to detect by other ways > +/// > +// Confidence: Average > +// Copyright: (C) 2015 Andrzej Hajda, Samsung Electronics Co., Ltd. GPLv2. > +// URL: http://coccinelle.lip6.fr/ > +// Options: --all-includes > + > +virtual context > +virtual org > +virtual report > + > + at r_cmp@ > +position p; > +typedef bool, u8, u16, u32, u64; > +{unsigned char, unsigned short, unsigned int, unsigned long, unsigned long long, > + size_t, bool, u8, u16, u32, u64} v; > +expression e; > +@@ > + \( v = e \| &v \) > + ... > + (\( v at p < 0 \| v at p <= 0 \| v@p >= 0 \| v at p > 0 \)) > + > + at r@ > +position r_cmp.p; > +typedef s8, s16, s32, s64; > +{char, short, int, long, long long, ssize_t, s8, s16, s32, s64} vs; > +expression c, e, v; > +identifier f !~ "^(ata_id_queue_depth|btrfs_copy_from_user|dma_map_sg|dma_map_sg_attrs|fls|fls64|gameport_time|get_write_extents|nla_len|ntoh24|of_flat_dt_match|of_get_child_count|uart_circ_chars_pending|[A-Z0-9_]+)$"; > +@@ > + > +( > + ... > +( > + (\( v at p < 0 \| v@p <= 0 \)) || ... || (\( v >= c \| v > c \)) > +| > + (\( v >= c \| v > c \)) || ... || (\( v at p < 0 \| v@p <= 0 \)) > +| > + (\( v at p >= 0 \| v at p > 0 \)) && ... && (\( v < c \| v <= c \)) > +| > + ((\( v < c \| v <= c \) && ... && \( v@p >= 0 \| v at p > 0 \))) > +| > +* (\( v at p <@e 0 \| v@p >=@e 0 \)) > +) > + ... > +| > + v = f(...)@vs; > + ... when != v = e; > +* (\( v at p <=@e 0 \| v@p >@e 0 \)) > + ... > +) > + > + at script:python depends on org@ > +p << r_cmp.p; > +e << r.e = ""; > +@@ > + > +msg = "WARNING: Unsigned expression compared with zero: %s" % (e) > +coccilib.org.print_todo(p[0], msg) > + > + at script:python depends on report@ > +p << r_cmp.p; > +e << r.e = ""; > +@@ > + > +msg = "WARNING: Unsigned expression compared with zero: %s" % (e) > +if e: > + coccilib.report.print_report(p[0], msg) > -- > 1.9.1 > >