HPImaging / sv-mbirct

High Performance Model Based Image Reconstruction for Computed Tomography
BSD 3-Clause "New" or "Revised" License
6 stars 7 forks source link

va_arg issue in multialloc() #16

Closed sjkisner closed 2 years ago

sjkisner commented 2 years ago

This historical multialloc() rears it's head!!!

After 30+ years, critical failure now observed on macOS/arm64 (M1), both with homebrew/gcc-11.3 and clang-13.1.

The issue is with variable arguments usage, va_arg (stdarg.h).

int Nx=128,Ny=128;
array = multialloc(sizeof(float),2,Nx,Ny);

produces indeterminate behavior because the va_arg argument was typed to size_t.

d1[i] = va_arg(ap,size_t);

The values returned by va_arg were random, the problem being something like filling 4 bytes of an 8 byte register, and not clearing the remaining bytes.

Applied the minimum necessary fix in commit 32965f0,

d1[i] = (size_t) va_arg(ap,int); 

But in general multialloc() needs to be used cautiously. For example, recast the variable arguments to 'int' when necessary.

unsigned short Nx=128,Ny=128;
array = multialloc(sizeof(float),2,(int)Nx,(int)Ny); 
dyang37 commented 2 years ago

Hey Jordan, just to make sure that I understand this correctly. Before this fix, we would observe that random amount of memory gets allocated on M1 machines because the randomness in va_arg. Are we supposed to see compiler errors or visual differences in reconstructed images as well?

sjkisner commented 2 years ago

Correct, about the random amount of memory.

If the va_arg problem happens, 99% chance the program would crash before getting any output. It's pretty ugly. You may not see any compiler errors or warnings related to this. At least I never have.