MDSplus / mdsplus

The MDSplus data management system
https://mdsplus.org/
Other
75 stars 44 forks source link

Bug in TDI functions REPLICATE/SPREAD with scalar inputs due to uninitialized memory #2831

Open merlea opened 1 day ago

merlea commented 1 day ago

Affiliation SPC-EPFL

Version(s) Affected Client Version: Stable 7.142.81, Alpha 7.148.1

Platform(s) Ubuntu 24.04

Installation Method(s) DEB package from official MDSplus repository

Describe the bug Calling the TDI functions REPLICATE or SPREAD with scalar inputs can result in arrays with uninitialized bounds. This can then result in segmentation faults upon use of these arrays on certain systems.

To Reproduce

  1. Build MDSplus without debug
  2. Open tdic
  3. Enter _aeps=replicate(0.05d0,0,5)
  4. Output is an array with bounds (SET_RANGE) where bounds are uninitialized.
    TDI> _aeps = replicate(0.05d0,0,5)
    Set_Range(1068079513:-1717986918,[.05D0,.05D0,.05D0,.05D0,.05D0])
  5. As this bug relies on uninitialized memory, the behaviour is not entirely predictable.
    Some other options that might trigger this output are replicate(0.05d0,0,5) or _aeps=replicate(0.05d0,0,5),_aeps.
    Be sure to try each option a few times (10 is usually enough).
  6. With the erroneous answer, subscripting the array yields various incorrect answers.
  7. A similar problem can be seen with the spread function

Expected behavior The return value should be a simple array.

TDI> _aeps = replicate(0.05d0,0,5)
[.05D0,.05D0,.05D0,.05D0,.05D0]

Additional context The source of the error seems to be the block starting at tdishr/TdiTrans.c#L296. There might be several problems:

  1. The variable array_bounds arr is never initialized, in particular arr.arflags.bounds can be 0 or 1 depending on the initial memory state (when compiled with debug, the memory is probably always initialized to 0 and thus the problem is absent.
  2. arr.m is filled even if arr.aflags.coeff is explicitly set to 0.

One possible fix is to initialize the variable arr with array_bounds arr={0}, but the conversion of scalar descriptors still seems dubious.

I also noticed that the behaviour of REPLICATE with array with bounds as inputs might not be well defined. At the moment we get the following:

TDI> _ra = replicate(set_range(2:3,[0.05d0,0.05d0]),0,5)
Set_Range(2:3,[.05D0,.05D0,.05D0,.05D0,.05D0,.05D0,.05D0,.05D0,.05D0,.05D0])

whereas either a simple array or an error (such as invalid class) should be triggered.

cenkoloji commented 1 day ago

related to #2517?