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

-Wmaybe-uninitialized when compiling with -O1 #1361

Closed
hebasto opened this issue Jun 27, 2023 · 3 comments · Fixed by #1364
Closed

-Wmaybe-uninitialized when compiling with -O1 #1361

hebasto opened this issue Jun 27, 2023 · 3 comments · Fixed by #1364

Comments

@hebasto
Copy link
Member

hebasto commented Jun 27, 2023

$ ./autogen.sh && ./configure CFLAGS=-O1
$ make clean > /dev/null && make > /dev/null 
In file included from src/secp256k1.c:30:
In function 'secp256k1_ecmult_strauss_wnaf',
    inlined from 'secp256k1_ecmult' at src/ecmult_impl.h:353:5:
src/ecmult_impl.h:291:5: warning: 'aux' may be used uninitialized [-Wmaybe-uninitialized]
  291 |     secp256k1_ge_table_set_globalz(ECMULT_TABLE_SIZE(WINDOW_A) * no, state->pre_a, state->aux);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from src/secp256k1.c:29:
src/group_impl.h: In function 'secp256k1_ecmult':
src/group_impl.h:224:13: note: by argument 3 of type 'const secp256k1_fe *' to 'secp256k1_ge_table_set_globalz' declared here
  224 | static void secp256k1_ge_table_set_globalz(size_t len, secp256k1_ge *a, const secp256k1_fe *zr) {
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/ecmult_impl.h:345:18: note: 'aux' declared here
  345 |     secp256k1_fe aux[ECMULT_TABLE_SIZE(WINDOW_A)];
      |                  ^~~
In file included from src/secp256k1.c:30,
                 from src/bench_internal.c:8:
In function ‘secp256k1_ecmult_strauss_wnaf’,
    inlined from ‘secp256k1_ecmult’ at src/ecmult_impl.h:353:5:
src/ecmult_impl.h:291:5: warning: ‘aux’ may be used uninitialized [-Wmaybe-uninitialized]
  291 |     secp256k1_ge_table_set_globalz(ECMULT_TABLE_SIZE(WINDOW_A) * no, state->pre_a, state->aux);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from src/secp256k1.c:29:
src/group_impl.h: In function ‘secp256k1_ecmult’:
src/group_impl.h:224:13: note: by argument 3 of type ‘const secp256k1_fe *’ to ‘secp256k1_ge_table_set_globalz’ declared here
  224 | static void secp256k1_ge_table_set_globalz(size_t len, secp256k1_ge *a, const secp256k1_fe *zr) {
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/ecmult_impl.h:345:18: note: ‘aux’ declared here
  345 |     secp256k1_fe aux[ECMULT_TABLE_SIZE(WINDOW_A)];
      |                  ^~~

Using gcc 12.2.

No warnings with clang 15.

@real-or-random
Copy link
Contributor

real-or-random commented Jun 27, 2023

Wrapping the call in if (no > 0) suppresses this:

    if (no > 0) {
        secp256k1_ge_table_set_globalz(ECMULT_TABLE_SIZE(WINDOW_A) * no, state->pre_a, state->aux);
    }

(Or adding SECP256K1_INLINE to secp256k1_ge_table_set_globalz, but this seems a bit arbitrary...)

@hebasto
Copy link
Member Author

hebasto commented Jun 27, 2023

Wrapping the call in if (no > 0) suppresses this:

    if (no > 0) {
        secp256k1_ge_table_set_globalz(ECMULT_TABLE_SIZE(WINDOW_A) * no, state->pre_a, state->aux);
    }

Would if (no) be more consistent with the code in for loop above?

@real-or-random
Copy link
Contributor

Would if (no) be more consistent with the code in for loop above?

I think so.

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 a pull request may close this issue.

2 participants