Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uninitialised variables in PropData class #90

Open
rtobar opened this issue Jun 16, 2021 · 0 comments
Open

Uninitialised variables in PropData class #90

rtobar opened this issue Jun 16, 2021 · 0 comments
Labels
bug Something isn't working

Comments

@rtobar
Copy link

rtobar commented Jun 16, 2021

When running the code with a similar setup to that from #87 under valgrind I get the following errors:

==25566== Thread 1:
==25566== Conditional jump or move depends on uninitialised value(s)
==25566==    at 0x375E3E: CalculateSphericalOverdensityExclusive(Options&, PropData&, long long&, NBody::Particle*, double&, double&, double&, double&, double&, std::vector<double, std::allocator<double> >&) (substructureproperties.cxx:7335)
==25566==    by 0x388960: GetProperties(Options&, long long, NBody::Particle*, long long, long long*&, long long*&, PropData*&, long long*&) [clone ._omp_fn.1] (substructureproperties.cxx:434)
==25566==    by 0x55648E5: GOMP_parallel (in /usr/lib/x86_64-linux-gnu/libgomp.so.1.0.0)
==25566==    by 0x353ECA: GetProperties(Options&, long long, NBody::Particle*, long long, long long*&, long long*&, PropData*&, long long*&) (substructureproperties.cxx:414)
==25566==    by 0x3674B5: SortAccordingtoBindingEnergy(Options&, long long, NBody::Particle*, long long, long long*&, long long*, PropData*, long long) (substructureproperties.cxx:5069)
==25566==    by 0x1BA40E: main (main.cxx:555)
==25566== 
==25566== Conditional jump or move depends on uninitialised value(s)
==25566==    at 0x375FD2: CalculateSphericalOverdensityExclusive(Options&, PropData&, long long&, NBody::Particle*, double&, double&, double&, double&, double&, std::vector<double, std::allocator<double> >&) (substructureproperties.cxx:7339)
==25566==    by 0x388960: GetProperties(Options&, long long, NBody::Particle*, long long, long long*&, long long*&, PropData*&, long long*&) [clone ._omp_fn.1] (substructureproperties.cxx:434)
==25566==    by 0x55648E5: GOMP_parallel (in /usr/lib/x86_64-linux-gnu/libgomp.so.1.0.0)
==25566==    by 0x353ECA: GetProperties(Options&, long long, NBody::Particle*, long long, long long*&, long long*&, PropData*&, long long*&) (substructureproperties.cxx:414)
==25566==    by 0x3674B5: SortAccordingtoBindingEnergy(Options&, long long, NBody::Particle*, long long, long long*&, long long*, PropData*, long long) (substructureproperties.cxx:5069)
==25566==    by 0x1BA40E: main (main.cxx:555)
==25566== 
==25566== Conditional jump or move depends on uninitialised value(s)
==25566==    at 0x376080: CalculateSphericalOverdensityExclusive(Options&, PropData&, long long&, NBody::Particle*, double&, double&, double&, double&, double&, std::vector<double, std::allocator<double> >&) (substructureproperties.cxx:7348)
==25566==    by 0x388960: GetProperties(Options&, long long, NBody::Particle*, long long, long long*&, long long*&, PropData*&, long long*&) [clone ._omp_fn.1] (substructureproperties.cxx:434)
==25566==    by 0x55648E5: GOMP_parallel (in /usr/lib/x86_64-linux-gnu/libgomp.so.1.0.0)
==25566==    by 0x353ECA: GetProperties(Options&, long long, NBody::Particle*, long long, long long*&, long long*&, PropData*&, long long*&) (substructureproperties.cxx:414)
==25566==    by 0x3674B5: SortAccordingtoBindingEnergy(Options&, long long, NBody::Particle*, long long, long long*&, long long*, PropData*, long long) (substructureproperties.cxx:5069)
==25566==    by 0x1BA40E: main (main.cxx:555)
==25566== 
==25566== Conditional jump or move depends on uninitialised value(s)
==25566==    at 0x376572: SetSphericalOverdensityMasstoTotalMassExclusive(Options&, PropData&) (substructureproperties.cxx:7397)
==25566==    by 0x388998: GetProperties(Options&, long long, NBody::Particle*, long long, long long*&, long long*&, PropData*&, long long*&) [clone ._omp_fn.1] (substructureproperties.cxx:435)
==25566==    by 0x55648E5: GOMP_parallel (in /usr/lib/x86_64-linux-gnu/libgomp.so.1.0.0)
==25566==    by 0x353ECA: GetProperties(Options&, long long, NBody::Particle*, long long, long long*&, long long*&, PropData*&, long long*&) (substructureproperties.cxx:414)
==25566==    by 0x3674B5: SortAccordingtoBindingEnergy(Options&, long long, NBody::Particle*, long long, long long*&, long long*, PropData*, long long) (substructureproperties.cxx:5069)
==25566==    by 0x1BA40E: main (main.cxx:555)
==25566== 
==25566== Conditional jump or move depends on uninitialised value(s)
==25566==    at 0x37657C: SetSphericalOverdensityMasstoTotalMassExclusive(Options&, PropData&) (substructureproperties.cxx:7397)
==25566==    by 0x388998: GetProperties(Options&, long long, NBody::Particle*, long long, long long*&, long long*&, PropData*&, long long*&) [clone ._omp_fn.1] (substructureproperties.cxx:435)
==25566==    by 0x55648E5: GOMP_parallel (in /usr/lib/x86_64-linux-gnu/libgomp.so.1.0.0)
==25566==    by 0x353ECA: GetProperties(Options&, long long, NBody::Particle*, long long, long long*&, long long*&, PropData*&, long long*&) (substructureproperties.cxx:414)
==25566==    by 0x3674B5: SortAccordingtoBindingEnergy(Options&, long long, NBody::Particle*, long long, long long*&, long long*, PropData*, long long) (substructureproperties.cxx:5069)
==25566==    by 0x1BA40E: main (main.cxx:555)
==25566== 
==25566== Conditional jump or move depends on uninitialised value(s)
==25566==    at 0x52348DC: sqrt (w_sqrt_compat.c:31)
==25566==    by 0x3876D7: GetProperties(Options&, long long, NBody::Particle*, long long, long long*&, long long*&, PropData*&, long long*&) [clone ._omp_fn.1] (substructureproperties.cxx:515)
==25566==    by 0x55648E5: GOMP_parallel (in /usr/lib/x86_64-linux-gnu/libgomp.so.1.0.0)
==25566==    by 0x353ECA: GetProperties(Options&, long long, NBody::Particle*, long long, long long*&, long long*&, PropData*&, long long*&) (substructureproperties.cxx:414)
==25566==    by 0x3674B5: SortAccordingtoBindingEnergy(Options&, long long, NBody::Particle*, long long, long long*&, long long*, PropData*, long long) (substructureproperties.cxx:5069)
==25566==    by 0x1BA40E: main (main.cxx:555)
==25566== Conditional jump or move depends on uninitialised value(s)
==25566==    at 0x375314: CalculateSphericalOverdensitySubhalo(Options&, PropData&, long long&, NBody::Particle*, double&, double&, double&, double&, double&, std::vector<double, std::allocator<double> >&, int) (substructureproperties.cxx:7236)
==25566==    by 0x388A94: GetProperties(Options&, long long, NBody::Particle*, long long, long long*&, long long*&, PropData*&, long long*&) [clone ._omp_fn.1] (substructureproperties.cxx:430)
==25566==    by 0x55648E5: GOMP_parallel (in /usr/lib/x86_64-linux-gnu/libgomp.so.1.0.0)
==25566==    by 0x353ECA: GetProperties(Options&, long long, NBody::Particle*, long long, long long*&, long long*&, PropData*&, long long*&) (substructureproperties.cxx:414)
==25566==    by 0x3674B5: SortAccordingtoBindingEnergy(Options&, long long, NBody::Particle*, long long, long long*&, long long*, PropData*, long long) (substructureproperties.cxx:5069)
==25566==    by 0x1BA40E: main (main.cxx:555)
==25566== 
==25566== Conditional jump or move depends on uninitialised value(s)
==25566==    at 0x375362: CalculateSphericalOverdensitySubhalo(Options&, PropData&, long long&, NBody::Particle*, double&, double&, double&, double&, double&, std::vector<double, std::allocator<double> >&, int) (substructureproperties.cxx:7237)
==25566==    by 0x388A94: GetProperties(Options&, long long, NBody::Particle*, long long, long long*&, long long*&, PropData*&, long long*&) [clone ._omp_fn.1] (substructureproperties.cxx:430)
==25566==    by 0x55648E5: GOMP_parallel (in /usr/lib/x86_64-linux-gnu/libgomp.so.1.0.0)
==25566==    by 0x353ECA: GetProperties(Options&, long long, NBody::Particle*, long long, long long*&, long long*&, PropData*&, long long*&) (substructureproperties.cxx:414)
==25566==    by 0x3674B5: SortAccordingtoBindingEnergy(Options&, long long, NBody::Particle*, long long, long long*&, long long*, PropData*, long long) (substructureproperties.cxx:5069)
==25566==    by 0x1BA40E: main (main.cxx:555)
==25566== 
==25566== Conditional jump or move depends on uninitialised value(s)
==25566==    at 0x3753B0: CalculateSphericalOverdensitySubhalo(Options&, PropData&, long long&, NBody::Particle*, double&, double&, double&, double&, double&, std::vector<double, std::allocator<double> >&, int) (substructureproperties.cxx:7238)
==25566==    by 0x388A94: GetProperties(Options&, long long, NBody::Particle*, long long, long long*&, long long*&, PropData*&, long long*&) [clone ._omp_fn.1] (substructureproperties.cxx:430)
==25566==    by 0x55648E5: GOMP_parallel (in /usr/lib/x86_64-linux-gnu/libgomp.so.1.0.0)
==25566==    by 0x353ECA: GetProperties(Options&, long long, NBody::Particle*, long long, long long*&, long long*&, PropData*&, long long*&) (substructureproperties.cxx:414)
==25566==    by 0x3674B5: SortAccordingtoBindingEnergy(Options&, long long, NBody::Particle*, long long, long long*&, long long*, PropData*, long long) (substructureproperties.cxx:5069)
==25566==    by 0x1BA40E: main (main.cxx:555)
==25566== 
==25566== Conditional jump or move depends on uninitialised value(s)
==25566==    at 0x3753FE: CalculateSphericalOverdensitySubhalo(Options&, PropData&, long long&, NBody::Particle*, double&, double&, double&, double&, double&, std::vector<double, std::allocator<double> >&, int) (substructureproperties.cxx:7239)
==25566==    by 0x388A94: GetProperties(Options&, long long, NBody::Particle*, long long, long long*&, long long*&, PropData*&, long long*&) [clone ._omp_fn.1] (substructureproperties.cxx:430)
==25566==    by 0x55648E5: GOMP_parallel (in /usr/lib/x86_64-linux-gnu/libgomp.so.1.0.0)
==25566==    by 0x353ECA: GetProperties(Options&, long long, NBody::Particle*, long long, long long*&, long long*&, PropData*&, long long*&) (substructureproperties.cxx:414)
==25566==    by 0x3674B5: SortAccordingtoBindingEnergy(Options&, long long, NBody::Particle*, long long, long long*&, long long*, PropData*, long long) (substructureproperties.cxx:5069)
==25566==    by 0x1BA40E: main (main.cxx:555)
==25566== 
==25566== Conditional jump or move depends on uninitialised value(s)
==25566==    at 0x37544C: CalculateSphericalOverdensitySubhalo(Options&, PropData&, long long&, NBody::Particle*, double&, double&, double&, double&, double&, std::vector<double, std::allocator<double> >&, int) (substructureproperties.cxx:7240)
==25566==    by 0x388A94: GetProperties(Options&, long long, NBody::Particle*, long long, long long*&, long long*&, PropData*&, long long*&) [clone ._omp_fn.1] (substructureproperties.cxx:430)
==25566==    by 0x55648E5: GOMP_parallel (in /usr/lib/x86_64-linux-gnu/libgomp.so.1.0.0)
==25566==    by 0x353ECA: GetProperties(Options&, long long, NBody::Particle*, long long, long long*&, long long*&, PropData*&, long long*&) (substructureproperties.cxx:414)
==25566==    by 0x3674B5: SortAccordingtoBindingEnergy(Options&, long long, NBody::Particle*, long long, long long*&, long long*, PropData*, long long) (substructureproperties.cxx:5069)
==25566==    by 0x1BA40E: main (main.cxx:555)
==25566== 

These all seem to stem from the fact that the PropData class has many members with uninitialised variables that are read in various places:

  • substructureproperties.cxx:7335, :7339 and :7397 read gRvir_excl.
  • substructureproperties.cxx:7236 and :515 read gM200c.
  • substructureproperties.cxx:7348 reads gMvir_excl.
  • substructureproperties.cxx:7237 reads gM200m.
  • substructureproperties.cxx:7238 reads gMvir.
  • It's not immediately obvious what's wrong with substructureproperties.cxx:7239 and :7240, so those will require more diagnosis.
@rtobar rtobar added the bug Something isn't working label Jun 16, 2021
rtobar added a commit that referenced this issue Jul 16, 2021
This is purely informational, as it will help us see if there's any
reduction in memory usage after the fixes introduced to address #90.

Signed-off-by: Rodrigo Tobar <rtobar@icrar.org>
rtobar added a commit that referenced this issue Jul 16, 2021
This commit simplifies the initialisation of PropData objects by doing
member initialisation at declaration time instead of duplicating this
(very long) list of members in the constructor body. This duplication
meant there were many fields missing initialisation, leading to reads
from uninitialised values, which in turn can have serious consequences.

This commit should fix all the initialised memory reads reported in #90.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant