Mercurial > hg > qm-dsp

changeset 170:8c86761a5533

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Fix overrun in reading inverse complex-to-real FFT input (contrary to docs)
author Chris Cannam
date Fri, 09 May 2014 14:35:46 +0100
parents 783fb5f0e626
children 5f720340b0dd
files build/linux/Makefile.linux dsp/transforms/FFT.cpp tests/Makefile tests/TestFFT.cpp
diffstat 4 files changed, 58 insertions(+), 16 deletions(-) [+]
line wrap: on
line diff
--- a/build/linux/Makefile.linux	Tue May 06 10:35:01 2014 +0100
+++ b/build/linux/Makefile.linux	Fri May 09 14:35:46 2014 +0100
@@ -1,5 +1,8 @@
 
 CFLAGS := -DNDEBUG -O3 -fPIC -ffast-math -msse -mfpmath=sse -ftree-vectorize -fomit-frame-pointer -DUSE_PTHREADS -I./include
+
+#CFLAGS := -DNDEBUG -g -fPIC -DUSE_PTHREADS -I./include
+
 CXXFLAGS := $(CFLAGS)
 
 include build/general/Makefile.inc
--- a/dsp/transforms/FFT.cpp	Tue May 06 10:35:01 2014 +0100
+++ b/dsp/transforms/FFT.cpp	Fri May 09 14:35:46 2014 +0100
@@ -147,7 +147,10 @@
 
     void inverse(const double *ri, const double *ii, double *ro) {
 
-        for (int i = 0; i < m_n; ++i) {
+        // kiss_fftr.h says
+        // "input freqdata has nfft/2+1 complex points"
+
+        for (int i = 0; i < m_n/2 + 1; ++i) {
             m_c[i].r = ri[i];
             m_c[i].i = ii[i];
         }
--- a/tests/Makefile	Tue May 06 10:35:01 2014 +0100
+++ b/tests/Makefile	Fri May 09 14:35:46 2014 +0100
@@ -7,7 +7,7 @@
 
 TESTS	:= test-mathutilities test-window test-fft test-pvoc test-resampler test-medianfilter
 
-#VG	:= valgrind
+VG	:= valgrind -q
 
 all: $(TESTS)
 	for t in $(TESTS); do echo "Running $$t"; $(VG) ./"$$t" || exit 1; done
--- a/tests/TestFFT.cpp	Tue May 06 10:35:01 2014 +0100
+++ b/tests/TestFFT.cpp	Fri May 09 14:35:46 2014 +0100
@@ -25,9 +25,15 @@
 
 BOOST_AUTO_TEST_CASE(forwardArrayBounds)
 {
-    // initialise bins to something recognisable, so we can tell
-    // if they haven't been written
-    double in[] = { 1, 1, -1, -1 };
+    // initialise bins to something recognisable, so we can tell if
+    // they haven't been written; and allocate the inputs on the heap
+    // so that, if running under valgrind, we get warnings about
+    // overruns
+    double *in = new double[4];
+    in[0] = 1;
+    in[1] = 1;
+    in[2] = -1;
+    in[3] = -1;
     double re[] = { 999, 999, 999, 999, 999, 999 };
     double im[] = { 999, 999, 999, 999, 999, 999 };
     FFT(4).process(false, in, 0, re+1, im+1);
@@ -36,13 +42,20 @@
     BOOST_CHECK_EQUAL(im[0], 999.0);
     BOOST_CHECK_EQUAL(re[5], 999.0);
     BOOST_CHECK_EQUAL(im[5], 999.0);
+    delete[] in;
 }
 
 BOOST_AUTO_TEST_CASE(r_forwardArrayBounds)
 {
-    // initialise bins to something recognisable, so we can tell
-    // if they haven't been written
-    double in[] = { 1, 1, -1, -1 };
+    // initialise bins to something recognisable, so we can tell if
+    // they haven't been written; and allocate the inputs on the heap
+    // so that, if running under valgrind, we get warnings about
+    // overruns
+    double *in = new double[4];
+    in[0] = 1;
+    in[1] = 1;
+    in[2] = -1;
+    in[3] = -1;
     double re[] = { 999, 999, 999, 999, 999, 999 };
     double im[] = { 999, 999, 999, 999, 999, 999 };
     FFTReal(4).forward(in, re+1, im+1);
@@ -51,14 +64,25 @@
     BOOST_CHECK_EQUAL(im[0], 999.0);
     BOOST_CHECK_EQUAL(re[5], 999.0);
     BOOST_CHECK_EQUAL(im[5], 999.0);
+    delete[] in;
 }
 
 BOOST_AUTO_TEST_CASE(inverseArrayBounds)
 {
-    // initialise bins to something recognisable, so we can tell
-    // if they haven't been written
-    double re[] = { 0, 1, 0, 1 };
-    double im[] = { 0, -2, 0, 2 };
+    // initialise bins to something recognisable, so we can tell if
+    // they haven't been written; and allocate the inputs on the heap
+    // so that, if running under valgrind, we get warnings about
+    // overruns
+    double *re = new double[4];
+    double *im = new double[4];
+    re[0] = 0;
+    re[1] = 1;
+    re[2] = 0;
+    re[3] = 1;
+    im[0] = 0;
+    im[1] = -2;
+    im[2] = 0;
+    im[3] = 2;
     double outre[] = { 999, 999, 999, 999, 999, 999 };
     double outim[] = { 999, 999, 999, 999, 999, 999 };
     FFT(4).process(true, re, im, outre+1, outim+1);
@@ -67,19 +91,31 @@
     BOOST_CHECK_EQUAL(outim[0], 999.0);
     BOOST_CHECK_EQUAL(outre[5], 999.0);
     BOOST_CHECK_EQUAL(outim[5], 999.0);
+    delete[] re;
+    delete[] im;
 }
 
 BOOST_AUTO_TEST_CASE(r_inverseArrayBounds)
 {
-    // initialise bins to something recognisable, so we can tell
-    // if they haven't been written
-    double re[] = { 0, 1, 0 };
-    double im[] = { 0, -2, 0 };
+    // initialise bins to something recognisable, so we can tell if
+    // they haven't been written; and allocate the inputs on the heap
+    // so that, if running under valgrind, we get warnings about
+    // overruns
+    double *re = new double[3];
+    double *im = new double[3];
+    re[0] = 0;
+    re[1] = 1;
+    re[2] = 0;
+    im[0] = 0;
+    im[1] = -2;
+    im[2] = 0;
     double outre[] = { 999, 999, 999, 999, 999, 999 };
     FFTReal(4).inverse(re, im, outre+1);
     // And check we haven't overrun the arrays
     BOOST_CHECK_EQUAL(outre[0], 999.0);
     BOOST_CHECK_EQUAL(outre[5], 999.0);
+    delete[] re;
+    delete[] im;
 }
 
 BOOST_AUTO_TEST_CASE(dc)