kassonlab / covid19-epi

Individual-based epidemiological modeling for COVID-19
GNU Lesser General Public License v2.1
5 stars 5 forks source link

Valgrind found jump on unitialized memory #12

Closed RogerJL closed 4 years ago

RogerJL commented 4 years ago

valgrind ./build/covid19 ==12714== Memcheck, a memory error detector ==12714== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al. ==12714== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info ==12714== Command: ./build/covid19 ==12714== Using seed 14755718885144 ==12714== Conditional jump or move depends on uninitialised value(s) ==12714== at 0x40AA80: household_lat_long (in /home/roger/Documents/2_code/covid19-epi/build/covid19) ==12714== by 0x4032AE: main (in /home/roger/Documents/2_code/covid19-epi/build/covid19) ==12714== max_HH_size = 7 All infections initialized Starting density kernel calculations Done with density kernel calculations in 65220.19 Total initialization time 67010.45 Starting simulation Timestep 0.00

https://github.com/kassonlab/covid19-epi/blob/3dab2299c1ec68e5ce945486edac4ccf9b23b658/covid19.c#L139

HH_count is limited, but this accesses outside the limit...

(gdb) list 0x40aa80 0x40aa80 is in household_lat_long (covid19.c:139). 134 tmp_lat = lat_locale[HH_count]; 135 tmp_lon = lon_locale[HH_count]; 136 / Scale population density / 137 pop_density_init_num[HH_count] = ceil(pop_density_init_num[HH_count] population / land_pop_total_density); 138 tot_pop_density += pop_density_init_num[HH_count]; 139 if (tot_pop_density > population || ceil(pop_density_init_num[HH_count+1] population / land_pop_total_density) < 0.5) { 140 num_locale = HH_count + 1; 141 } 142 143 // Determine city of each population square. Use city data to determine which schools students attend. Workplaces are placed by county. //

HH_count is limited to number, but here one more is added...

RogerJL commented 4 years ago

I have run on unmodified cloned code, using valgrind to find usage of uninitialized memory, array overruns, etc

FACT is - an uninitialized memory is being used to determine if a jump is taken or not on that line. If you think that is OK its on you. (removing the additional comment as it might lead you wrong)

RogerJL commented 4 years ago

I understand code, when my spine tingles there are usually something strange going on. At first look I might not understand exactly what the code does. But this report was really about an issue found by valgrind, YOU closed the issue without addressing the valgrind one just because of this - it can be related, but it is better to keep the issues apart in one report...

peterkasson commented 4 years ago

@RogerJL would you be willing to test master:HEAD to see if this is fixed?

RogerJL commented 4 years ago

That error did not trigger now

peterkasson commented 4 years ago

Excellent, thanks. We'll consider this one closed.