#2034 Possible use of uninitialized values in ReSID "new" 8580 filter

[compyx]

GCC 14.1 warns about possible use of uninitialized values in filter8580new.cc:

Making all in resid
../../../../trunk/vice/src/resid/filter8580new.cc: In constructor 'reSID::Filter::Filter()':
../../../../trunk/vice/src/resid/filter8580new.cc:326:54: warning: 'scaled_voltage' may be used uninitialized [-Wmaybe-uninitialized]
  326 |       if (scaled_voltage[fi.opamp_voltage_size - 1][0] > 65535.) {
      |           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
../../../../trunk/vice/src/resid/filter8580new.cc:301:20: note: 'scaled_voltage' declared here
  301 |       double_point scaled_voltage[50];
      |                    ^~~~~~~~~~~~~~

The code initializing the scaled_voltage[] array looks okay to me though, but still worth taking a look I suppose.

[Leandro Nini]

Seems like a false positive to me, maybe the compiler is getting confused by the fact we only use part of the scaled_voltage array. This could be silenced with a memset call.

Will give it a more thorough check once I have gcc-14 installed.

[Leandro Nini]

Just for the record I can reproduce this with -O3 but not with -O2 (gcc-14.2)

[Leandro Nini]

Hmmm, this trivial patch silence the warning:

--- a/src/resid/filter8580new.cc
+++ b/src/resid/filter8580new.cc
@@ -300,6 +300,7 @@ Filter::Filter()
       // double_point scaled_voltage[fi.opamp_voltage_size];
       double_point scaled_voltage[50];

+      const int opamp_voltage_last = fi.opamp_voltage_size - 1;
       for (int i = 0; i < fi.opamp_voltage_size; i++) {
         // The target output range is 16 bits, in order to fit in an unsigned
         // short.
@@ -329,7 +330,7 @@ Filter::Filter()
             scaled_voltage[fi.opamp_voltage_size - 2][0] = 65535.;
       }

-      interpolate(scaled_voltage, scaled_voltage + fi.opamp_voltage_size - 1,
+      interpolate(scaled_voltage, scaled_voltage + opamp_voltage_last,
                     PointPlotter<unsigned int>(voltages), 1.0);

       // Store both fn and dfn in the same table.

Since fi.opamp_voltage_size - 1 is used in multiple places we can apply a nicer version like the one attached, but I'd like to file a gcc bug in case the compiler is optimizing too aggressively, unless someone can spot something wrong in the code. Next steps are single out the specific optimization flag that trigger the issue and produce a minimal test case.

[Leandro Nini]

Bug filed: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116315

[Leandro Nini]

As suggested the proper way to silence the warning is adding an assert.
This however would require some machinery to define NDEBUG for standard builds in resid (and while at it maybe rename the configure.in into a more modern configure.ac)

[Leandro Nini]

And here comes the build part, adding an --enable-debug option to resid.
To be applied with the above v2 patch.

[gpz]

so i should apply the last 2 patches? :)

[Leandro Nini]

Yes, one to define NDEBUG in resid for non-debug builds and the other for the assert that silence the warning

[gpz]

applied in r45317 - thanks!