Project

General

Profile

Bug #2598

Tools using read_next_x cannot read TNG files with sanitzers

Added by Paul Bauer 4 months ago. Updated about 1 month ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
analysis tools
Target version:
Affected version - extra info:
Affected version:
Difficulty:
uncategorized
Close

Description

While trying to generate more tests I stumbled into this particular bug through TSAN

10:26:50 WARNING: ThreadSanitizer: heap-use-after-free (pid=2863)
10:26:50   Read of size 4 at 0x7b1400000aa0 by main thread:
10:26:50     #0 calc_xcm /home/jenkins/workspace/Matrix_PreSubmit_master@2/a5691cd7/gromacs/src/gromacs/gmxana/princ.cpp:246 (libgromacs.so.4+0x00000047e3f1)
10:26:50     #1 sub_xcm /home/jenkins/workspace/Matrix_PreSubmit_master@2/a5691cd7/gromacs/src/gromacs/gmxana/princ.cpp:263 (libgromacs.so.4+0x00000047e4d8)
10:26:50     #2 gmx_rmsf /home/jenkins/workspace/Matrix_PreSubmit_master@2/a5691cd7/gromacs/src/gromacs/gmxana/gmx_rmsf.cpp:371 (libgromacs.so.4+0x000000415f72)
.
.
.
10:26:50   Previous write of size 8 at 0x7b1400000aa0 by main thread:
10:26:50     #0 realloc ../../../../gcc-7.1.0/libsanitizer/tsan/tsan_interceptors.cc:619 (libtsan.so.0+0x000000027ddb)
10:26:50     #1 save_realloc(char const*, char const*, int, void*, unsigned long, unsigned long) /home/jenkins/workspace/Matrix_PreSubmit_master@2/a5691cd7/gromacs/src/gromacs/utility/smalloc.cpp:163 (libgromacs.so.4+0x0000002451bb)
10:26:50     #2 gmx_srenew_impl<float [3]> /mnt/workspace/Matrix_PreSubmit_master@2/a5691cd7/gromacs/src/gromacs/utility/smalloc.h:225 (libgromacs.so.4+0x0000002898d6)
10:26:50     #3 gmx_read_next_tng_frame(gmx_tng_trajectory*, t_trxframe*, long*, int) /home/jenkins/workspace/Matrix_PreSubmit_master@2/a5691cd7/gromacs/src/gromacs/fileio/tngio.cpp:1523 (libgromacs.so.4+0x0000002898d6)
10:26:50     #4 read_next_frame /home/jenkins/workspace/Matrix_PreSubmit_master@2/a5691cd7/gromacs/src/gromacs/fileio/trxio.cpp:877 (libgromacs.so.4+0x00000029ef7b)
10:26:50     #5 read_next_x /home/jenkins/workspace/Matrix_PreSubmit_master@2/a5691cd7/gromacs/src/gromacs/fileio/trxio.cpp:1116 (libgromacs.so.4+0x0000002a0619)
10:26:50     #6 gmx_rmsf /home/jenkins/workspace/Matrix_PreSubmit_master@2/a5691cd7/gromacs/src/gromacs/gmxana/gmx_rmsf.cpp:406 (libgromacs.so.4+0x0000004163a7)

Not really sure what the issue is, as looking at the functions didn't really help me.
I used the files in src/gromacs/gmxana/tests to run this command
gmx rmsf -s spc2.gro -f spc2-traj.tng -o file -oq pdbfile

I would guess this is a TNG issue, but I don't know enough about it to be sure.
The debugger says that the pointer is before the beginning of an allocation that has been free'd.

History

#1 Updated by Mark Abraham 4 months ago

  • Subject changed from Heap use after free caused by gmx_read_next_tng_frame to Tools using read_next_x cannot read TNG files with sanitzers
  • Category set to analysis tools
  • Target version set to 2019

The read_next_x functionality has been deprecated for ~10 years now. The t_trxframe functionality used by read_next_frame allows e.g. the x vector to be reallocated, and the tool then uses frame.x in its logic. However read_next_x silently continues to use the old x vector that it passed in by value. The shallow copy in the implementation of read_next_x that calls read_next_frame does not work for any input format that might reallocate status->xframe->x. Generally such a reallocation doesn't happen.

However, TNG reading caters to the way TNG frames can have varying numbers of particles by calling srenew, which calls realloc. The sanitizer builds do a reallocation and copy, which breaks the above assumption of read_next_x. TNG reading from read_next_x and read_next_frame with normal builds works fine, so there is formally no bug, but we can't add tests for such tools reading TNG files until we address this.

We could hack the TNG reading to work around this (if the former fr->natoms is the same as the number of particles read from the frame, then skip the call to srenew). But since read_next_x is anyway deprecated, I propose that we
  • add tests for such tools and explicitly avoid reading TNG files for now (noting TODOs), and
  • once all use of read_next_x is gone, then we can activate TNG reading for the tests.
    That way we test the logic of the tool appropriately, and eventually test all the file types that we intend to support.

#2 Updated by Paul Bauer about 1 month ago

  • Target version changed from 2019 to future

not going to happen soon

Also available in: Atom PDF