Skip to content

ecdsa: Fix uninitialized field element in signature verification#1838

Closed
mllwchrry wants to merge 1 commit intobitcoin-core:masterfrom
mllwchrry:fix-ecdsa-xr-initialization
Closed

ecdsa: Fix uninitialized field element in signature verification#1838
mllwchrry wants to merge 1 commit intobitcoin-core:masterfrom
mllwchrry:fix-ecdsa-xr-initialization

Conversation

@mllwchrry
Copy link
Copy Markdown
Contributor

Fixes an uninitialized variable issue in src/ecdsa_impl.h where the xr field element may not be fully initialized before use.

This error was exposed while I was testing the improved test coverage in CI. The initial plan was to simplify the configuration of modules in CI by enabling all modules by default and testing the disabling of each module independently.

Error: src/field_impl.h:157:20: error: 'xr.normalized' may be used uninitialized [-Werror=maybe-uninitialized].

The error occurred when running the x86_64: Linux (Debian stable) (-DDETERMINISTIC, gcc-snapshot) CI job, which uses GCC 16 (snapshot) with strict compilation flags. See this action run for reference: https://github.com/mllwchrry/secp256k1/actions/runs/23301905657/job/67769464777.

@mllwchrry
Copy link
Copy Markdown
Contributor Author

Closing in favor of #1839.

@mllwchrry mllwchrry closed this Mar 24, 2026
real-or-random added a commit that referenced this pull request Mar 25, 2026
43fca0f ecdsa: VERIFY_CHECK result of _fe_set_b32_limit (Tim Ruffing)

Pull request description:

  This also avoids a spurious `-Wmaybe-uninitialized` warning emitted by gcc 16 (snapshot) when compiling with `-DDETERMINISTIC`.

  Alternative to #1838 by @mllwchrry who tried very a similar thing as this PR but couldn't convince the compiler. (The GCC snapshot is very annoying: a simple `VERIFY_CHECK(secp256k1_fe_set_b32_limit(&xr, c))` doesn't do the trick. I found this variant here with a local store rather by accident.)

ACKs for top commit:
  mllwchrry:
    ACK 43fca0f
  theStack:
    utACK 43fca0f

Tree-SHA512: 2550043e953675db7614f98bbdffb706721834967ef36f7c905f7cbfeee5d88189a9acfcd64865ef822bb0e3272d228440bdfb1124228afe083e025056e53212
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants