Avoids a potentially shortening size_t to int cast in strauss_wnaf_ #841

pull real-or-random wants to merge 1 commits into bitcoin-core:master from real-or-random:202010_strauss_wnaf_counters changing 1 files +3 −3

real-or-random commented at 1:57 PM on October 26, 2020: contributor

Fixes #834.
sipa cross-referenced this on Oct 26, 2020 from issue gcc emits -Warray-bounds warnings by hebasto

sipa commented at 7:47 PM on October 26, 2020: contributor

I confirm this fixes the

In file included from src/secp256k1.c:16:
src/ecmult_impl.h: In function ‘secp256k1_ecmult.constprop’:
src/ecmult_impl.h:496:48: warning: array subscript [1, 268435456] is outside array bounds of ‘struct secp256k1_strauss_point_state[1]’ [-Warray-bounds]
  496 |             secp256k1_gej tmp = a[state->ps[np].input_pos];
      |                                   ~~~~~~~~~~~~~^~~~~~~~~~
src/ecmult_impl.h:565:42: note: while referencing ‘ps’
  565 |     struct secp256k1_strauss_point_state ps[1];
      |                                          ^~
src/ecmult_impl.h:502:139: warning: array subscript [1, 268435456] is outside array bounds of ‘struct secp256k1_strauss_point_state[1]’ [-Warray-bounds]
  502 |             secp256k1_fe_mul(state->zr + np * ECMULT_TABLE_SIZE(WINDOW_A), state->zr + np * ECMULT_TABLE_SIZE(WINDOW_A), &(a[state->ps[np].input_pos].z));
      |                                                                                                                              ~~~~~~~~~~~~~^~~~~~~~~~
src/ecmult_impl.h:565:42: note: while referencing ‘ps’
  565 |     struct secp256k1_strauss_point_state ps[1];

warning.

hebasto commented at 8:21 PM on October 26, 2020: member

Tested f1c9131c798107efa4b2e660ae37b183ac662741 on Linux Mint 20 (x86_64):

$ gcc --version
gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
Copyright (C) 2019 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ make > /dev/null
# no warnings :)

gmaxwell commented at 11:56 PM on October 26, 2020: contributor

This looks reasonable to me.
Avoids a potentially shortening size_t to int cast in strauss_wnaf_
Fixes #834.
8893f42438
real-or-random force-pushed on Oct 27, 2020
real-or-random marked this as ready for review on Oct 27, 2020
real-or-random commented at 1:19 AM on October 27, 2020: contributor

Ok, I didn't expect that this will also fix the warning! Updated the commit message accordingly.
sipa commented at 1:24 AM on October 27, 2020: contributor

Ok, I didn't expect that this will also fix the warning! Updated the commit message accordingly.

Bogus compiler warning acts bogusly.
sipa commented at 1:27 AM on October 27, 2020: contributor

ACK 8893f42438ac75838a9dc7df7e98b29e9a1a085f. np and no shouldn't ever take on negative values.
jonasnick commented at 9:20 AM on October 27, 2020: contributor

ACK 8893f42438ac75838a9dc7df7e98b29e9a1a085f

Also kicked travis since it hadn't started the job.
elichai commented at 10:06 AM on October 27, 2020: contributor

ACK 8893f42438ac75838a9dc7df7e98b29e9a1a085f Cool that it fixed the warning hehe
jonasnick merged this on Oct 27, 2020
jonasnick closed this on Oct 27, 2020
hebasto commented at 1:29 PM on October 27, 2020: member

Is it planned to pull this change into the Bitcoin Core repo before 0.21.0 branching off?
gmaxwell commented at 4:54 PM on October 27, 2020: contributor

@hebasto You're asking in the wrong place, other than sipa no one else here regularly contributes to bitcoin-core. :)
jasonbcox referenced this in commit 8f1b1aaef1 on Oct 27, 2020
deadalnix referenced this in commit 43bb9cebec on Oct 28, 2020

Contributors

Linked (view graph)

#834 gcc emits -Warray-bounds warnings