Valgrind: Conditional jump or move depends on uninitialised value(s)

[JamieAl]

Hello, I am running into an issue with Valgrind with c++
It is yelling when initializing multidimensional arrays in my code and I don't know why. I have a long code that I will post an example of so people can understand my issue.

I am using mac os with VS code for c++ and running Valgrind with this.


I get the following error:

==42988== Conditional jump or move depends on uninitialised value(s)
==42988==    at 0x100005855: max2Dk(double*) (aFunctions.cpp:265)
==42988==    by 0x10000598F: max_absComp(double (*) [2]) (aFunctions.cpp:295)
==42988==    by 0x100003C9A: Pk(double (*) [2], double (*) [2], double (*) [2], double (*) [2], double (*) [2], double*, double*, double*, double, int) (sFunctions.cpp:485)
==42988==    by 0x10000208E: main (testBed.cpp:304)

I define function max2Dk, which finds the max value of a 2D array in aFunctions.cpp, as the following:

double max2Dk(double arr2D[]){
    double maxVal = 0.;
    for (int i = 0; i < nx; i++){
        for (int j = 0; j < nyk; j++){
265         if (arr2D[j + nyk*i] > maxVal){
                maxVal = arr2D[j + nyk*i];
            }
        }
    }   
    return maxVal;
}

Where, arr2D has a size of nx by nyk are just int

The second function, max_absComp is defined as the following:

double max_absComp(double arr3D[][ncomp]){
    double *absArr;
    absArr = (double*) fftw_malloc(nx*nyk*sizeof(double));
    
    absComp(arr3D, absArr);
    
    
295 double maxVal = max2Dk(absArr);
    
    return maxVal;
}

where arr3D has a size of nx by nyk by ncomp (all are ints), I don't know why valgrind is yelling about initializing here, I thought for multidimensional arrays in C/C++ you have to specify all dimensions except the first.

Definition of absComp function:

void absComp(double arr3D[][ncomp], double arrOut[]){
    for (int i = 0; i < nx ; i++){
        for (int j = 0 ; j < nyk; j++){
            arrOut[j + nyk*i] = sqrt(arr3D[j + nyk*i][0]*arr3D[j + nyk*i][0] + arr3D[j + nyk*i][1]*arr3D[j + nyk*i][1]);
        }
    }
}

I am not sure what is the problem, how any of this is not initialized

Example code:

#include<stdio.h>
#include "sFunctions.h"
#include "aFunctions.h"
#include "fftw3.h"

int main(){     
    double dt_max = 0.1;        
    double tend = 1000.;         
    double err_max = 1.e-8;      
    
    static const int nx = 512;
    static const int ny = 512;
    static const int nyk = ny/2 + 1;
    static const int ncomp = 2;

    int saveNum = 1;
    int iter_max = tend / dt_max; 

//Initialize variables

    fftw_complex *dndxk;
    dndxk = (fftw_complex*) fftw_malloc(nx*nyk* sizeof(fftw_complex)); 
    fftw_complex *dndyk;
    dndyk = (fftw_complex*) fftw_malloc(nx*nyk* sizeof(fftw_complex)); 
//similarly for all variables

    double Oci = e*Bmag/mi;
    double Oce = e*Bmag/me;
    double u[3] = {-500, 0, 0};
    double B[3] = {0,0,5E-5}; 


for (int iter = 0; iter < iter_max; iter++){
        
        calcPotSourcek(dndxk, dndyk, Pik, Pek, nuink, nuiek, nuenk, nueik, isigPk, Oci, Oce, u, B, ksqu, potSourcek);
    

printf("Iteration = %d    t = %.10f ", iter);

  }

  return 0;
}

[dutch]

You probably need to post the whole thing. You can upload a zip somewhere and post a link here.

The printf in main is missing an argument to go with %.10f. You should get a compiler warning about that. Pay attention to and fix all warnings.

max_abxComp is leaking memory since it doesn't free absArr. And if you mean to free it then it seems kind of pointless to have created it in the first place. Why not just:

double max_absComp(double a[][ncomp]) {
    double m = 0;
    for (int i = 0; i < nx * nyk; ++i) {
        double x = sqrt(a[i][0] * a[i][0]
                      + a[i][1] * a[i][1]);
        if (x > m) m = x;
    }
    return m;
}

[JamieAl]

Okay, yes I forgot to add the argument for the time iteration. I am really new to all of this and it's so confusing, I thought the problem has to do with initializing my arrays.

Here's a zip file with the whole thing! Thanks
https://www.zipshare.com/download/eyJhcmNoaXZlSWQiOiJkYWExMGE1ZC02NjMyLTQ1ZDQtYmJiMC1jYjRjNDdjNzdiZjIiLCJlbWFpbCI6ImFzdHJvbHVqeUBnbWFpbC5jb20ifQ==

[JamieAl]

So, basically I am testing some of my functions since when I run my original code I get this huge memory build up when running my memory activity monitor. So, I tested each function individually and it turns out this function I posted about is causing a huge memory leak basically.

[dutch]

Well I showed you one place where you're leaking memory with an array that you don't even need.

Anywhere you have an fftw_malloc you need an fftw_free to free the memory. You are missing a lot of frees.

And you can get rid of all the global variables that shadow function parameters, too. They are meaningless and just waste memory.

[MikeyBoy]

global variables that shadow function parameters

Surely it's the other way around - function parameters shadow global variables? It's the function parameters that the compiler sees, not the global variables.

[ne555]

$ valgrind --track-origins=yes ./program.bin
==50190== Conditional jump or move depends on uninitialised value(s)
==50190==    at 0x4CB87BC: sqrt (in /usr/lib/libm-2.32.so)
==50190==    by 0x10A3A8: absComp(double (*) [2], double*) (arithmeticFunctions.cpp:254)
==50190==    by 0x10A536: max_absComp(double (*) [2]) (arithmeticFunctions.cpp:292)
==50190==    by 0x10BEFD: potentialk(double (*) [2], double (*) [2], double (*) [2], double (*) [2], double (*) [2], double*, double*, double*, double, int) (spectralFunctions.cpp:491)
==50190==    by 0x10D93E: main (testBed.cpp:304)
==50190==  Uninitialised value was created by a heap allocation
==50190==    at 0x483D118: memalign (vg_replace_malloc.c:907)
==50190==    by 0x10D8DD: main (testBed.cpp:300)

//potentialk()
		// Calculate maximum of absolute value of previous phi
		phik_max_old = max_absComp(phik); //485
		
		// Multiply by ninvksqu to get the updated phi
		Arr3DArr2DMult(RHS, ninvksqu, phik);
		
		// Calculate maximum of absolute value of updated phi
		phik_max = max_absComp(phik); //491

//main()
potSourcek = (fftw_complex*) fftw_malloc(nx*nyk* sizeof(fftw_complex));  //testBed.cpp:300 

notice:
- it complains in line 491, but not on 485, however both are the same call: max_absComp(phik)
- the uninitialised value was pointed out to `potSourcek', not `phik'

so, phik was fine at line 485, you called `Arr3DArr2DMult(RHS, ninvksqu, phik)' and now it's uninitialised
checking out `RHS', it's filled like
RHS[j + nyk*i][k] = potSourcek[j + nyk*i][k] - gradNgradPhi_x[j + nyk*i][k] - gradNgradPhi_y[j + nyk*i][k]; //476
using the invalid values from `potSourcek'

so there you are, initialise `potSourcek'
I was hoping that valgrind would detect faulty line 476, however



apart:
- `Comp' makes me think on «compare/comparison», not «computing», I'll suggest to drop the Comp suffix on absComp(), max_absComp()
- «Arr3DArr2DMult() [This function multiplies a 3D Fourier array by a 2D real valued array.]» no, it multiplies a "complex" array by a "real" array. They are both 2d (xy plane) but are stored in a single dimension array, so the function may work for 1d signals, just pass a size argument
- int potentialk(double invnk[16][ncomp], double dndxk[16][ncomp], double dndyk[16][ncomp] ...) first dimension is meaningless to the compiler, it may be used for documentation but that's not your case, as your arrays are not size 16 (or limited to size 16)
- test first with toy examples, things that you may see at a glance that are wrong, a 512x257 matrix is too big
- a little explanation of what the program does would be nice (¿poisson heat equation? ¿what conditions?)
- you are not charged by vowels, don't compress your variable names (you are not the only one that sees your code, not all are immersed in your study subject)
- read and fix the warnings that your compiler throws (-W{all,extra,pedantic} to compile with warnings)

regards

[ne555]

> And you can get rid of all the global variables that shadow function
> parameters, too. They are meaningless and just waste memory.

>> Surely it's the other way around - function parameters shadow global
>> variables? It's the function parameters that the compiler sees, not the
>> global variables.

perhaps «shadow» was not the correct term.
the code has a lot of global variables with the same name as the parameters of the functions, but they are not intended to be used (not the correct size)
so they do nothing, just confuse the reader/programmer

(I thought that all the «set but not used» warnings were referring to those global variables but that isn't the case...)

[dutch]

MikeyBoy wrote:

Surely it's the other way around - function parameters shadow global variables?

You're right, of course. I said "shadow" without really thinking about the technical meaning. I should have just said "pointlessly duplicate".

[JamieAl]

First of all my apologies, I am not familiar with C/C++ and it is very confusing to me. So, I thought I have initialized the dynamic Fourer array 'potSourcek' in main code:

  
//main()
potSourcek = (fftw_complex*) fftw_malloc(nx*nyk* sizeof(fftw_complex));  //testBed.cpp:300  

But, you're saying that because of the new definition of this from RHS, I will have to initialize it again? and where? in the SpectralFunctions.cpp? Sorry trying to understand how these things work.

using the invalid values from `potSourcek'

RHS[j + nyk*i][k] = potSourcek[j + nyk*i][k] - gradNgradPhi_x[j + nyk*i][k] - gradNgradPhi_y[j + nyk*i][k]; //476

Also, this is incorrect, I clearly was misunderstanding the valgrind messages and was testing if the problem was in the function parameters:
int potentialk(double invnk[16][ncomp], double dndxk[16][ncomp], double dndyk[16][ncomp] ...)

I am trying to do a simulation for a 2D fluid model for an instability in the ionosphere. Using a pseudo-spectral method for spatially discretization (2D FFTs in space) and RK (Runge-Kutta) for temporal discretization of the governing equations (continuity, momentum, and energy)
I can write a bit more about the model but you can see I am clearly struggling with syntax and coding unfortunately.

[ne555]

fftw_malloc() only allocates memory, it reserves space for your nx*nyk matrix but it does not set its values
for example, for a constant source term: memset(potSourcek, 42, nx*nyk* sizeof(fftw_complex)); (or just use a loop)

initialise it on main(), after the malloc

[JamieAl]

Thanks! Is there any memory leak you see when running the code? I am running on mac and getting some "bogus" Valgrind error that has to do with mac. After initializing when running this function again, I am still getting a huge surge of memory on my memory activity monitor which means maybe memory isn't being freed properly?

[ne555]

show your updated code, there were other arrays beside `potSourcek' that were not initialised
prepare a smaller testcase to be run (too much time, had to interrupt it, so can't say if you actually `free()' all)

[JamieAl]

When I am running Valgrind, I use the command:

gcc testBed.cpp spectralFunctions.cpp arithmeticFunctions.cpp -lfftw3 -lm -g -o ../../Datasim1_copy/test.out && valgrind --leak-check=yes ./../../Datasim1_copy/test.out

So, wondering if you're running other tools for Valgrind. This command is giving me one error(the same):

==51512== Conditional jump or move depends on uninitialised value(s)
==51512==    at 0x100005845: max2Dk(double*) (arithmeticFunctions.cpp:265)
==51512==    by 0x10000597F: max_absComp(double (*) [2]) (arithmeticFunctions.cpp:295)
==51512==    by 0x100003CA9: potentialk(double (*) [2], double (*) [2], double (*) [2], double (*) [2], double (*) [2], double*, double*, double*, double, int) (spectralFunctions.cpp:492)
==51512==    by 0x10000208C: main (testBed.cpp:306)

So, I only initialized 'potSourcek' and 'absArr' (since it shows in max2Dk and max_absComp and spectralFunction.cpp line 491)

Changes I made in main code:

fftw_complex *potSourcek;
	potSourcek = (fftw_complex*) fftw_malloc(nx*nyk* sizeof(fftw_complex)); 
	//initialize potSourceK, use memset for bits and for loop for values
	
	memset(potSourcek, 42, nx*nyk* sizeof(fftw_complex)); //test  

and in arithmeticFunctions in the max_absComp:

double max_absComp(double arr3D[][ncomp]){
	// Take the absolute value
	double *absArr;
	absArr = (double*) fftw_malloc(nx*nyk*sizeof(double));
	memset(absArr, 42, nx*nyk* sizeof(double)); //test 

	absComp(arr3D, absArr);
	
	// Calculate the max value
	double maxVal = max2Dk(absArr); //by
	
	return maxVal;
}

How can I ask Valgrind to show all uninitialized arrays? Very difficult to understand these vague errors (vague to me at least)

[TheIdeasMan]

Hi,

You could use the compiler to show more warnings:

g++ -std=c++17 -Wall -Wextra -pedantic-errors cpp.*

Note this is a partial command, you can add all your other options after. Also I used g++ rather than gcc.

It is worth reading the compiler manual, there are even more options you could have that are not used with the above command.

[ne555]

use --track-origins=yes on valgrind to trace the uninitialised values

> How can I ask Valgrind to show all uninitialized arrays?
I don't know if it can.
the «Conditional jump or move depends on uninitialised value(s)» means that you did if(n), but `n' had not a valid value
(like if (arr2D[j + nyk*i] > maxVal) in your code)
however, as I said before, it did not detect RHS[j + nyk*i][k] = potSourcek[j + nyk*i][k] - gradNgradPhi_x[j + nyk*i][k] - gradNgradPhi_y[j + nyk*i][k]; which was actually the source of the error

so, as advice, initialise at the point of declaration

    fftw_complex *dndxk; //there is a pointer
    dndxk = (fftw_complex*) fftw_malloc(nx*nyk* sizeof(fftw_complex)); //it points to valid memory
	//¿but what values do its element have? 

analyse every single array that you have

	double *absArr;
	absArr = (double*) fftw_malloc(nx*nyk*sizeof(double));
	memset(absArr, 42, nx*nyk* sizeof(double));

	absComp(arr3D, absArr);

irrc, absComp() will fill `absArr', so there is no need for `memset(absArr)', but doesn't hurt too much either

perhaps you could use `std::vector<std::complex>' instead of your pointers, there you wouldn't have memory issues, just have to adapt your library or search for a wrapper (maybe opencv)


> I am not familiar with C/C++ and it is very confusing to me.
python, octave, r
consider changing your hammer or hiring a carpenter

[JamieAl]

@TheIdeasMan

You could use the compiler to show more warnings:

g++ -std=c++17 -Wall -Wextra -pedantic-errors cpp.*

Thanks! I needed this.

[JamieAl]

@ne555
When I followed your advice, this fixed my original initializing issue. Thanks!

But, now I have this error to deal with:

==57680== Conditional jump or move depends on uninitialised value(s)
==57680==    at 0x100584AD0: __dtoa (in /usr/lib/system/libsystem_c.dylib)
==57680==    by 0x1005AB2DD: __vfprintf (in /usr/lib/system/libsystem_c.dylib)
==57680==    by 0x1005D1AD2: __v2printf (in /usr/lib/system/libsystem_c.dylib)
==57680==    by 0x1005AA2B1: vfprintf_l (in /usr/lib/system/libsystem_c.dylib)
==57680==    by 0x1005A841D: printf (in /usr/lib/system/libsystem_c.dylib)
==57680==    by 0x100002466: main (testBed.cpp:500)
==57680==  Uninitialised value was created by a stack allocation
==57680==    at 0x100002025: main (testBed.cpp:336)

which is line 500:

printf("Iteration = %d    t = %.10f   phi_iter = %d\n", iter, time[iter+1], phi_iter);

and 336:

double time[iter_max + 1];

Thanks again! your comments are vary helpful

[MikeyBoy]

double time[iter_max + 1];

Where are you initialising the contents of the memory that time is pointing to?

Declaring a variable-sized array like that is illegal in standard C++. The size of the array should be a constant known at compile-time.

On line 500, you attempt to access time[iter+1]. You're aware that if iter == iter_max, this will be accessing memory past the end of the array, yes?

[JamieAl]

I wrote underneath this time[0]=0; but as I recently learned that is not a valid c++ expression. So, I wrote, std::vector<double> time(iter_max + 1, 0.0);

Actually, I didn't realize that, if iter == iter_max this will be past the end of array! Thanks!