From: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
To: linux-sparse@vger.kernel.org
Cc: Stafford Horne <shorne@gmail.com>,
Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Subject: [PATCH 4/4] bad-shift: wait dead code elimination to warn about bad shifts
Date: Thu, 6 Aug 2020 21:30:03 +0200 [thread overview]
Message-ID: <20200806193003.10144-5-luc.vanoostenryck@gmail.com> (raw)
In-Reply-To: <20200806193003.10144-1-luc.vanoostenryck@gmail.com>
Sparse complains when a shift amount is too big for the size
of its operand or if it's negative.
However, it does this even for expressions that are never evaluated.
It's especially annoying in the kernel for type generic macros,
for example the ones in arch/*/include/asm/cmpxchg.h
So, remove all warnings done at expansion time and avoid any
simplifications of such expressions. Same, at linearization
and optimization time but in this case mark the instructions as
'tainted' to inhibit any further simplifications. Finally, at the
end of the optimization phase, warn for the tainted instructions.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
expand.c | 18 -------------
linearize.c | 44 ++++++++++++++++++++++++++++++++
simplify.c | 20 ++++-----------
validation/expand/bad-shift.c | 8 +++---
validation/optim/shift-big.c | 12 ++++++---
validation/shift-negative.c | 4 +--
validation/shift-undef-long.c | 7 +++--
validation/shift-undef.c | 48 +++++++++--------------------------
8 files changed, 78 insertions(+), 83 deletions(-)
diff --git a/expand.c b/expand.c
index b07893318382..623b180025ad 100644
--- a/expand.c
+++ b/expand.c
@@ -170,22 +170,6 @@ Float:
expr->type = EXPR_FVALUE;
}
-static void warn_shift_count(struct expression *expr, struct symbol *ctype, long long count)
-{
- if (count < 0) {
- if (!Wshift_count_negative)
- return;
- warning(expr->pos, "shift count is negative (%lld)", count);
- return;
- }
- if (ctype->type == SYM_NODE)
- ctype = ctype->ctype.base_type;
-
- if (!Wshift_count_overflow)
- return;
- warning(expr->pos, "shift too big (%llu) for type %s", count, show_typename(ctype));
-}
-
/* Return true if constant shift size is valid */
static bool check_shift_count(struct expression *expr, struct expression *right)
{
@@ -194,8 +178,6 @@ static bool check_shift_count(struct expression *expr, struct expression *right)
if (count >= 0 && count < ctype->bit_size)
return true;
- if (!conservative)
- warn_shift_count(expr, ctype, count);
return false;
}
diff --git a/linearize.c b/linearize.c
index 4927468183b0..5a8e74970d98 100644
--- a/linearize.c
+++ b/linearize.c
@@ -2468,6 +2468,49 @@ static pseudo_t linearize_statement(struct entrypoint *ep, struct statement *stm
return VOID;
}
+static void check_tainted_insn(struct instruction *insn)
+{
+ unsigned long long uval;
+ long long sval;
+ pseudo_t src2;
+
+ switch (insn->opcode) {
+ case OP_DIVU: case OP_DIVS:
+ case OP_MODU: case OP_MODS:
+ if (insn->src2 == value_pseudo(0))
+ warning(insn->pos, "divide by zero");
+ break;
+ case OP_SHL: case OP_LSR: case OP_ASR:
+ src2 = insn->src2;
+ if (src2->type != PSEUDO_VAL)
+ break;
+ uval = src2->value;
+ if (uval < insn->size)
+ break;
+ sval = sign_extend(uval, insn->size);
+ if (Wshift_count_negative && sval < 0)
+ warning(insn->pos, "shift count is negative (%lld)", sval);
+ else if (Wshift_count_overflow)
+ warning(insn->pos, "shift too big (%llu) for type %s", uval, show_typename(insn->type));
+ }
+}
+
+///
+// issue warnings after all possible DCE
+static void late_warnings(struct entrypoint *ep)
+{
+ struct basic_block *bb;
+ FOR_EACH_PTR(ep->bbs, bb) {
+ struct instruction *insn;
+ FOR_EACH_PTR(bb->insns, insn) {
+ if (!insn->bb)
+ continue;
+ if (insn->tainted)
+ check_tainted_insn(insn);
+ } END_FOR_EACH_PTR(insn);
+ } END_FOR_EACH_PTR(bb);
+}
+
static struct entrypoint *linearize_fn(struct symbol *sym, struct symbol *base_type)
{
struct statement *stmt = base_type->stmt;
@@ -2514,6 +2557,7 @@ static struct entrypoint *linearize_fn(struct symbol *sym, struct symbol *base_t
add_one_insn(ep, ret);
optimize(ep);
+ late_warnings(ep);
return ep;
}
diff --git a/simplify.c b/simplify.c
index 7850bcdc6069..f6b79685f439 100644
--- a/simplify.c
+++ b/simplify.c
@@ -754,28 +754,18 @@ static long long check_shift_count(struct instruction *insn, unsigned long long
unsigned int size = insn->size;
long long sval = uval;
+ if (insn->tainted)
+ return -1;
+
if (uval < size)
return uval;
+ insn->tainted = 1;
sval = sign_extend_safe(sval, size);
sval = sign_extend_safe(sval, bits_in_int);
if (sval < 0)
insn->src2 = value_pseudo(sval);
- if (insn->tainted)
- return sval;
-
- if (sval < 0 && Wshift_count_negative)
- warning(insn->pos, "shift count is negative (%lld)", sval);
- if (sval > 0 && Wshift_count_overflow) {
- struct symbol *ctype = insn->type;
- const char *tname;
- if (ctype->type == SYM_NODE)
- ctype = ctype->ctype.base_type;
- tname = show_typename(ctype);
- warning(insn->pos, "shift too big (%llu) for type %s", sval, tname);
- }
- insn->tainted = 1;
- return sval;
+ return -1;
}
static int simplify_shift(struct instruction *insn, pseudo_t pseudo, long long value)
diff --git a/validation/expand/bad-shift.c b/validation/expand/bad-shift.c
index 22c4341f1680..b68866c2b048 100644
--- a/validation/expand/bad-shift.c
+++ b/validation/expand/bad-shift.c
@@ -56,9 +56,9 @@ rneg:
* check-output-end
*
* check-error-start
-expand/bad-shift.c:5:18: warning: shift too big (32) for type int
-expand/bad-shift.c:10:18: warning: shift count is negative (-1)
-expand/bad-shift.c:15:18: warning: shift too big (32) for type int
-expand/bad-shift.c:20:18: warning: shift count is negative (-1)
+expand/bad-shift.c:5:21: warning: shift too big (32) for type int
+expand/bad-shift.c:10:21: warning: shift count is negative (-1)
+expand/bad-shift.c:15:21: warning: shift too big (32) for type int
+expand/bad-shift.c:20:21: warning: shift count is negative (-1)
* check-error-end
*/
diff --git a/validation/optim/shift-big.c b/validation/optim/shift-big.c
index 84bcd2ce01a3..e7bf22fe36ff 100644
--- a/validation/optim/shift-big.c
+++ b/validation/optim/shift-big.c
@@ -50,13 +50,15 @@ lsr31:
lsr32:
.L8:
<entry-point>
- ret.32 $0
+ lsr.32 %r14 <- %arg1, $32
+ ret.32 %r14
lsr33:
.L10:
<entry-point>
- ret.32 $0
+ lsr.32 %r17 <- %arg1, $33
+ ret.32 %r17
shl31:
@@ -69,13 +71,15 @@ shl31:
shl32:
.L14:
<entry-point>
- ret.32 $0
+ shl.32 %r23 <- %arg1, $32
+ ret.32 %r23
shl33:
.L16:
<entry-point>
- ret.32 $0
+ shl.32 %r26 <- %arg1, $33
+ ret.32 %r26
* check-output-end
diff --git a/validation/shift-negative.c b/validation/shift-negative.c
index fff5cf123dd6..6df02b18468f 100644
--- a/validation/shift-negative.c
+++ b/validation/shift-negative.c
@@ -9,8 +9,8 @@ unsigned int fo2(unsigned int a) { return a >> ((a & 0) ^ ~0); }
* check-command: sparse -Wno-decl $file
*
* check-error-start
-shift-negative.c:1:45: warning: shift count is negative (-1)
-shift-negative.c:2:45: warning: shift count is negative (-1)
+shift-negative.c:1:48: warning: shift count is negative (-1)
+shift-negative.c:2:48: warning: shift count is negative (-1)
shift-negative.c:4:59: warning: shift count is negative (-1)
shift-negative.c:5:59: warning: shift count is negative (-1)
* check-error-end
diff --git a/validation/shift-undef-long.c b/validation/shift-undef-long.c
index 326267436ec7..985fe4c4c595 100644
--- a/validation/shift-undef-long.c
+++ b/validation/shift-undef-long.c
@@ -13,9 +13,8 @@ static unsigned very_big_shift(unsigned int a)
* check-command: sparse -m64 $file
*
* check-error-start
-shift-undef-long.c:4:16: warning: shift too big (4294967295) for type unsigned int
-shift-undef-long.c:5:16: warning: shift too big (4294967296) for type unsigned int
-shift-undef-long.c:6:16: warning: shift too big (4294967296) for type unsigned int
-shift-undef-long.c:7:16: warning: shift count is negative (-4294967296)
+shift-undef-long.c:4:25: warning: shift count is negative (-1)
+shift-undef-long.c:5:47: warning: shift too big (4294967296) for type unsigned int
+shift-undef-long.c:7:20: warning: shift count is negative (-4294967296)
* check-error-end
*/
diff --git a/validation/shift-undef.c b/validation/shift-undef.c
index 613c6b95b113..0c7541e9ffd9 100644
--- a/validation/shift-undef.c
+++ b/validation/shift-undef.c
@@ -112,48 +112,24 @@ void hw_write(u32 val)
* check-command: sparse -Wno-decl $file
*
* check-error-start
-shift-undef.c:3:15: warning: shift too big (100) for type int
-shift-undef.c:4:15: warning: shift too big (101) for type unsigned int
-shift-undef.c:5:15: warning: shift too big (102) for type unsigned int
-shift-undef.c:6:15: warning: shift count is negative (-1)
-shift-undef.c:7:15: warning: shift count is negative (-2)
-shift-undef.c:8:15: warning: shift count is negative (-3)
-shift-undef.c:9:25: warning: shift too big (103) for type int
-shift-undef.c:10:25: warning: shift too big (104) for type unsigned int
-shift-undef.c:11:25: warning: shift too big (105) for type unsigned int
-shift-undef.c:12:25: warning: shift count is negative (-4)
-shift-undef.c:13:25: warning: shift count is negative (-5)
-shift-undef.c:14:25: warning: shift count is negative (-6)
-shift-undef.c:15:30: warning: shift too big (106) for type int
-shift-undef.c:16:30: warning: shift too big (107) for type unsigned int
-shift-undef.c:17:30: warning: shift too big (108) for type unsigned int
-shift-undef.c:18:30: warning: shift count is negative (-7)
-shift-undef.c:19:30: warning: shift count is negative (-8)
-shift-undef.c:20:30: warning: shift count is negative (-9)
+shift-undef.c:3:18: warning: shift too big (100) for type int
+shift-undef.c:4:18: warning: shift too big (101) for type unsigned int
+shift-undef.c:5:18: warning: shift too big (102) for type unsigned int
+shift-undef.c:6:19: warning: shift count is negative (-1)
+shift-undef.c:7:19: warning: shift count is negative (-2)
+shift-undef.c:8:19: warning: shift count is negative (-3)
shift-undef.c:21:29: warning: shift too big (109) for type int
shift-undef.c:22:29: warning: shift too big (110) for type unsigned int
shift-undef.c:23:29: warning: shift too big (111) for type unsigned int
shift-undef.c:24:29: warning: shift count is negative (-10)
shift-undef.c:25:29: warning: shift count is negative (-11)
shift-undef.c:26:29: warning: shift count is negative (-12)
-shift-undef.c:32:11: warning: shift too big (100) for type int
-shift-undef.c:33:11: warning: shift too big (101) for type unsigned int
-shift-undef.c:34:11: warning: shift too big (102) for type unsigned int
-shift-undef.c:35:11: warning: shift count is negative (-1)
-shift-undef.c:36:11: warning: shift too big (4294967294) for type unsigned int
-shift-undef.c:37:11: warning: shift too big (4294967293) for type unsigned int
-shift-undef.c:38:25: warning: shift too big (103) for type int
-shift-undef.c:39:25: warning: shift too big (104) for type unsigned int
-shift-undef.c:40:25: warning: shift too big (105) for type unsigned int
-shift-undef.c:41:25: warning: shift count is negative (-4)
-shift-undef.c:42:25: warning: shift too big (4294967291) for type unsigned int
-shift-undef.c:43:25: warning: shift too big (4294967290) for type unsigned int
-shift-undef.c:44:30: warning: shift too big (106) for type int
-shift-undef.c:45:30: warning: shift too big (107) for type unsigned int
-shift-undef.c:46:30: warning: shift too big (108) for type unsigned int
-shift-undef.c:47:30: warning: shift count is negative (-7)
-shift-undef.c:48:30: warning: shift too big (4294967288) for type unsigned int
-shift-undef.c:49:30: warning: shift too big (4294967287) for type unsigned int
+shift-undef.c:32:15: warning: shift too big (100) for type int
+shift-undef.c:33:15: warning: shift too big (101) for type unsigned int
+shift-undef.c:34:15: warning: shift too big (102) for type unsigned int
+shift-undef.c:35:16: warning: shift count is negative (-1)
+shift-undef.c:36:16: warning: shift count is negative (-2)
+shift-undef.c:37:16: warning: shift count is negative (-3)
shift-undef.c:50:26: warning: shift too big (109) for type int
shift-undef.c:51:26: warning: shift too big (110) for type unsigned int
shift-undef.c:52:26: warning: shift too big (111) for type unsigned int
--
2.28.0
next prev parent reply other threads:[~2020-08-06 19:30 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-06 19:29 [PATCH 0/4] Fix shifts-assigns and avoid warns on deadcode Luc Van Oostenryck
2020-08-06 19:30 ` [PATCH 1/4] shift-assign: add more testcases for bogus linearization Luc Van Oostenryck
2020-08-06 19:30 ` [PATCH 2/4] shift-assign: fix linearization of shift-assign Luc Van Oostenryck
2020-08-06 19:30 ` [PATCH 3/4] shift-assign: restrict shift count to unsigned int Luc Van Oostenryck
2020-08-06 19:30 ` Luc Van Oostenryck [this message]
2020-08-15 9:57 ` [PATCH 4/4] bad-shift: wait dead code elimination to warn about bad shifts Stafford Horne
2020-08-15 11:15 ` Luc Van Oostenryck
2020-08-15 13:51 ` Stafford Horne
2020-08-15 9:59 ` [PATCH 0/4] Fix shifts-assigns and avoid warns on deadcode Stafford Horne
2020-08-15 11:03 ` 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=20200806193003.10144-5-luc.vanoostenryck@gmail.com \
--to=luc.vanoostenryck@gmail.com \
--cc=linux-sparse@vger.kernel.org \
--cc=shorne@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;
as well as URLs for NNTP newsgroup(s).