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@xxxxxxxxxxxxxx Originally-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@xxxxxxxxx> --- 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
