Currently, during simplification, when it's checked if the shift
amount of a shift instruction is in range, it's not the
instruction's size (which is the same as the type's width)
that is used but the 'operand size' as returned by operand_size().
operand_size() is a bit like poor man's Value Range Propagation
or VRP without Propagation, and use the knowledge of previous
instructions to deduce the effective size of the operand.
For example, if the operand is the result of a narrowing cast
or have been ANDed with a known mask, or was a bitfield, the
size returned is the one of the cast or the mask and may be
smaller than its type.
A priori, using more precise knowledge is a good thing but in
the present case it's not because it causes warning for things
that are totally legal, meaningfull and used all the time.
For example, we don't want to warn in the following code:
struct bf { int u:8; };
int bf(struct bf *p) { return p->s << 24; }
Another situation where such warnings are not desirable is
when the shift instruction 'is far' from the one defining
the size, for example when the shift occurs in an inline function
and this inline function is called with a smaller type.
So, use the instruction size instead of the effective operand size
when checking the range of a shift instruction.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@xxxxxxxxx>
---
simplify.c | 4 +--
validation/shift-undef.c | 57 +++++++++++++++++++++++++++++++---------
2 files changed, 47 insertions(+), 14 deletions(-)
diff --git a/simplify.c b/simplify.c
index 3b8e22c54..19ff47a32 100644
--- a/simplify.c
+++ b/simplify.c
@@ -547,9 +547,9 @@ static int simplify_shift(struct instruction *insn, pseudo_t pseudo, long long v
if (!value)
return replace_with_pseudo(insn, pseudo);
- size = operand_size(insn, pseudo);
+ size = insn->size;
if (value >= size && !insn->tainted) {
- warning(insn->pos, "right shift by bigger than source value");
+ warning(insn->pos, "shift by bigger than operand's width");
insn->tainted = 1;
}
switch (insn->opcode) {
diff --git a/validation/shift-undef.c b/validation/shift-undef.c
index aba226c19..bc4b2b847 100644
--- a/validation/shift-undef.c
+++ b/validation/shift-undef.c
@@ -74,6 +74,39 @@ int ok(int s, unsigned int u, int p)
return 1;
}
+struct bf {
+ unsigned int u:8;
+ int s:8;
+};
+
+int bf(struct bf *p)
+{
+ unsigned int r = 0;
+ r += p->s << 8;
+ r += p->s >> 8;
+ r += p->u >> 8;
+ return r;
+}
+
+/*
+ * The following is used in the kernel at several places
+ * It shouldn't emit any warnings.
+ */
+typedef unsigned long long u64;
+typedef unsigned int u32;
+
+extern void hw_w32x2(u32 hi, u32 lo);
+
+inline void hw_w64(u64 val)
+{
+ hw_w32x2(val >> 32, (u32) val);
+}
+
+void hw_write(u32 val)
+{
+ hw_w64(val);
+}
+
/*
* check-name: shift too big or negative
* check-command: sparse -Wno-decl $file
@@ -97,12 +130,12 @@ shift-undef.c:17:30: warning: shift too big (108) for type unsigned int
shift-undef.c:18:30: warning: shift too big (4294967289) for type int
shift-undef.c:19:30: warning: shift too big (4294967288) for type unsigned int
shift-undef.c:20:30: warning: shift too big (4294967287) for type unsigned int
-shift-undef.c:21:29: warning: right shift by bigger than source value
-shift-undef.c:22:29: warning: right shift by bigger than source value
-shift-undef.c:23:29: warning: right shift by bigger than source value
-shift-undef.c:24:29: warning: right shift by bigger than source value
-shift-undef.c:25:29: warning: right shift by bigger than source value
-shift-undef.c:26:29: warning: right shift by bigger than source value
+shift-undef.c:21:29: warning: shift by bigger than operand's width
+shift-undef.c:22:29: warning: shift by bigger than operand's width
+shift-undef.c:23:29: warning: shift by bigger than operand's width
+shift-undef.c:24:29: warning: shift by bigger than operand's width
+shift-undef.c:25:29: warning: shift by bigger than operand's width
+shift-undef.c:26:29: warning: shift by bigger than operand's width
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
@@ -121,11 +154,11 @@ shift-undef.c:46:30: warning: shift too big (108) for type unsigned int
shift-undef.c:47:30: warning: shift too big (4294967289) for type int
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:50:26: warning: right shift by bigger than source value
-shift-undef.c:51:26: warning: right shift by bigger than source value
-shift-undef.c:52:26: warning: right shift by bigger than source value
-shift-undef.c:53:26: warning: right shift by bigger than source value
-shift-undef.c:54:26: warning: right shift by bigger than source value
-shift-undef.c:55:26: warning: right shift by bigger than source value
+shift-undef.c:50:26: warning: shift by bigger than operand's width
+shift-undef.c:51:26: warning: shift by bigger than operand's width
+shift-undef.c:52:26: warning: shift by bigger than operand's width
+shift-undef.c:53:26: warning: shift by bigger than operand's width
+shift-undef.c:54:26: warning: shift by bigger than operand's width
+shift-undef.c:55:26: warning: shift by bigger than operand's width
* check-error-end
*/
--
2.18.0
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
