Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged

Conversation

real-or-random
Copy link
Contributor

@real-or-random real-or-random commented Oct 26, 2020

Fixes #834.

@sipa
Copy link
Contributor

sipa commented Oct 26, 2020

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.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested f1c9131 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
Copy link
Contributor

This looks reasonable to me.

@real-or-random real-or-random force-pushed the 202010_strauss_wnaf_counters branch from f1c9131 to 8893f42 Compare October 27, 2020 01:17
@real-or-random real-or-random marked this pull request as ready for review October 27, 2020 01:18
@real-or-random
Copy link
Contributor Author

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

@sipa
Copy link
Contributor

sipa commented Oct 27, 2020

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

Bogus compiler warning acts bogusly.

@sipa
Copy link
Contributor

sipa commented Oct 27, 2020

ACK 8893f42. np and no shouldn't ever take on negative values.

@jonasnick
Copy link
Contributor

ACK 8893f42

Also kicked travis since it hadn't started the job.

@elichai
Copy link
Contributor

elichai commented Oct 27, 2020

ACK 8893f42
Cool that it fixed the warning hehe

@jonasnick jonasnick merged commit 6f54e69 into bitcoin-core:master Oct 27, 2020
@hebasto
Copy link
Member

hebasto commented Oct 27, 2020

Is it planned to pull this change into the Bitcoin Core repo before 0.21.0 branching off?

@gmaxwell
Copy link
Contributor

@hebasto You're asking in the wrong place, other than sipa no one else here regularly contributes to bitcoin-core. :)

jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Oct 27, 2020
Summary: Backport of secp256k1 [[bitcoin-core/secp256k1#841 | PR841]].

Test Plan:
  ninja check-secp256k1

Reviewers: #bitcoin_abc, PiRK

Reviewed By: PiRK

Differential Revision: https://reviews.bitcoinabc.org/D8143
deadalnix pushed a commit to Bitcoin-ABC/secp256k1 that referenced this pull request Oct 28, 2020
Summary: Backport of secp256k1 [[bitcoin-core/secp256k1#841 | PR841]].

Test Plan:
  ninja check-secp256k1

Reviewers: #bitcoin_abc, PiRK

Reviewed By: PiRK

Differential Revision: https://reviews.bitcoinabc.org/D8143
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gcc emits -Warray-bounds warnings
6 participants