libtom / libtommath

LibTomMath is a free open source portable number theoretic multiple-precision integer library written entirely in C.
https://www.libtom.net
Other
645 stars 194 forks source link

clang-tidy warnings #317

Closed karel-m closed 5 years ago

karel-m commented 5 years ago

clang-tidy run win the same rule set as we use in libtomcrypt (and have no warnings):

bn_mp_cmp.c:13:9: error: do not use 'else' after 'return' [readability-else-after-return,-warnings-as-errors]
      } else {
        ^~~~~~
bn_mp_cmp.c:22:6: error: do not use 'else' after 'return' [readability-else-after-return,-warnings-as-errors]
   } else {
     ^~~~~~
bn_mp_cmp_d.c:22:6: error: do not use 'else' after 'return' [readability-else-after-return,-warnings-as-errors]
   } else if (a->dp[0] < b) {
     ^~~~~~~~
bn_mp_decr.c:13:6: error: do not use 'else' after 'return' [readability-else-after-return,-warnings-as-errors]
   } else if (a->sign == MP_NEG) {
     ^~~~~~~~
bn_mp_exptmod.c:81:6: error: do not use 'else' after 'return' [readability-else-after-return,-warnings-as-errors]
   } else {
     ^~~~~~
bn_mp_incr.c:12:6: error: do not use 'else' after 'return' [readability-else-after-return,-warnings-as-errors]
   } else if (a->sign == MP_NEG) {
     ^~~~~~~~
bn_mp_prime_is_prime.c:148:7: error: Value stored to 't' is never read [clang-analyzer-deadcode.DeadStores,-warnings-as-errors]
      t = -t;
      ^
bn_mp_prime_is_prime.c:148:7: note: Value stored to 't' is never read
bn_mp_prime_rabin_miller_trials.c:33:9: error: do not use 'else' after 'return' [readability-else-after-return,-warnings-as-errors]
      } else if (sizes[x].k > size) {
        ^~~~~~~~
bn_mp_prime_rand.c:71:17: error: The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage [clang-analyzer-core.uninitialized.Assign,-warnings-as-errors]
      tmp[0]    &= maskAND;
                ^
bn_mp_prime_rand.c:137:11: note: Calling 's_mp_prime_random_ex'
   return s_mp_prime_random_ex(a, t, size, flags, s_mp_rand_cb, NULL);
          ^
bn_mp_prime_rand.c:29:9: note: Assuming 'size' is > 1
   if ((size <= 1) || (t <= 0)) {
        ^
bn_mp_prime_rand.c:29:8: note: Left side of '||' is false
   if ((size <= 1) || (t <= 0)) {
       ^
bn_mp_prime_rand.c:29:24: note: Assuming 't' is > 0
   if ((size <= 1) || (t <= 0)) {
                       ^
bn_mp_prime_rand.c:29:4: note: Taking false branch
   if ((size <= 1) || (t <= 0)) {
   ^
bn_mp_prime_rand.c:34:8: note: Assuming the condition is false
   if ((flags & MP_PRIME_SAFE) != 0) {
       ^
bn_mp_prime_rand.c:34:4: note: Taking false branch
   if ((flags & MP_PRIME_SAFE) != 0) {
   ^
bn_mp_prime_rand.c:39:25: note: Assuming the condition is false
   bsize = (size>>3) + ((size&7)?1:0);
                        ^
bn_mp_prime_rand.c:39:25: note: '?' condition is false
bn_mp_prime_rand.c:42:28: note: Storing uninitialized value
   tmp = (unsigned char *) MP_MALLOC((size_t)bsize);
                           ^
../libtommath/tommath_private.h:131:46: note: expanded from macro 'MP_MALLOC'
#   define MP_MALLOC(size)                   malloc(size)
                                             ^
bn_mp_prime_rand.c:43:8: note: Assuming 'tmp' is not equal to NULL
   if (tmp == NULL) {
       ^
bn_mp_prime_rand.c:43:4: note: Taking false branch
   if (tmp == NULL) {
   ^
bn_mp_prime_rand.c:48:14: note: '?' condition is true
   maskAND = ((size&7) == 0) ? 0xFFu : (unsigned char)(0xFFu >> (8 - (size & 7)));
             ^
bn_mp_prime_rand.c:52:24: note: '?' condition is false
   maskOR_msb_offset = ((size & 7) == 1) ? 1 : 0;
                       ^
bn_mp_prime_rand.c:53:8: note: Assuming the condition is false
   if ((flags & MP_PRIME_2MSB_ON) != 0) {
       ^
bn_mp_prime_rand.c:53:4: note: Taking false branch
   if ((flags & MP_PRIME_2MSB_ON) != 0) {
   ^
bn_mp_prime_rand.c:59:8: note: Assuming the condition is false
   if ((flags & MP_PRIME_BBS) != 0) {
       ^
bn_mp_prime_rand.c:59:4: note: Taking false branch
   if ((flags & MP_PRIME_BBS) != 0) {
   ^
bn_mp_prime_rand.c:65:11: note: Calling 's_mp_rand_cb'
      if (cb(tmp, bsize, dat) != bsize) {
          ^
bn_mp_prime_rand.c:126:8: note: Assuming 'len' is <= 0
   if (len <= 0) {
       ^
bn_mp_prime_rand.c:126:4: note: Taking true branch
   if (len <= 0) {
   ^
bn_mp_prime_rand.c:65:11: note: Returning from 's_mp_rand_cb'
      if (cb(tmp, bsize, dat) != bsize) {
          ^
bn_mp_prime_rand.c:65:7: note: Taking false branch
      if (cb(tmp, bsize, dat) != bsize) {
      ^
bn_mp_prime_rand.c:71:17: note: The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage
      tmp[0]    &= maskAND;
                ^
bn_mp_prime_strong_lucas_selfridge.c:191:7: error: Value stored to 'Q' is never read [clang-analyzer-deadcode.DeadStores,-warnings-as-errors]
      Q = -Q;
      ^
bn_mp_prime_strong_lucas_selfridge.c:191:7: note: Value stored to 'Q' is never read
bn_mp_reduce.c:67:50: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
      if ((err = mp_lshd(&q, um + 1)) != MP_OKAY)
                                                 ^
                                                  {
bn_mp_reduce.c:69:47: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
      if ((err = mp_add(x, &q, x)) != MP_OKAY)
                                              ^
                                               {
bn_mp_reduce_is_2k.c:14:6: error: do not use 'else' after 'return' [readability-else-after-return,-warnings-as-errors]
   } else if (a->used == 1) {
     ^~~~~~~~
bn_mp_reduce_is_2k_l.c:13:6: error: do not use 'else' after 'return' [readability-else-after-return,-warnings-as-errors]
   } else if (a->used == 1) {
     ^~~~~~~~
bn_s_mp_invmod_slow.c:128:24: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (!MP_IS_ZERO(&u))
                       ^
                        {
bn_s_mp_karatsuba_mul.c:48:40: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_init_size(&x0, B) != MP_OKAY)
                                       ^
                                        {
bn_s_mp_karatsuba_mul.c:50:50: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_init_size(&x1, a->used - B) != MP_OKAY)
                                                 ^
                                                  {
bn_s_mp_karatsuba_mul.c:52:40: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_init_size(&y0, B) != MP_OKAY)
                                       ^
                                        {
bn_s_mp_karatsuba_mul.c:54:50: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_init_size(&y1, b->used - B) != MP_OKAY)
                                                 ^
                                                  {
bn_s_mp_karatsuba_mul.c:58:44: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_init_size(&t1, B * 2) != MP_OKAY)
                                           ^
                                            {
bn_s_mp_karatsuba_mul.c:60:46: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_init_size(&x0y0, B * 2) != MP_OKAY)
                                             ^
                                              {
bn_s_mp_karatsuba_mul.c:62:46: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_init_size(&x1y1, B * 2) != MP_OKAY)
                                             ^
                                              {
bn_s_mp_karatsuba_mul.c:106:43: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_mul(&x0, &y0, &x0y0) != MP_OKAY)
                                          ^
                                           {
bn_s_mp_karatsuba_mul.c:108:43: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_mul(&x1, &y1, &x1y1) != MP_OKAY)
                                          ^
                                           {
bn_s_mp_karatsuba_mul.c:112:43: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (s_mp_add(&x1, &x0, &t1) != MP_OKAY)
                                          ^
                                           {
bn_s_mp_karatsuba_mul.c:114:43: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (s_mp_add(&y1, &y0, &x0) != MP_OKAY)
                                          ^
                                           {
bn_s_mp_karatsuba_mul.c:116:41: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_mul(&t1, &x0, &t1) != MP_OKAY)
                                        ^
                                         {
bn_s_mp_karatsuba_mul.c:120:45: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_add(&x0y0, &x1y1, &x0) != MP_OKAY)
                                            ^
                                             {
bn_s_mp_karatsuba_mul.c:122:43: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (s_mp_sub(&t1, &x0, &t1) != MP_OKAY)
                                          ^
                                           {
bn_s_mp_karatsuba_mul.c:126:35: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_lshd(&t1, B) != MP_OKAY)
                                  ^
                                   {
bn_s_mp_karatsuba_mul.c:128:41: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_lshd(&x1y1, B * 2) != MP_OKAY)
                                        ^
                                         {
bn_s_mp_karatsuba_mul.c:131:43: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_add(&x0y0, &t1, &t1) != MP_OKAY)
                                          ^
                                           {
bn_s_mp_karatsuba_mul.c:133:41: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_add(&t1, &x1y1, c) != MP_OKAY)
                                        ^
                                         {
bn_s_mp_karatsuba_sqr.c:26:40: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_init_size(&x0, B) != MP_OKAY)
                                       ^
                                        {
bn_s_mp_karatsuba_sqr.c:28:50: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_init_size(&x1, a->used - B) != MP_OKAY)
                                                 ^
                                                  {
bn_s_mp_karatsuba_sqr.c:32:50: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_init_size(&t1, a->used * 2) != MP_OKAY)
                                                 ^
                                                  {
bn_s_mp_karatsuba_sqr.c:34:50: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_init_size(&t2, a->used * 2) != MP_OKAY)
                                                 ^
                                                  {
bn_s_mp_karatsuba_sqr.c:36:46: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_init_size(&x0x0, B * 2) != MP_OKAY)
                                             ^
                                              {
bn_s_mp_karatsuba_sqr.c:38:58: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_init_size(&x1x1, (a->used - B) * 2) != MP_OKAY)
                                                         ^
                                                          {
bn_s_mp_karatsuba_sqr.c:65:38: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_sqr(&x0, &x0x0) != MP_OKAY)
                                     ^
                                      {
bn_s_mp_karatsuba_sqr.c:67:38: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_sqr(&x1, &x1x1) != MP_OKAY)
                                     ^
                                      {
bn_s_mp_karatsuba_sqr.c:71:43: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (s_mp_add(&x1, &x0, &t1) != MP_OKAY)
                                          ^
                                           {
bn_s_mp_karatsuba_sqr.c:73:36: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_sqr(&t1, &t1) != MP_OKAY)
                                   ^
                                    {
bn_s_mp_karatsuba_sqr.c:77:47: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (s_mp_add(&x0x0, &x1x1, &t2) != MP_OKAY)
                                              ^
                                               {
bn_s_mp_karatsuba_sqr.c:79:43: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (s_mp_sub(&t1, &t2, &t1) != MP_OKAY)
                                          ^
                                           {
bn_s_mp_karatsuba_sqr.c:83:35: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_lshd(&t1, B) != MP_OKAY)
                                  ^
                                   {
bn_s_mp_karatsuba_sqr.c:85:41: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_lshd(&x1x1, B * 2) != MP_OKAY)
                                        ^
                                         {
bn_s_mp_karatsuba_sqr.c:88:43: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_add(&x0x0, &t1, &t1) != MP_OKAY)
                                          ^
                                           {
bn_s_mp_karatsuba_sqr.c:90:41: error: statement should be inside braces [google-readability-braces-around-statements,-warnings-as-errors]
   if (mp_add(&t1, &x1x1, b) != MP_OKAY)
                                        ^
                                         {
bn_s_mp_montgomery_reduce_fast.c:69:20: error: The left operand of '&' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult,-warnings-as-errors]
      mu = ((W[ix] & MP_MASK) * rho) & MP_MASK;
                   ^
bn_s_mp_montgomery_reduce_fast.c:20:8: note: Assuming the condition is false
   if (x->used > MP_WARRAY) {
       ^
bn_s_mp_montgomery_reduce_fast.c:20:4: note: Taking false branch
   if (x->used > MP_WARRAY) {
   ^
bn_s_mp_montgomery_reduce_fast.c:28:4: note: Taking false branch
   if (x->alloc < (n->used + 1)) {
   ^
bn_s_mp_montgomery_reduce_fast.c:48:20: note: Assuming the condition is false
      for (ix = 0; ix < x->used; ix++) {
                   ^
bn_s_mp_montgomery_reduce_fast.c:48:7: note: Loop condition is false. Execution continues on line 53
      for (ix = 0; ix < x->used; ix++) {
      ^
bn_s_mp_montgomery_reduce_fast.c:53:11: note: Assuming the condition is false
      if (ix < ((n->used * 2) + 1)) {
          ^
bn_s_mp_montgomery_reduce_fast.c:53:7: note: Taking false branch
      if (ix < ((n->used * 2) + 1)) {
      ^
bn_s_mp_montgomery_reduce_fast.c:61:17: note: Assuming the condition is true
   for (ix = 0; ix < n->used; ix++) {
                ^
bn_s_mp_montgomery_reduce_fast.c:61:4: note: Loop condition is true.  Entering loop body
   for (ix = 0; ix < n->used; ix++) {
   ^
bn_s_mp_montgomery_reduce_fast.c:69:20: note: The left operand of '&' is a garbage value
      mu = ((W[ix] & MP_MASK) * rho) & MP_MASK;
                   ^
bn_s_mp_montgomery_reduce_fast.c:98:19: error: The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage [clang-analyzer-core.uninitialized.Assign,-warnings-as-errors]
            *_W++ += (mp_word)mu * (mp_word)*tmpn++;
                  ^
bn_s_mp_montgomery_reduce_fast.c:20:8: note: Assuming the condition is false
   if (x->used > MP_WARRAY) {
       ^
bn_s_mp_montgomery_reduce_fast.c:20:4: note: Taking false branch
   if (x->used > MP_WARRAY) {
   ^
bn_s_mp_montgomery_reduce_fast.c:28:4: note: Taking false branch
   if (x->alloc < (n->used + 1)) {
   ^
bn_s_mp_montgomery_reduce_fast.c:48:20: note: Assuming the condition is true
      for (ix = 0; ix < x->used; ix++) {
                   ^
bn_s_mp_montgomery_reduce_fast.c:48:7: note: Loop condition is true.  Entering loop body
      for (ix = 0; ix < x->used; ix++) {
      ^
bn_s_mp_montgomery_reduce_fast.c:48:20: note: Assuming the condition is false
      for (ix = 0; ix < x->used; ix++) {
                   ^
bn_s_mp_montgomery_reduce_fast.c:48:7: note: Loop condition is false. Execution continues on line 53
      for (ix = 0; ix < x->used; ix++) {
      ^
bn_s_mp_montgomery_reduce_fast.c:53:11: note: Assuming the condition is false
      if (ix < ((n->used * 2) + 1)) {
          ^
bn_s_mp_montgomery_reduce_fast.c:53:7: note: Taking false branch
      if (ix < ((n->used * 2) + 1)) {
      ^
bn_s_mp_montgomery_reduce_fast.c:61:17: note: Assuming the condition is true
   for (ix = 0; ix < n->used; ix++) {
                ^
bn_s_mp_montgomery_reduce_fast.c:61:4: note: Loop condition is true.  Entering loop body
   for (ix = 0; ix < n->used; ix++) {
   ^
bn_s_mp_montgomery_reduce_fast.c:97:10: note: Loop condition is true.  Entering loop body
         for (iy = 0; iy < n->used; iy++) {
         ^
bn_s_mp_montgomery_reduce_fast.c:97:23: note: Assuming the condition is true
         for (iy = 0; iy < n->used; iy++) {
                      ^
bn_s_mp_montgomery_reduce_fast.c:97:10: note: Loop condition is true.  Entering loop body
         for (iy = 0; iy < n->used; iy++) {
         ^
bn_s_mp_montgomery_reduce_fast.c:98:19: note: The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage
            *_W++ += (mp_word)mu * (mp_word)*tmpn++;
                  ^
bn_s_mp_montgomery_reduce_fast.c:103:17: error: The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage [clang-analyzer-core.uninitialized.Assign,-warnings-as-errors]
      W[ix + 1] += W[ix] >> (mp_word)MP_DIGIT_BIT;
                ^
bn_s_mp_montgomery_reduce_fast.c:20:8: note: Assuming the condition is false
   if (x->used > MP_WARRAY) {
       ^
bn_s_mp_montgomery_reduce_fast.c:20:4: note: Taking false branch
   if (x->used > MP_WARRAY) {
   ^
bn_s_mp_montgomery_reduce_fast.c:28:4: note: Taking false branch
   if (x->alloc < (n->used + 1)) {
   ^
bn_s_mp_montgomery_reduce_fast.c:48:20: note: Assuming the condition is true
      for (ix = 0; ix < x->used; ix++) {
                   ^
bn_s_mp_montgomery_reduce_fast.c:48:7: note: Loop condition is true.  Entering loop body
      for (ix = 0; ix < x->used; ix++) {
      ^
bn_s_mp_montgomery_reduce_fast.c:48:20: note: Assuming the condition is false
      for (ix = 0; ix < x->used; ix++) {
                   ^
bn_s_mp_montgomery_reduce_fast.c:48:7: note: Loop condition is false. Execution continues on line 53
      for (ix = 0; ix < x->used; ix++) {
      ^
bn_s_mp_montgomery_reduce_fast.c:53:11: note: Assuming the condition is false
      if (ix < ((n->used * 2) + 1)) {
          ^
bn_s_mp_montgomery_reduce_fast.c:53:7: note: Taking false branch
      if (ix < ((n->used * 2) + 1)) {
      ^
bn_s_mp_montgomery_reduce_fast.c:61:17: note: Assuming the condition is true
   for (ix = 0; ix < n->used; ix++) {
                ^
bn_s_mp_montgomery_reduce_fast.c:61:4: note: Loop condition is true.  Entering loop body
   for (ix = 0; ix < n->used; ix++) {
   ^
bn_s_mp_montgomery_reduce_fast.c:97:10: note: Loop condition is true.  Entering loop body
         for (iy = 0; iy < n->used; iy++) {
         ^
bn_s_mp_montgomery_reduce_fast.c:97:23: note: Assuming the condition is false
         for (iy = 0; iy < n->used; iy++) {
                      ^
bn_s_mp_montgomery_reduce_fast.c:97:10: note: Loop condition is false. Execution continues on line 103
         for (iy = 0; iy < n->used; iy++) {
         ^
bn_s_mp_montgomery_reduce_fast.c:103:17: note: The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage
      W[ix + 1] += W[ix] >> (mp_word)MP_DIGIT_BIT;
                ^
bn_s_mp_montgomery_reduce_fast.c:123:16: error: The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage [clang-analyzer-core.uninitialized.Assign,-warnings-as-errors]
         *_W++ += *_W1++ >> (mp_word)MP_DIGIT_BIT;
               ^
bn_s_mp_montgomery_reduce_fast.c:20:8: note: Assuming the condition is false
   if (x->used > MP_WARRAY) {
       ^
bn_s_mp_montgomery_reduce_fast.c:20:4: note: Taking false branch
   if (x->used > MP_WARRAY) {
   ^
bn_s_mp_montgomery_reduce_fast.c:28:4: note: Taking false branch
   if (x->alloc < (n->used + 1)) {
   ^
bn_s_mp_montgomery_reduce_fast.c:48:20: note: Assuming the condition is true
      for (ix = 0; ix < x->used; ix++) {
                   ^
bn_s_mp_montgomery_reduce_fast.c:48:7: note: Loop condition is true.  Entering loop body
      for (ix = 0; ix < x->used; ix++) {
      ^
bn_s_mp_montgomery_reduce_fast.c:48:20: note: Assuming the condition is false
      for (ix = 0; ix < x->used; ix++) {
                   ^
bn_s_mp_montgomery_reduce_fast.c:48:7: note: Loop condition is false. Execution continues on line 53
      for (ix = 0; ix < x->used; ix++) {
      ^
bn_s_mp_montgomery_reduce_fast.c:53:11: note: Assuming the condition is false
      if (ix < ((n->used * 2) + 1)) {
          ^
bn_s_mp_montgomery_reduce_fast.c:53:7: note: Taking false branch
      if (ix < ((n->used * 2) + 1)) {
      ^
bn_s_mp_montgomery_reduce_fast.c:61:17: note: Assuming the condition is false
   for (ix = 0; ix < n->used; ix++) {
                ^
bn_s_mp_montgomery_reduce_fast.c:61:4: note: Loop condition is false. Execution continues on line 111
   for (ix = 0; ix < n->used; ix++) {
   ^
bn_s_mp_montgomery_reduce_fast.c:122:14: note: Assuming the condition is true
      for (; ix <= ((n->used * 2) + 1); ix++) {
             ^
bn_s_mp_montgomery_reduce_fast.c:122:7: note: Loop condition is true.  Entering loop body
      for (; ix <= ((n->used * 2) + 1); ix++) {
      ^
bn_s_mp_montgomery_reduce_fast.c:123:16: note: The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage
         *_W++ += *_W1++ >> (mp_word)MP_DIGIT_BIT;
               ^
bn_s_mp_montgomery_reduce_fast.c:123:26: error: The left operand of '>>' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult,-warnings-as-errors]
         *_W++ += *_W1++ >> (mp_word)MP_DIGIT_BIT;
                         ^
bn_s_mp_montgomery_reduce_fast.c:20:8: note: Assuming the condition is false
   if (x->used > MP_WARRAY) {
       ^
bn_s_mp_montgomery_reduce_fast.c:20:4: note: Taking false branch
   if (x->used > MP_WARRAY) {
   ^
bn_s_mp_montgomery_reduce_fast.c:28:4: note: Taking false branch
   if (x->alloc < (n->used + 1)) {
   ^
bn_s_mp_montgomery_reduce_fast.c:48:20: note: Assuming the condition is false
      for (ix = 0; ix < x->used; ix++) {
                   ^
bn_s_mp_montgomery_reduce_fast.c:48:7: note: Loop condition is false. Execution continues on line 53
      for (ix = 0; ix < x->used; ix++) {
      ^
bn_s_mp_montgomery_reduce_fast.c:53:11: note: Assuming the condition is true
      if (ix < ((n->used * 2) + 1)) {
          ^
bn_s_mp_montgomery_reduce_fast.c:53:7: note: Taking true branch
      if (ix < ((n->used * 2) + 1)) {
      ^
bn_s_mp_montgomery_reduce_fast.c:61:17: note: Assuming the condition is false
   for (ix = 0; ix < n->used; ix++) {
                ^
bn_s_mp_montgomery_reduce_fast.c:61:4: note: Loop condition is false. Execution continues on line 111
   for (ix = 0; ix < n->used; ix++) {
   ^
bn_s_mp_montgomery_reduce_fast.c:122:7: note: Loop condition is true.  Entering loop body
      for (; ix <= ((n->used * 2) + 1); ix++) {
      ^
bn_s_mp_montgomery_reduce_fast.c:123:26: note: The left operand of '>>' is a garbage value
         *_W++ += *_W1++ >> (mp_word)MP_DIGIT_BIT;

I am not sure if we want to fix them, feel free to close this issue if the findings are acceptable.

czurnieden commented 5 years ago

I am not sure if we want to fix them

It found at least two "leftovers" (both mine *sigh*). The rest are either false positives or debatable.

[several places] do not use 'else' after 'return'

In most cases it would need a temporary variably to avoid that warning. I would say: not worth it, but that's only my opinion.

bn_mp_prime_is_prime.c:148:7

That is a correct one, t = -t can go, the code using it is gone, it's a leftover.

bn_mp_prime_strong_lucas_selfridge.c:191:7: error: Value stored to 'Q' is never read

That is a correct one, the second Q = -Q can go. Actually, the whole thing can go, since we have mp_set_i32 now.

Both done in #318

The rest are false positives or at least debatable.

bn_mp_prime_rand.c:71:17: error: The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage

That is a correct one. A cheap repair would be to use calloc() instead although it is debatable if a repair is necessary in the first place (memory is allocated and that it is garbage does not matter)

bn_mp_prime_rand.c:39:25: note: Assuming the condition is false bsize = (size>>3) + ((size&7)?1:0)

No, cannot happen, that's the whole point of that line together with the checks above. Condition size > 1 has been checked and can be assumed. If size < 8, (size&7) is true. If size == 8, (size&7) is false but size >> 3 is 1 one, so bsize is at least 1 (one)

[several] error: statement should be inside braces

I'm all for braces, even for a single statement, but because it is a question of style I'm bracing for impact, too.

The warnings for bn_s_mp_montgomery_reduce_fast.c all have a single source:

bn_s_mp_montgomery_reduce_fast.c:69:20: error: The left operand of '&' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult,-warnings-as-errors] mu = ((W[ix] & MP_MASK) * rho) & MP_MASK;

False positive. (caller has done the necessary checks. T.B.C.!)

bn_s_mp_montgomery_reduce_fast.c:98:19: error: The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage [clang-analyzer-core.uninitialized.Assign,-warnings-as-errors] _W++ += (mp_word)mu (mp_word)*tmpn++;

False positive. (See above)

bn_s_mp_montgomery_reduce_fast.c:103:17: error: The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage [clang-analyzer-core.uninitialized.Assign,-warnings-as-errors] W[ix + 1] += W[ix] >> (mp_word)MP_DIGIT_BIT;

False positive. (See above)

sjaeckel commented 5 years ago

I think we can safely assume that readability-else-after-return can be disabled for ltm ;-)

after #318 and #319 are those left which are classed as false positive by @czurnieden

bn_s_mp_montgomery_reduce_fast.c:69:20: error: The left operand of '&' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult,-warnings-as-errors] mu = ((W[ix] & MP_MASK) * rho) & MP_MASK;

False positive. (caller has done the necessary checks. T.B.C.!)

False positive (and the rest of bn_s_mp_montgomery_reduce_fast.c errors)

can most likely be proven by de-obfuscating the initialization of W by _W!? how about the speed difference? Is this implementation really that much faster than simply indexing into the array?

Would it probably make sense to report this to the clang-tidy team?

@karel-m could you please check again after cherry-picking #318 and #319 ?

karel-m commented 5 years ago

Ad google-readability-braces-around-statements - it sounds a bit stricter than it actually is.

This is wrong:

      if ((err = mp_lshd(&q, um + 1)) != MP_OKAY)
         goto CLEANUP;

This is good:

      if ((err = mp_lshd(&q, um + 1)) != MP_OKAY) {
         goto CLEANUP;
      }

However, this is good as well:

      if ((err = mp_lshd(&q, um + 1)) != MP_OKAY) goto CLEANUP;

IMO reasonably relaxed.

Ad ... report this to the clang-tidy team? Firstly we should perhaps try the latest clang-tidy (mine is from LLVM 6.0.0).

sjaeckel commented 5 years ago

uhm, I just wanted to say "how about adding a clang-tidy job to travis where we build with clang-9?" but apparently we only build up to clang-7... :) the "how about adding a clang-tidy job to travis" still holds ;)

karel-m commented 5 years ago

without readability-else-after-return and google-readability-braces-around-statements+ cherry-picked #318 and #319

bn_mp_prime_rand.c:71:17: error: The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage [clang-analyzer-core.uninitialized.Assign,-warnings-as-errors]
      tmp[0]    &= maskAND;
                ^
bn_mp_prime_rand.c:137:11: note: Calling 's_mp_prime_random_ex'
   return s_mp_prime_random_ex(a, t, size, flags, s_mp_rand_cb, NULL);
          ^
bn_mp_prime_rand.c:29:9: note: Assuming 'size' is > 1
   if ((size <= 1) || (t <= 0)) {
        ^
bn_mp_prime_rand.c:29:8: note: Left side of '||' is false
   if ((size <= 1) || (t <= 0)) {
       ^
bn_mp_prime_rand.c:29:24: note: Assuming 't' is > 0
   if ((size <= 1) || (t <= 0)) {
                       ^
bn_mp_prime_rand.c:29:4: note: Taking false branch
   if ((size <= 1) || (t <= 0)) {
   ^
bn_mp_prime_rand.c:34:8: note: Assuming the condition is false
   if ((flags & MP_PRIME_SAFE) != 0) {
       ^
bn_mp_prime_rand.c:34:4: note: Taking false branch
   if ((flags & MP_PRIME_SAFE) != 0) {
   ^
bn_mp_prime_rand.c:39:25: note: Assuming the condition is false
   bsize = (size>>3) + ((size&7)?1:0);
                        ^
bn_mp_prime_rand.c:39:25: note: '?' condition is false
bn_mp_prime_rand.c:42:28: note: Storing uninitialized value
   tmp = (unsigned char *) MP_MALLOC((size_t)bsize);
                           ^
./tommath_private.h:131:46: note: expanded from macro 'MP_MALLOC'
#   define MP_MALLOC(size)                   malloc(size)
                                             ^
bn_mp_prime_rand.c:43:8: note: Assuming 'tmp' is not equal to NULL
   if (tmp == NULL) {
       ^
bn_mp_prime_rand.c:43:4: note: Taking false branch
   if (tmp == NULL) {
   ^
bn_mp_prime_rand.c:48:14: note: '?' condition is true
   maskAND = ((size&7) == 0) ? 0xFFu : (unsigned char)(0xFFu >> (8 - (size & 7)));
             ^
bn_mp_prime_rand.c:52:24: note: '?' condition is false
   maskOR_msb_offset = ((size & 7) == 1) ? 1 : 0;
                       ^
bn_mp_prime_rand.c:53:8: note: Assuming the condition is false
   if ((flags & MP_PRIME_2MSB_ON) != 0) {
       ^
bn_mp_prime_rand.c:53:4: note: Taking false branch
   if ((flags & MP_PRIME_2MSB_ON) != 0) {
   ^
bn_mp_prime_rand.c:59:8: note: Assuming the condition is false
   if ((flags & MP_PRIME_BBS) != 0) {
       ^
bn_mp_prime_rand.c:59:4: note: Taking false branch
   if ((flags & MP_PRIME_BBS) != 0) {
   ^
bn_mp_prime_rand.c:65:11: note: Calling 's_mp_rand_cb'
      if (cb(tmp, bsize, dat) != bsize) {
          ^
bn_mp_prime_rand.c:126:8: note: Assuming 'len' is <= 0
   if (len <= 0) {
       ^
bn_mp_prime_rand.c:126:4: note: Taking true branch
   if (len <= 0) {
   ^
bn_mp_prime_rand.c:65:11: note: Returning from 's_mp_rand_cb'
      if (cb(tmp, bsize, dat) != bsize) {
          ^
bn_mp_prime_rand.c:65:7: note: Taking false branch
      if (cb(tmp, bsize, dat) != bsize) {
      ^
bn_mp_prime_rand.c:71:17: note: The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage
      tmp[0]    &= maskAND;
                ^

bn_s_mp_montgomery_reduce_fast.c:69:20: error: The left operand of '&' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult,-warnings-as-errors]
      mu = ((W[ix] & MP_MASK) * rho) & MP_MASK;
                   ^
bn_s_mp_montgomery_reduce_fast.c:20:8: note: Assuming the condition is false
   if (x->used > MP_WARRAY) {
       ^
bn_s_mp_montgomery_reduce_fast.c:20:4: note: Taking false branch
   if (x->used > MP_WARRAY) {
   ^
bn_s_mp_montgomery_reduce_fast.c:28:4: note: Taking false branch
   if (x->alloc < (n->used + 1)) {
   ^
bn_s_mp_montgomery_reduce_fast.c:48:20: note: Assuming the condition is false
      for (ix = 0; ix < x->used; ix++) {
                   ^
bn_s_mp_montgomery_reduce_fast.c:48:7: note: Loop condition is false. Execution continues on line 53
      for (ix = 0; ix < x->used; ix++) {
      ^
bn_s_mp_montgomery_reduce_fast.c:53:11: note: Assuming the condition is false
      if (ix < ((n->used * 2) + 1)) {
          ^
bn_s_mp_montgomery_reduce_fast.c:53:7: note: Taking false branch
      if (ix < ((n->used * 2) + 1)) {
      ^
bn_s_mp_montgomery_reduce_fast.c:61:17: note: Assuming the condition is true
   for (ix = 0; ix < n->used; ix++) {
                ^
bn_s_mp_montgomery_reduce_fast.c:61:4: note: Loop condition is true.  Entering loop body
   for (ix = 0; ix < n->used; ix++) {
   ^
bn_s_mp_montgomery_reduce_fast.c:69:20: note: The left operand of '&' is a garbage value
      mu = ((W[ix] & MP_MASK) * rho) & MP_MASK;
                   ^

bn_s_mp_montgomery_reduce_fast.c:98:19: error: The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage [clang-analyzer-core.uninitialized.Assign,-warnings-as-errors]
            *_W++ += (mp_word)mu * (mp_word)*tmpn++;
                  ^
bn_s_mp_montgomery_reduce_fast.c:20:8: note: Assuming the condition is false
   if (x->used > MP_WARRAY) {
       ^
bn_s_mp_montgomery_reduce_fast.c:20:4: note: Taking false branch
   if (x->used > MP_WARRAY) {
   ^
bn_s_mp_montgomery_reduce_fast.c:28:4: note: Taking false branch
   if (x->alloc < (n->used + 1)) {
   ^
bn_s_mp_montgomery_reduce_fast.c:48:20: note: Assuming the condition is true
      for (ix = 0; ix < x->used; ix++) {
                   ^
bn_s_mp_montgomery_reduce_fast.c:48:7: note: Loop condition is true.  Entering loop body
      for (ix = 0; ix < x->used; ix++) {
      ^
bn_s_mp_montgomery_reduce_fast.c:48:20: note: Assuming the condition is false
      for (ix = 0; ix < x->used; ix++) {
                   ^
bn_s_mp_montgomery_reduce_fast.c:48:7: note: Loop condition is false. Execution continues on line 53
      for (ix = 0; ix < x->used; ix++) {
      ^
bn_s_mp_montgomery_reduce_fast.c:53:11: note: Assuming the condition is false
      if (ix < ((n->used * 2) + 1)) {
          ^
bn_s_mp_montgomery_reduce_fast.c:53:7: note: Taking false branch
      if (ix < ((n->used * 2) + 1)) {
      ^
bn_s_mp_montgomery_reduce_fast.c:61:17: note: Assuming the condition is true
   for (ix = 0; ix < n->used; ix++) {
                ^
bn_s_mp_montgomery_reduce_fast.c:61:4: note: Loop condition is true.  Entering loop body
   for (ix = 0; ix < n->used; ix++) {
   ^
bn_s_mp_montgomery_reduce_fast.c:97:10: note: Loop condition is true.  Entering loop body
         for (iy = 0; iy < n->used; iy++) {
         ^
bn_s_mp_montgomery_reduce_fast.c:97:23: note: Assuming the condition is true
         for (iy = 0; iy < n->used; iy++) {
                      ^
bn_s_mp_montgomery_reduce_fast.c:97:10: note: Loop condition is true.  Entering loop body
         for (iy = 0; iy < n->used; iy++) {
         ^
bn_s_mp_montgomery_reduce_fast.c:98:19: note: The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage
            *_W++ += (mp_word)mu * (mp_word)*tmpn++;
                  ^

bn_s_mp_montgomery_reduce_fast.c:103:17: error: The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage [clang-analyzer-core.uninitialized.Assign,-warnings-as-errors]
      W[ix + 1] += W[ix] >> (mp_word)MP_DIGIT_BIT;
                ^
bn_s_mp_montgomery_reduce_fast.c:20:8: note: Assuming the condition is false
   if (x->used > MP_WARRAY) {
       ^
bn_s_mp_montgomery_reduce_fast.c:20:4: note: Taking false branch
   if (x->used > MP_WARRAY) {
   ^
bn_s_mp_montgomery_reduce_fast.c:28:4: note: Taking false branch
   if (x->alloc < (n->used + 1)) {
   ^
bn_s_mp_montgomery_reduce_fast.c:48:20: note: Assuming the condition is true
      for (ix = 0; ix < x->used; ix++) {
                   ^
bn_s_mp_montgomery_reduce_fast.c:48:7: note: Loop condition is true.  Entering loop body
      for (ix = 0; ix < x->used; ix++) {
      ^
bn_s_mp_montgomery_reduce_fast.c:48:20: note: Assuming the condition is false
      for (ix = 0; ix < x->used; ix++) {
                   ^
bn_s_mp_montgomery_reduce_fast.c:48:7: note: Loop condition is false. Execution continues on line 53
      for (ix = 0; ix < x->used; ix++) {
      ^
bn_s_mp_montgomery_reduce_fast.c:53:11: note: Assuming the condition is false
      if (ix < ((n->used * 2) + 1)) {
          ^
bn_s_mp_montgomery_reduce_fast.c:53:7: note: Taking false branch
      if (ix < ((n->used * 2) + 1)) {
      ^
bn_s_mp_montgomery_reduce_fast.c:61:17: note: Assuming the condition is true
   for (ix = 0; ix < n->used; ix++) {
                ^
bn_s_mp_montgomery_reduce_fast.c:61:4: note: Loop condition is true.  Entering loop body
   for (ix = 0; ix < n->used; ix++) {
   ^
bn_s_mp_montgomery_reduce_fast.c:97:10: note: Loop condition is true.  Entering loop body
         for (iy = 0; iy < n->used; iy++) {
         ^
bn_s_mp_montgomery_reduce_fast.c:97:23: note: Assuming the condition is false
         for (iy = 0; iy < n->used; iy++) {
                      ^
bn_s_mp_montgomery_reduce_fast.c:97:10: note: Loop condition is false. Execution continues on line 103
         for (iy = 0; iy < n->used; iy++) {
         ^
bn_s_mp_montgomery_reduce_fast.c:103:17: note: The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage
      W[ix + 1] += W[ix] >> (mp_word)MP_DIGIT_BIT;
                ^

bn_s_mp_montgomery_reduce_fast.c:123:16: error: The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage [clang-analyzer-core.uninitialized.Assign,-warnings-as-errors]
         *_W++ += *_W1++ >> (mp_word)MP_DIGIT_BIT;
               ^
bn_s_mp_montgomery_reduce_fast.c:20:8: note: Assuming the condition is false
   if (x->used > MP_WARRAY) {
       ^
bn_s_mp_montgomery_reduce_fast.c:20:4: note: Taking false branch
   if (x->used > MP_WARRAY) {
   ^
bn_s_mp_montgomery_reduce_fast.c:28:4: note: Taking false branch
   if (x->alloc < (n->used + 1)) {
   ^
bn_s_mp_montgomery_reduce_fast.c:48:20: note: Assuming the condition is true
      for (ix = 0; ix < x->used; ix++) {
                   ^
bn_s_mp_montgomery_reduce_fast.c:48:7: note: Loop condition is true.  Entering loop body
      for (ix = 0; ix < x->used; ix++) {
      ^
bn_s_mp_montgomery_reduce_fast.c:48:20: note: Assuming the condition is false
      for (ix = 0; ix < x->used; ix++) {
                   ^
bn_s_mp_montgomery_reduce_fast.c:48:7: note: Loop condition is false. Execution continues on line 53
      for (ix = 0; ix < x->used; ix++) {
      ^
bn_s_mp_montgomery_reduce_fast.c:53:11: note: Assuming the condition is false
      if (ix < ((n->used * 2) + 1)) {
          ^
bn_s_mp_montgomery_reduce_fast.c:53:7: note: Taking false branch
      if (ix < ((n->used * 2) + 1)) {
      ^
bn_s_mp_montgomery_reduce_fast.c:61:17: note: Assuming the condition is false
   for (ix = 0; ix < n->used; ix++) {
                ^
bn_s_mp_montgomery_reduce_fast.c:61:4: note: Loop condition is false. Execution continues on line 111
   for (ix = 0; ix < n->used; ix++) {
   ^
bn_s_mp_montgomery_reduce_fast.c:122:14: note: Assuming the condition is true
      for (; ix <= ((n->used * 2) + 1); ix++) {
             ^
bn_s_mp_montgomery_reduce_fast.c:122:7: note: Loop condition is true.  Entering loop body
      for (; ix <= ((n->used * 2) + 1); ix++) {
      ^
bn_s_mp_montgomery_reduce_fast.c:123:16: note: The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage
         *_W++ += *_W1++ >> (mp_word)MP_DIGIT_BIT;
               ^

bn_s_mp_montgomery_reduce_fast.c:123:26: error: The left operand of '>>' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult,-warnings-as-errors]
         *_W++ += *_W1++ >> (mp_word)MP_DIGIT_BIT;
                         ^
bn_s_mp_montgomery_reduce_fast.c:20:8: note: Assuming the condition is false
   if (x->used > MP_WARRAY) {
       ^
bn_s_mp_montgomery_reduce_fast.c:20:4: note: Taking false branch
   if (x->used > MP_WARRAY) {
   ^
bn_s_mp_montgomery_reduce_fast.c:28:4: note: Taking false branch
   if (x->alloc < (n->used + 1)) {
   ^
bn_s_mp_montgomery_reduce_fast.c:48:20: note: Assuming the condition is false
      for (ix = 0; ix < x->used; ix++) {
                   ^
bn_s_mp_montgomery_reduce_fast.c:48:7: note: Loop condition is false. Execution continues on line 53
      for (ix = 0; ix < x->used; ix++) {
      ^
bn_s_mp_montgomery_reduce_fast.c:53:11: note: Assuming the condition is true
      if (ix < ((n->used * 2) + 1)) {
          ^
bn_s_mp_montgomery_reduce_fast.c:53:7: note: Taking true branch
      if (ix < ((n->used * 2) + 1)) {
      ^
bn_s_mp_montgomery_reduce_fast.c:61:17: note: Assuming the condition is false
   for (ix = 0; ix < n->used; ix++) {
                ^
bn_s_mp_montgomery_reduce_fast.c:61:4: note: Loop condition is false. Execution continues on line 111
   for (ix = 0; ix < n->used; ix++) {
   ^
bn_s_mp_montgomery_reduce_fast.c:122:7: note: Loop condition is true.  Entering loop body
      for (; ix <= ((n->used * 2) + 1); ix++) {
      ^
bn_s_mp_montgomery_reduce_fast.c:123:26: note: The left operand of '>>' is a garbage value
         *_W++ += *_W1++ >> (mp_word)MP_DIGIT_BIT;
                         ^
sjaeckel commented 5 years ago

clang-9 added

bn_mp_clear_multi.c:15:17: warning: va_arg() is called on an uninitialized va_list [clang-analyzer-valist.Uninitialized]
      next_mp = va_arg(args, mp_int *);
                ^
/usr/lib/llvm-9/lib/clang/9.0.0/include/stdarg.h:19:29: note: expanded from macro 'va_arg'
#define va_arg(ap, type)    __builtin_va_arg(ap, type)
                            ^
bn_mp_clear_multi.c:13:11: note: Assuming 'next_mp' is not equal to NULL
   while (next_mp != NULL) {
          ^
bn_mp_clear_multi.c:13:4: note: Loop condition is true.  Entering loop body
   while (next_mp != NULL) {
   ^
bn_mp_clear_multi.c:15:17: note: va_arg() is called on an uninitialized va_list
      next_mp = va_arg(args, mp_int *);
                ^
/usr/lib/llvm-9/lib/clang/9.0.0/include/stdarg.h:19:29: note: expanded from macro 'va_arg'
#define va_arg(ap, type)    __builtin_va_arg(ap, type)
                            ^
bn_mp_init_multi.c:35:17: warning: va_arg() is called on an uninitialized va_list [clang-analyzer-valist.Uninitialized]
      cur_arg = va_arg(args, mp_int *);
                ^
/usr/lib/llvm-9/lib/clang/9.0.0/include/stdarg.h:19:29: note: expanded from macro 'va_arg'
#define va_arg(ap, type)    __builtin_va_arg(ap, type)
                            ^
bn_mp_init_multi.c:16:11: note: Assuming 'cur_arg' is not equal to NULL
   while (cur_arg != NULL) {
          ^
bn_mp_init_multi.c:16:4: note: Loop condition is true.  Entering loop body
   while (cur_arg != NULL) {
   ^
bn_mp_init_multi.c:17:11: note: Assuming the condition is false
      if (mp_init(cur_arg) != MP_OKAY) {
          ^
bn_mp_init_multi.c:17:7: note: Taking false branch
      if (mp_init(cur_arg) != MP_OKAY) {
      ^
bn_mp_init_multi.c:35:17: note: va_arg() is called on an uninitialized va_list
      cur_arg = va_arg(args, mp_int *);
                ^
/usr/lib/llvm-9/lib/clang/9.0.0/include/stdarg.h:19:29: note: expanded from macro 'va_arg'
#define va_arg(ap, type)    __builtin_va_arg(ap, type)
karel-m commented 5 years ago

how about adding a clang-tidy job to travis?

I run something like the following:

clang-tidy *.c -warnings-as-errors='*' --quiet --checks=-*,\
readability-*,-readability-function-size,-readability-braces-around-statements,\
misc-*,-misc-macro-parentheses,\
clang-analyzer-*,-clang-analyzer-valist.Uninitialized,\
google-*,-google-readability-function-size,-google-readability-casting,\
-readability-else-after-return,\
-google-readability-braces-around-statements,\
performance-*,\
modernize-*,\
cert-*,\
bugprone-*,\
portability-* -- -I.
sjaeckel commented 5 years ago

lulz if I let it run like that it looks like

...
2835 warnings generated.
2854 warnings generated.
bn_deprecated.c:21:20: error: integer literal has suffix 'uL', which is not uppercase [readability-uppercase-literal-suffix,-warnings-as-errors]
   if (mp_cmp_d(n, 0uL) != MP_GT) {
                   ^~~
                    UL
... a huge list of errors ...
bn_s_mp_toom_sqr.c:25:4: error: multiple declarations in a single statement reduces readability [readability-isolate-declaration,-warnings-as-errors]
   mp_err err, B, count;
   ^~~~~~~~~~~~~~~~~~~~~
karel-m commented 5 years ago

@czurnieden I do not want to argue about (not) being false-positive; however, the following patch silences all 5 warnings in bn_s_mp_montgomery_reduce_fast.c

diff --git a/bn_s_mp_montgomery_reduce_fast.c b/bn_s_mp_montgomery_reduce_fast.c
index 843ad12..fa9548f 100644
--- a/bn_s_mp_montgomery_reduce_fast.c
+++ b/bn_s_mp_montgomery_reduce_fast.c
@@ -15,7 +15,7 @@ mp_err s_mp_montgomery_reduce_fast(mp_int *x, const mp_int *n, mp_digit rho)
 {
    int     ix, olduse;
    mp_err  err;
-   mp_word W[MP_WARRAY];
+   mp_word W[MP_WARRAY] = { 0 };

    if (x->used > MP_WARRAY) {
       return MP_VAL;
czurnieden commented 5 years ago

@karel-m

the following patch silences all 5 warnings

Yes, if you initialize the variable W, clang-tidy doesn't complain about an "uninitialized variable W" anymore. It is redundant, though, but it doesn't hurt (I think). The other case in bn_mp_prime_rand.c would need calloc, which is expensive.

It is all debatable, so I stepped back and restricted myself to repairing my own stuff only which was not debatable, because clang-tidy was right in those cases.

Thank you for taking the time to run clang-tidy! It should be done more often. Don't know if Travis is the right place but…why not.

sjaeckel commented 5 years ago

@czurnieden I do not want to argue about (not) being false-positive; however, the following patch silences all 5 warnings in bn_s_mp_montgomery_reduce_fast.c

this one as well

diff --git a/bn_s_mp_montgomery_reduce_fast.c b/bn_s_mp_montgomery_reduce_fast.c
index 843ad12..629e0ac 100644
--- a/bn_s_mp_montgomery_reduce_fast.c
+++ b/bn_s_mp_montgomery_reduce_fast.c
@@ -52,5 +52,3 @@ mp_err s_mp_montgomery_reduce_fast(mp_int *x, const mp_int *n, mp_digit rho)
       /* zero the high words of W[a->used..m->used*2] */
-      if (ix < ((n->used * 2) + 1)) {
-         MP_ZERO_BUFFER(_W, sizeof(mp_word) * (size_t)(((n->used * 2) + 1) - ix));
-      }
+      MP_ZERO_BUFFER(_W, (size_t)(MP_WARRAY - ix));
    }
sjaeckel commented 5 years ago

uhm, does this also fix a stack BOF? nope it doesn't