jdk11u-dev icon indicating copy to clipboard operation
jdk11u-dev copied to clipboard

8367766: [11u] src/jdk.crypto.ec/share/native/libsunec/impl/mpi.c:321:3: error: 'tmp.dp' may be used uninitialized

Open zzambers opened this issue 2 months ago • 4 comments

This error/warning has been seen on newer gcc versions on openjdk 11:

In function 'mp_zero',
    inlined from 'mp_zero' at /home/tester/temurinbuild-1757944128/workspace/build/src/src/jdk.crypto.ec/share/native/libsunec/impl/mpi.c:316:8,
    inlined from 'mp_set_int' at /home/tester/temurinbuild-1757944128/workspace/build/src/src/jdk.crypto.ec/share/native/libsunec/impl/mpi.c:353:3,
    inlined from 'mp_cmp_int' at /home/tester/temurinbuild-1757944128/workspace/build/src/src/jdk.crypto.ec/share/native/libsunec/impl/mpi.c:1735:26:
/home/tester/temurinbuild-1757944128/workspace/build/src/src/jdk.crypto.ec/share/native/libsunec/impl/mpi.c:321:3: error: 'tmp.dp' may be used uninitialized [-Werror=maybe-uninitialized]
  321 |   s_mp_setz(DIGITS(mp), ALLOC(mp));
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/tester/temurinbuild-1757944128/workspace/build/src/src/jdk.crypto.ec/share/native/libsunec/impl/mpi.c: In function 'mp_cmp_int':
/home/tester/temurinbuild-1757944128/workspace/build/src/src/jdk.crypto.ec/share/native/libsunec/impl/mpi.c:1730:11: note: 'tmp' declared here
 1730 |   mp_int  tmp;
      |           ^~~

Problem does not affect newer jdks, because affected code was removed by JDK-8235710.

Also whether warning/error is generated depends on other conditions like compiler version / arch / build kind etc. Seems like compiler needs to do enough inlining to detect this, as analysis needs to cross function boundary.

Details/Fix: Underlining issue seems to be that mp_init called on tmp (struct) may theoretically fail, unitialized tmp would then still be passed to subsequent functions. Unfortunately mp_cmp_int is not really designed to be able to handle errors.

So I just zero initialize tmp, which makes error/warning go away. I think making more involved changes to API/implementation is probably not worth the effort, especially when code is for legacy curves, removed in newer jdks. It would also increase risk of regressions.


Progress

  • [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • [x] Change must not contain extraneous whitespace
  • [x] Commit message must refer to an issue
  • [ ] JDK-8367766 needs maintainer approval

Issue

  • JDK-8367766: [11u] src/jdk.crypto.ec/share/native/libsunec/impl/mpi.c:321:3: error: 'tmp.dp' may be used uninitialized (Bug - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk11u-dev.git pull/3095/head:pull/3095
$ git checkout pull/3095

Update a local copy of the PR:
$ git checkout pull/3095
$ git pull https://git.openjdk.org/jdk11u-dev.git pull/3095/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 3095

View PR using the GUI difftool:
$ git pr show -t 3095

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk11u-dev/pull/3095.diff

Using Webrev

Link to Webrev Comment

zzambers avatar Sep 16 '25 16:09 zzambers