search

sunpho84 / quda

QUDA is a library for performing calculations in lattice QCD on GPUs.
https://lattice.github.io/quda
Other
0 stars 0 forks source link

Initialization of madwf_param_infile and madwf_param_outfile #1

Open sunpho84 opened 2 years ago

sunpho84 commented 2 years ago

https://github.com/sunpho84/quda/blob/5431b168b09343503d0d676425069dc895879c92/include/invert_quda.h#L377-L378

copy some string around. Problem is, the strings are not guaranteed to be initialized, in facts the initialization is missing from check_params.h https://github.com/sunpho84/quda/blob/5431b168b09343503d0d676425069dc895879c92/lib/check_params.h

so the strcpy of the nonterminated string can do supernasty things, as manifested by valgrind in a tmLQCD run

# TM_QUDA: Performing MG Preconditioner Setup for gauge_id: 0.000000
# TM_QUDA: Generating MG Setup with mu = 0.001120515200 instead of 0.005282279000
==103571== Conditional jump or move depends on uninitialised value(s)
==103571==    at 0x4089A0C: strcpy (vg_replace_strmem.c:513)
==103571==    by 0x1491CA3F: quda::SolverParam::SolverParam(QudaInvertParam_s const&) (invert_quda.h:377)
==103571==    by 0x1490809F: newMultigridQuda (interface_quda.cpp:2474)
==103571==    by 0x10049DA3: _updateQudaMultigridPreconditioner (quda_interface.c:1510)
==103571==    by 0x1004ABDB: _setOneFlavourSolverParam (quda_interface.c:1483)
==103571==    by 0x1004C02F: invert_eo_degenerate_quda (quda_interface.c:2070)
==103571==    by 0x1013FB13: solve_degenerate (monomial_solve.c:127)
==103571==    by 0x100775A7: cloverdetratio_heatbath (cloverdetratio_monomial.c:287)
==103571==    by 0x100366A3: update_tm (update_tm.c:130)
==103571==    by 0x1000758F: main (hmc_tm.c:402)
==103571==  Uninitialised value was created by a stack allocation
==103571==    at 0x10049F3C: _setMGInvertParam (quda_interface.c:1729)

@Marcogarofalo and @kostrzewa I attach you to the topic

I believe the members should be initialized, with Marco we were thinking to something in the line of 

P(madwf_param_infile[0], '\0');
P(madwf_param_outfile[0], '\0');

What do you think?

sunpho84 commented 2 years ago

Alternatively one could edit invert_quda.h like this:

if(madwf_param.madwf_param_load)
    strcpy(madwf_param.madwf_param_infile, param.madwf_param_infile);
kostrzewa commented 2 years ago

Well-spotted! Some of the DW stuff is new, so it may well be that new issues have been introduced recently. When the ndeg-twisted-clover stuff was merged, "my" Wilson two-flavour operator was also replaced in favour of a re-use of the 4dim-part of the DW operator (which makes total sense). It may be that this has led to a behavioural difference between the feature/ndeg-twisted-clover branch (which was tested quite thoroughly) and the resulting code merged into develop.

Marcogarofalo commented 2 years ago

Initialization of madwf_param_infile and madwf_param_outfile #1246