[PATCH] warn when zero-extending a negation

[Luc Van Oostenryck <luc.vanoostenryck@gmail.com>]

When an unsigned value is negated (logical or arithmetical) and
then converted to a wider type, this value will be zero-extended,
not sign-extended. In other words, upper bits won't be negated.

This may be the intention but may also be a source of errors.

So, add a warning for this. Also, because this warning may be too
noise because most catches will possibly be false-positives, add a
specific warning flag to disable it: -Wno-zero-extend-negation.

Link: https://lore.kernel.org/r/CAHk-=wjiC6UejP6xob9BMQy98O6OLGDhy-qDfaFcOJxo90iOFg@mail.gmail.com
Originally-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---

On my usual test setup (defconfig + allyesconfig) this gives 199 warnings.
I only checked a couple of them, they we're false positives but somehow
error-prone if some definitions are changed. For example:
* struct super_block::s_flags is defined as 'unsigned long', all flags
  are hold in 32-bits but struct fs_context::sb_flags_mask is defined
  as 'unsigned int'.
* struct inode::i_stat is defined as 'unsigned long', all I_* are defined
  as (signed) 'int' but some code do 'unsigned dirty = I_DIRTY;'

For the moment, I've left the warning enabled by default but it should
probably only be enabled at W=1.


@Linus,

I suppose that it is fine for you that I your SoB instead of the
'Originally-by' I used here?

-- Luc


 linearize.c | 25 +++++++++++++++++++++++++
 options.c   |  2 ++
 options.h   |  1 +
 sparse.1    |  8 ++++++++
 4 files changed, 36 insertions(+)

diff --git a/linearize.c b/linearize.c
index 0250c6bb17ef..b9faac78ebb7 100644
--- a/linearize.c
+++ b/linearize.c
@@ -2520,6 +2520,27 @@ static void check_tainted_insn(struct instruction *insn)
 	}
 }
 
+static void check_zero_extend(struct instruction *insn)
+{
+	struct instruction *def;
+	pseudo_t src = insn->src1;
+
+	if (!Wzero_extend_negation)
+		return;
+	if (src->type != PSEUDO_REG)
+		return;
+	def = src->def;
+	if (!def)
+		return;
+	switch (def->opcode) {
+	case OP_NEG: case OP_NOT:
+		warning(insn->pos, "zero-extending a negation - upper bits not negated");
+		break;
+	default:
+		break;
+	}
+}
+
 ///
 // issue warnings after all possible DCE
 static void late_warnings(struct entrypoint *ep)
@@ -2537,6 +2558,10 @@ static void late_warnings(struct entrypoint *ep)
 				// Check for illegal offsets.
 				check_access(insn);
 				break;
+			case OP_ZEXT:
+				// Check for missing sign extension..
+				check_zero_extend(insn);
+				break;
 			}
 		} END_FOR_EACH_PTR(insn);
 	} END_FOR_EACH_PTR(bb);
diff --git a/options.c b/options.c
index 17da5f367e24..5323ddc05861 100644
--- a/options.c
+++ b/options.c
@@ -139,6 +139,7 @@ int Wunion_cast = 0;
 int Wuniversal_initializer = 0;
 int Wunknown_attribute = 0;
 int Wvla = 1;
+int Wzero_extend_negation = 1;
 
 ////////////////////////////////////////////////////////////////////////////////
 // Helpers for option parsing
@@ -884,6 +885,7 @@ static const struct flag warnings[] = {
 	{ "universal-initializer", &Wuniversal_initializer },
 	{ "unknown-attribute", &Wunknown_attribute },
 	{ "vla", &Wvla },
+	{ "zero-extend-negation", &Wzero_extend_negation },
 	{ }
 };
 
diff --git a/options.h b/options.h
index 0aec8764d27d..3403c9518ead 100644
--- a/options.h
+++ b/options.h
@@ -138,6 +138,7 @@ extern int Wunion_cast;
 extern int Wuniversal_initializer;
 extern int Wunknown_attribute;
 extern int Wvla;
+extern int Wzero_extend_negation;
 
 extern char **handle_switch(char *arg, char **next);
 extern void handle_switch_finalize(void);
diff --git a/sparse.1 b/sparse.1
index 430b3710b260..928e3513b9b6 100644
--- a/sparse.1
+++ b/sparse.1
@@ -494,6 +494,14 @@ Warn on casts to union types.
 
 Sparse does not issues these warnings by default.
 .
+.TP
+.B -Wzero-extend-negation
+Warn when an unsigned value is first negated (logical or arithmetical)
+and then converted to a wider type.
+
+Sparse issues these warnings by default.
+To turn them off, use \fB-Wno-zero-extend-negation\fR.
+.
 .SH MISC OPTIONS
 .TP
 .B \-\-arch=\fIARCH\fR
-- 
2.29.2