[PATCH] D10634: Loss of Sign Checker

[Daniel Marjamäki]

danielmarjamaki added a comment.

Here are some warnings.. could you please look at some of them...

There are a few cases where RHS is a constant integer literal.. do you think there could be a bug in some of them?

ftp://ftp.sunet.se/pub/Linux/distributions/Debian/debian/pool/main/a/abcm2ps/abcm2ps_7.8.9.orig.tar.gz
clang-tidy abcm2ps-7.8.9/deco.c
deco.c:788:9: warning: assigning negative value to unsigned variable loses sign and may cause undesired runtime behavior [clang-analyzer-alpha.core.LossOfSignAssign]

  ps_x = -1;

ftp://ftp.sunet.se/pub/Linux/distributions/Debian/debian/pool/main/a/adolc/adolc_2.5.2.orig.tar.gz
clang-tidy ADOL-C-2.5.2/ADOL-C/src/taping.c
taping.c:914:12: warning: assigning negative value to unsigned variable loses sign and may cause undesired runtime behavior [clang-analyzer-alpha.core.LossOfSignAssign]

  number = lastTayP1 - ADOLC_CURRENT_TAPE_INFOS.tayBuffer;

ftp://ftp.sunet.se/pub/Linux/distributions/Debian/debian/pool/main/a/alsa-lib/alsa-lib_1.0.28.orig.tar.bz2
clang-tidy alsa-lib-1.0.28/src/pcm/pcm_dsnoop.c
pcm_dsnoop.c:60:20: warning: assigning negative value to unsigned variable loses sign and may cause undesired runtime behavior [clang-analyzer-alpha.core.LossOfSignAssign]

  snd_pcm_uframes_t ptr1 = -2LL /* invalid value */, ptr2;

ftp://ftp.sunet.se/pub/Linux/distributions/Debian/debian/pool/main/a/anubis/anubis_4.1.1+dfsg1.orig.tar.gz
clang-tidy anubis-4.1.1+dfsg1/src/rcfile-lex.c
rcfile-lex.c:1482:3: warning: assigning negative value to unsigned variable loses sign and may cause undesired runtime behavior [clang-analyzer-alpha.core.LossOfSignAssign]

  rc_yy_size_t new_size = (rc_yy_n_chars) + number_to_move + ((rc_yy_n_chars) >> 1);

ftp://ftp.sunet.se/pub/Linux/distributions/Debian/debian/pool/main/a/atheme-services/atheme-services_7.0.7.orig.tar.bz2
clang-tidy atheme-services-7.0.7/modules/groupserv/main/database.c
database.c:9:21: warning: assigning negative value to unsigned variable loses sign and may cause undesired runtime behavior [clang-analyzer-alpha.core.LossOfSignAssign]
static unsigned int loading_gdbv = -1;

clang-tidy atheme-services-7.0.7/libathemecore/reslib.c
reslib.c:1093:13: warning: assigning negative value to unsigned variable loses sign and may cause undesired runtime behavior [clang-analyzer-alpha.core.LossOfSignAssign]

  n = labellen(cp - 1); /* XXX */

clang-tidy atheme-services-7.0.7/libmowgli-2/src/libmowgli/dns/dns_evloop_reslib.c
dns_evloop_reslib.c:1011:8: warning: assigning negative value to unsigned variable loses sign and may cause undesired runtime behavior [clang-analyzer-alpha.core.LossOfSignAssign]

  n = labellen(cp - 1);   /* XXX */

clang-tidy avfs-1.0.1/src/sysdeps.c
sysdeps.c:251:5: warning: assigning negative value to unsigned variable loses sign and may cause undesired runtime behavior [clang-analyzer-alpha.core.LossOfSignAssign]

  static avuid_t myuid = -1;
  ^

sysdeps.c:251:5: note: assigning negative value to unsigned variable loses sign and may cause undesired runtime behavior

  static avuid_t myuid = -1;
  ^

sysdeps.c:252:5: warning: assigning negative value to unsigned variable loses sign and may cause undesired runtime behavior [clang-analyzer-alpha.core.LossOfSignAssign]

  static avuid_t mygid = -1;
  ^

sysdeps.c:252:5: note: assigning negative value to unsigned variable loses sign and may cause undesired runtime behavior

  static avuid_t mygid = -1;
  ^

sysdeps.c:251:5: warning: assigning negative value to unsigned variable loses sign and may cause undesired runtime behavior [clang-analyzer-alpha.core.LossOfSignAssign]

  static avuid_t myuid = -1;
  ^

sysdeps.c:251:5: note: assigning negative value to unsigned variable loses sign and may cause undesired runtime behavior

  static avuid_t myuid = -1;
  ^

sysdeps.c:252:5: warning: assigning negative value to unsigned variable loses sign and may cause undesired runtime behavior [clang-analyzer-alpha.core.LossOfSignAssign]

  static avuid_t mygid = -1;
  ^

sysdeps.c:252:5: note: assigning negative value to unsigned variable loses sign and may cause undesired runtime behavior

  static avuid_t mygid = -1;
  ^

clang-tidy avfs-1.0.1/zlib/deflate.c
deflate.c:1033:11: warning: assigning negative value to unsigned variable loses sign and may cause undesired runtime behavior [clang-analyzer-alpha.core.LossOfSignAssign]

  n = read_buf(s->strm, s->window + s->strstart + s->lookahead, more);


================
Comment at: lib/StaticAnalyzer/Checkers/Checkers.td:91
@@ -90,1 +90,3 @@
 
+def LossOfSignChecker : Checker<"LossOfSignAssign">,
+  HelpText<"Warn about (possible) loss of sign in assignments and initializations">,
----------------
Please put this in alphabetical order somewhat.

================
Comment at: lib/StaticAnalyzer/Checkers/LossOfSignChecker.cpp:71
@@ +70,3 @@
+                                                   CheckerContext &C) const {
+  ProgramStateRef state = C.getState(); 
+  SValBuilder &svalBuilder = C.getSValBuilder();
----------------
danielmarjamaki wrote:
> Variables should start with a uppercase letter. And abbreviations are preferred.
I looked in the llvm coding style guide. It does not say that abbreviations are preferred as far as I see. Ignore my comment about that.

================
Comment at: lib/StaticAnalyzer/Checkers/LossOfSignChecker.cpp:88
@@ +87,3 @@
+  //
+  // check for negative value.
+  //
----------------
Comments should start with uppercase


================
Comment at: lib/StaticAnalyzer/Checkers/LossOfSignChecker.cpp:93
@@ +92,3 @@
+  // Cast the RHS value to the correct (original) type that it appeared in.
+  SVal castVal = SValBldr.evalCast(Val, RHSTy, VarTy);
+
----------------
you missed this variable, use uppercase for the first character.


http://reviews.llvm.org/D10634

http://reviews.llvm.org/settings/panel/emailpreferences/